mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
refactor(adapter-utils): extract the shared workspace-restore teardown factory (#12196)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Agent adapters run workspace restore steps when an ACP run ends. > - Claude, Codex, and Gemini each kept a near-identical teardown closure. > - Duplicate closures require the same defect fix in three files. > - This pull request adds one shared workspace-restore teardown factory and keeps each adapter's message strings. > - The benefit is one tested restore-failure path with the same output and outcome for all three adapters. ## Linked Issues or Issue Description **What existing behavior does this improve?** The Claude, Codex, and Gemini ACP adapters restore the workspace during teardown and report restore failures with an allowlisted message. **Subsystem affected** `packages/adapters/` and `packages/adapter-utils/`. **Current behavior** Each adapter keeps a near-identical closure. The closure logs a start line, restores the workspace, classifies errors, and logs a fixed failure line. **Proposed behavior** A shared `createWorkspaceRestoreTeardown` factory owns the common steps. Each adapter passes its staged runtime, log sink, start line, and failure prefix. **Reason and benefit** The shared factory removes duplicate error handling. One tested implementation now preserves the existing output and outcome for all three adapters. **Breaking changes** None. The refactor preserves the emitted lines and returned outcomes. **Additional context** This pull request contains no public issue reference because no related public issue was found. ## What Changed - Add `createWorkspaceRestoreTeardown` to `packages/adapter-utils`. - Move the shared restore, classify, and allowlisted log flow into the factory. - Update the Claude, Codex, and Gemini ACP adapters to call the factory. - Add a table-driven test for all three message pairs. - Keep one end-to-end restore-failure regression test per adapter. ## Verification - `pnpm --filter @paperclipai/adapter-claude-local typecheck` - `pnpm --filter @paperclipai/adapter-codex-local typecheck` - `pnpm --filter @paperclipai/adapter-gemini-local typecheck` - `npx vitest run packages/adapter-utils/src/workspace-restore-teardown.test.ts` - `npx vitest run packages/adapter-utils/src/workspace-restore-merge.test.ts` - `npx vitest run packages/adapters/claude-local/src/server/acp.test.ts` - `npx vitest run packages/adapters/codex-local/src/server/acp.test.ts` - `npx vitest run packages/adapters/gemini-local/src/server/acp.test.ts` - Continuous integration must pass before merge, except for the known pre-existing failures listed in the handoff. ## Risks Low risk. This change moves shared code without changing behavior. The adapter-specific message strings remain unchanged. ## Model Used OpenAI GPT-5, exact model ID `gpt-5`, tool use and code review assistance. The context window size was not provided by the runtime. ## 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>
This commit is contained in:
Nicky LeachandPaperclip authored and GitHub committed
1 parent
dc30dc4f34
commit
821573ede8
7 files changed
+276
-236
No files matched your search
@@ -48,10 +48,7 @@ import {
|
||||
classifyThrownErrorClass,
|
||||
logSandboxProbeDiagnostic,
|
||||
} from "./probe-diagnostics.js";
|
||||
import {
|
||||
classifyWorkspaceRestoreFailure,
|
||||
describeWorkspaceRestoreFailure,
|
||||
} from "@paperclipai/adapter-utils/workspace-restore-merge";
|
||||
import { createWorkspaceRestoreTeardown } from "@paperclipai/adapter-utils/workspace-restore-teardown";
|
||||
import { buildLocalAdapterTestProbeEnv } from "./probe-env.js";
|
||||
import { detectClaudeLoginRequired, parseClaudeStreamJson } from "./parse.js";
|
||||
import { buildClaudeProbePermissionArgs } from "./permissions.js";
|
||||
@@ -219,24 +216,13 @@ async function prepareClaudeRemoteManagedHome(
|
||||
// the host. A restore miss is logged and never fails the run.
|
||||
const registerWorkspaceSyncBack = (
|
||||
stagedRuntime: AcpxRemoteManagedHomeResult["stagedRuntime"],
|
||||
): AcpxRemoteManagedHomeResult["teardown"] => async () => {
|
||||
try {
|
||||
await onLog("stdout", "[paperclip] Restoring workspace changes from the sandbox.\n");
|
||||
await stagedRuntime.restoreWorkspace((line) => onLog("stdout", line));
|
||||
return { ok: true };
|
||||
} catch (err) {
|
||||
// The run log is readable by any same-company actor, so it must never
|
||||
// carry the caught error's own message: that message can hold a host
|
||||
// filesystem path or a process id. Log only the fixed, allowlisted
|
||||
// diagnostic for the classified code.
|
||||
const code = classifyWorkspaceRestoreFailure(err);
|
||||
await onLog(
|
||||
"stderr",
|
||||
`[paperclip] Claude ACP teardown workspace restore failed: ${describeWorkspaceRestoreFailure(code)}\n`,
|
||||
);
|
||||
return { ok: false, code };
|
||||
}
|
||||
};
|
||||
): AcpxRemoteManagedHomeResult["teardown"] =>
|
||||
createWorkspaceRestoreTeardown({
|
||||
stagedRuntime,
|
||||
onLog,
|
||||
startMessage: "[paperclip] Restoring workspace changes from the sandbox.\n",
|
||||
failurePrefix: "[paperclip] Claude ACP teardown workspace restore failed",
|
||||
});
|
||||
const envConfig = parseObject(input.config.env);
|
||||
const explicitClaudeConfigDir =
|
||||
typeof envConfig.CLAUDE_CONFIG_DIR === "string" && envConfig.CLAUDE_CONFIG_DIR.trim().length > 0
|
||||
|
||||
@@ -1,9 +1,26 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import type { AdapterExecutionContext, AdapterInvocationMeta } from "@paperclipai/adapter-utils";
|
||||
import { runChildProcess } from "@paperclipai/adapter-utils/server-utils";
|
||||
|
||||
// Every test in this file needs a real teardown, so the mock below delegates
|
||||
// to the actual factory by default. Only the wiring test further down reads
|
||||
// the call arguments; it does not change this behavior.
|
||||
const mockCreateWorkspaceRestoreTeardown = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("@paperclipai/adapter-utils/workspace-restore-teardown", async (importOriginal) => {
|
||||
const actual = await importOriginal<Record<string, unknown>>();
|
||||
mockCreateWorkspaceRestoreTeardown.mockImplementation(
|
||||
actual.createWorkspaceRestoreTeardown as (...args: unknown[]) => unknown,
|
||||
);
|
||||
return {
|
||||
...actual,
|
||||
createWorkspaceRestoreTeardown: mockCreateWorkspaceRestoreTeardown,
|
||||
};
|
||||
});
|
||||
|
||||
import {
|
||||
buildCodexAcpConfig,
|
||||
createCodexAcpExecutor,
|
||||
@@ -1135,99 +1152,57 @@ describe("codex_local ACP lane", () => {
|
||||
expect(hostAuth.tokens.refresh_token).toBe("ref-sandbox-newer");
|
||||
});
|
||||
|
||||
it("test_codex_acp_teardown_restore_failure_sanitizes_the_run_log", async () => {
|
||||
// Security regression for a workspace-restore write failure: the run log
|
||||
// is readable by any same-company actor, so the teardown must never write
|
||||
// the caught error's own message there — that message can carry the host
|
||||
// workspace path. Force a real EACCES by making the workspace read-only,
|
||||
// and name it with a sentinel marker so any leak is easy to spot.
|
||||
const runId = "run-restore-failure";
|
||||
const root = await makeTempRoot("paperclip-codex-acp-restore-failure-");
|
||||
const localCwd = path.join(root, "SENTINEL-HOST-PATH-marker", "worktree");
|
||||
it("passes the Codex-specific teardown messages to the shared workspace-restore-teardown factory", async () => {
|
||||
// Wiring test only: `createWorkspaceRestoreTeardown` owns the
|
||||
// classify-and-redact contract, proven once for all three adapters by its
|
||||
// own table-driven test in `packages/adapter-utils`. This test proves only
|
||||
// that the Codex adapter passes its own two message strings to it.
|
||||
// Clear the shared, hoisted mock first: earlier tests in this file also
|
||||
// call through it, and a leftover call could hide a real wiring bug.
|
||||
mockCreateWorkspaceRestoreTeardown.mockClear();
|
||||
const root = await makeTempRoot("paperclip-codex-acp-teardown-wiring-");
|
||||
const localCwd = path.join(root, "worktree");
|
||||
const remoteCwd = path.join(root, "remote-workspace");
|
||||
const sourceHome = path.join(root, "codex-home");
|
||||
const sharedHostHome = path.join(root, "shared-codex-home");
|
||||
await fs.mkdir(localCwd, { recursive: true });
|
||||
await fs.mkdir(remoteCwd, { recursive: true });
|
||||
await fs.mkdir(sourceHome, { recursive: true });
|
||||
await fs.mkdir(sharedHostHome, { recursive: true });
|
||||
await fs.writeFile(path.join(localCwd, "hello.txt"), "hi", "utf8");
|
||||
process.env.CODEX_HOME = sharedHostHome;
|
||||
|
||||
// The runtime writes a new file into the in-sandbox workspace during the
|
||||
// turn, so the teardown's restore has something to copy back — and a new
|
||||
// file is exactly what a read-only workspace directory rejects. The
|
||||
// workspace turns read-only only after the turn's own writes, so the
|
||||
// teardown restore that runs after the turn is the write this forces to
|
||||
// fail.
|
||||
const runtime = new FakeRuntime({});
|
||||
const startTurn = runtime.startTurn.bind(runtime);
|
||||
runtime.startTurn = (input) => {
|
||||
const turn = startTurn(input);
|
||||
const remoteWorkspaceCwd = input.handle.cwd ?? remoteCwd;
|
||||
return {
|
||||
...turn,
|
||||
result: (async () => {
|
||||
await fs.writeFile(path.join(remoteWorkspaceCwd, "from-sandbox.txt"), "synced", "utf8");
|
||||
await fs.chmod(localCwd, 0o500);
|
||||
return await turn.result;
|
||||
})(),
|
||||
};
|
||||
};
|
||||
|
||||
const stagedRuntimes = new Map();
|
||||
const execute = createCodexAcpExecutor({
|
||||
createRuntime: (options: FakeRuntimeOptions) => {
|
||||
Object.assign(runtime.options, options);
|
||||
return runtime as never;
|
||||
},
|
||||
stagedRuntimes,
|
||||
createRuntime: (options: FakeRuntimeOptions) => new FakeRuntime(options) as never,
|
||||
stagedRuntimes: new Map(),
|
||||
stagingLocks: new Map(),
|
||||
});
|
||||
const result = await execute(
|
||||
buildContext(localCwd, {
|
||||
config: {
|
||||
engine: "acp",
|
||||
cwd: localCwd,
|
||||
agentCommand: "node ./fake-acp.js",
|
||||
stateDir: path.join(root, "state"),
|
||||
env: { CODEX_HOME: path.join(root, "codex-home") },
|
||||
promptTemplate: "Do the assigned work.",
|
||||
},
|
||||
context: {
|
||||
issueId: "issue-1",
|
||||
paperclipWorkspace: { cwd: localCwd, source: "project_workspace", workspaceId: "workspace-1" },
|
||||
},
|
||||
executionTarget: {
|
||||
kind: "remote",
|
||||
transport: "sandbox",
|
||||
providerKey: "fake-plugin",
|
||||
remoteCwd,
|
||||
runner: createLocalSandboxRunner(),
|
||||
} as never,
|
||||
authToken: "real-run-jwt",
|
||||
}),
|
||||
);
|
||||
|
||||
const loggedLines: string[] = [];
|
||||
try {
|
||||
const result = await execute(
|
||||
buildContext(localCwd, {
|
||||
runId,
|
||||
config: {
|
||||
engine: "acp",
|
||||
cwd: localCwd,
|
||||
agentCommand: "node ./fake-acp.js",
|
||||
stateDir: path.join(root, "state"),
|
||||
env: { CODEX_HOME: sourceHome },
|
||||
promptTemplate: "Do the assigned work.",
|
||||
},
|
||||
context: {
|
||||
issueId: "issue-1",
|
||||
paperclipWorkspace: { cwd: localCwd, source: "project_workspace", workspaceId: "workspace-1" },
|
||||
},
|
||||
executionTarget: {
|
||||
kind: "remote",
|
||||
transport: "sandbox",
|
||||
providerKey: "fake-plugin",
|
||||
remoteCwd,
|
||||
runner: createLocalSandboxRunner(),
|
||||
} as never,
|
||||
authToken: "real-run-jwt",
|
||||
onLog: async (_stream, chunk) => {
|
||||
loggedLines.push(chunk);
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
// Fail-open: the restore miss never changes the run's exit code or
|
||||
// status, and it surfaces as one allowlisted code — never the raw error.
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.resultJson?.workspaceRestoreFailure).toBe("restore_permission_denied");
|
||||
const allLogs = loggedLines.join("");
|
||||
expect(allLogs).not.toContain("SENTINEL-HOST-PATH-marker");
|
||||
expect(allLogs).not.toContain(localCwd);
|
||||
expect(allLogs).not.toContain("EACCES");
|
||||
expect(allLogs).toContain("permission denied");
|
||||
} finally {
|
||||
await fs.chmod(localCwd, 0o700).catch(() => undefined);
|
||||
}
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(mockCreateWorkspaceRestoreTeardown).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
startMessage: "[paperclip] Restoring workspace changes and Codex auth from the sandbox.\n",
|
||||
failurePrefix: "[paperclip] Codex ACP teardown restore/copy-back failed",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("falls back to the CLI lane for a runner-less sandbox even when the ACP command is set", async () => {
|
||||
|
||||
@@ -35,10 +35,7 @@ import {
|
||||
asString,
|
||||
parseObject,
|
||||
} from "@paperclipai/adapter-utils/server-utils";
|
||||
import {
|
||||
classifyWorkspaceRestoreFailure,
|
||||
describeWorkspaceRestoreFailure,
|
||||
} from "@paperclipai/adapter-utils/workspace-restore-merge";
|
||||
import { createWorkspaceRestoreTeardown } from "@paperclipai/adapter-utils/workspace-restore-teardown";
|
||||
import { normalizeCodexModel } from "../index.js";
|
||||
import { classifyCodexAuthRefreshFailure } from "./parse.js";
|
||||
import { copyBackCodexAuth } from "./codex-auth-copyback.js";
|
||||
@@ -240,32 +237,16 @@ async function prepareCodexRemoteManagedHome(
|
||||
// without its staged home. Host staged-temp removal is deliberately NOT here
|
||||
// — see `disposeStaged` — so caching this runtime for reuse never destroys
|
||||
// resources the next resume needs.
|
||||
teardown: async () => {
|
||||
try {
|
||||
await onLog(
|
||||
"stdout",
|
||||
"[paperclip] Restoring workspace changes and Codex auth from the sandbox.\n",
|
||||
);
|
||||
await stagedRuntime.restoreWorkspace((line) => onLog("stdout", line));
|
||||
return { ok: true };
|
||||
} catch (err) {
|
||||
// Fail-soft: a teardown copy-back miss loses this rotation and surfaces
|
||||
// loudly as refresh_token_reused on the next host Codex use (re-auth
|
||||
// recovers) — never silent host-credential corruption, so it must not
|
||||
// mask the run result.
|
||||
//
|
||||
// The run log is readable by any same-company actor, so it must never
|
||||
// carry the caught error's own message: that message can hold a host
|
||||
// filesystem path or a process id. Log only the fixed, allowlisted
|
||||
// diagnostic for the classified code.
|
||||
const code = classifyWorkspaceRestoreFailure(err);
|
||||
await onLog(
|
||||
"stderr",
|
||||
`[paperclip] Codex ACP teardown restore/copy-back failed: ${describeWorkspaceRestoreFailure(code)}\n`,
|
||||
);
|
||||
return { ok: false, code };
|
||||
}
|
||||
},
|
||||
// Fail-soft: a teardown copy-back miss loses this rotation and surfaces
|
||||
// loudly as refresh_token_reused on the next host Codex use (re-auth
|
||||
// recovers) — never silent host-credential corruption, so it must not
|
||||
// mask the run result.
|
||||
teardown: createWorkspaceRestoreTeardown({
|
||||
stagedRuntime,
|
||||
onLog,
|
||||
startMessage: "[paperclip] Restoring workspace changes and Codex auth from the sandbox.\n",
|
||||
failurePrefix: "[paperclip] Codex ACP teardown restore/copy-back failed",
|
||||
}),
|
||||
// One-time cleanup of the HOST staged home temp dir. Fired ONLY when the
|
||||
// staged runtime is dropped (failed/cancelled/timed-out turn, incompatible
|
||||
// re-stage, idle eviction) — never on a clean turn that keeps the runtime
|
||||
|
||||
@@ -1,9 +1,26 @@
|
||||
import fs from "node:fs/promises";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { afterEach, describe, expect, it } from "vitest";
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import type { AdapterExecutionContext, AdapterInvocationMeta } from "@paperclipai/adapter-utils";
|
||||
import { runChildProcess } from "@paperclipai/adapter-utils/server-utils";
|
||||
|
||||
// Every test in this file needs a real teardown, so the mock below delegates
|
||||
// to the actual factory by default. Only the wiring test further down reads
|
||||
// the call arguments; it does not change this behavior.
|
||||
const mockCreateWorkspaceRestoreTeardown = vi.hoisted(() => vi.fn());
|
||||
|
||||
vi.mock("@paperclipai/adapter-utils/workspace-restore-teardown", async (importOriginal) => {
|
||||
const actual = await importOriginal<Record<string, unknown>>();
|
||||
mockCreateWorkspaceRestoreTeardown.mockImplementation(
|
||||
actual.createWorkspaceRestoreTeardown as (...args: unknown[]) => unknown,
|
||||
);
|
||||
return {
|
||||
...actual,
|
||||
createWorkspaceRestoreTeardown: mockCreateWorkspaceRestoreTeardown,
|
||||
};
|
||||
});
|
||||
|
||||
import {
|
||||
buildGeminiAcpConfig,
|
||||
createGeminiAcpExecutor,
|
||||
@@ -613,88 +630,54 @@ describe("gemini_local ACP lane", () => {
|
||||
await expect(fs.readFile(path.join(localCwd, "from-sandbox.txt"), "utf8")).resolves.toBe("synced");
|
||||
});
|
||||
|
||||
it("test_gemini_acp_teardown_restore_failure_sanitizes_the_run_log", async () => {
|
||||
// Security regression for a workspace-restore write failure: the run log
|
||||
// is readable by any same-company actor, so the teardown must never write
|
||||
// the caught error's own message there — that message can carry the host
|
||||
// workspace path. Force a real EACCES by making the workspace read-only,
|
||||
// and name it with a sentinel marker so any leak is easy to spot.
|
||||
const root = await makeTempRoot("paperclip-gemini-acp-restore-failure-");
|
||||
const localCwd = path.join(root, "SENTINEL-HOST-PATH-marker", "worktree");
|
||||
it("passes the Gemini-specific teardown messages to the shared workspace-restore-teardown factory", async () => {
|
||||
// Wiring test only: `createWorkspaceRestoreTeardown` owns the
|
||||
// classify-and-redact contract, proven once for all three adapters by its
|
||||
// own table-driven test in `packages/adapter-utils`. This test proves only
|
||||
// that the Gemini adapter passes its own two message strings to it.
|
||||
// Clear the shared, hoisted mock first: earlier tests in this file also
|
||||
// call through it, and a leftover call could hide a real wiring bug.
|
||||
mockCreateWorkspaceRestoreTeardown.mockClear();
|
||||
const root = await makeTempRoot("paperclip-gemini-acp-teardown-wiring-");
|
||||
const localCwd = path.join(root, "worktree");
|
||||
const remoteCwd = path.join(root, "remote-workspace");
|
||||
await fs.mkdir(localCwd, { recursive: true });
|
||||
await fs.mkdir(remoteCwd, { recursive: true });
|
||||
await fs.writeFile(path.join(localCwd, "hello.txt"), "hi", "utf8");
|
||||
|
||||
// The runtime writes a new file into the in-sandbox workspace during the
|
||||
// turn, so the teardown's restore has something to copy back — and a new
|
||||
// file is exactly what a read-only workspace directory rejects. The
|
||||
// workspace turns read-only only after the turn's own writes, so the
|
||||
// teardown restore that runs after the turn is the write this forces to
|
||||
// fail.
|
||||
const runtime = new FakeRuntime({});
|
||||
const startTurn = runtime.startTurn.bind(runtime);
|
||||
runtime.startTurn = (input) => {
|
||||
const turn = startTurn(input);
|
||||
const remoteWorkspaceCwd = input.handle.cwd ?? remoteCwd;
|
||||
return {
|
||||
...turn,
|
||||
result: (async () => {
|
||||
await fs.writeFile(path.join(remoteWorkspaceCwd, "from-sandbox.txt"), "synced", "utf8");
|
||||
await fs.chmod(localCwd, 0o500);
|
||||
return await turn.result;
|
||||
})(),
|
||||
};
|
||||
};
|
||||
|
||||
const execute = createGeminiAcpExecutor({
|
||||
createRuntime: (options) => {
|
||||
Object.assign(runtime.options, options);
|
||||
return runtime as never;
|
||||
},
|
||||
createRuntime: (options) => new FakeRuntime(options) as never,
|
||||
});
|
||||
const result = await execute(
|
||||
buildContext(localCwd, {
|
||||
config: {
|
||||
engine: "acp",
|
||||
cwd: localCwd,
|
||||
agentCommand: "node ./fake-acp.js",
|
||||
stateDir: path.join(root, "state"),
|
||||
promptTemplate: "Do the assigned work.",
|
||||
},
|
||||
context: {
|
||||
issueId: "issue-1",
|
||||
paperclipWorkspace: { cwd: localCwd, source: "project_workspace", workspaceId: "workspace-1" },
|
||||
},
|
||||
executionTarget: {
|
||||
kind: "remote",
|
||||
transport: "sandbox",
|
||||
providerKey: "fake-plugin",
|
||||
remoteCwd,
|
||||
runner: createLocalSandboxRunner(),
|
||||
} as never,
|
||||
authToken: "real-run-jwt",
|
||||
}),
|
||||
);
|
||||
|
||||
const loggedLines: string[] = [];
|
||||
try {
|
||||
const result = await execute(
|
||||
buildContext(localCwd, {
|
||||
config: {
|
||||
engine: "acp",
|
||||
cwd: localCwd,
|
||||
agentCommand: "node ./fake-acp.js",
|
||||
stateDir: path.join(root, "state"),
|
||||
promptTemplate: "Do the assigned work.",
|
||||
},
|
||||
context: {
|
||||
issueId: "issue-1",
|
||||
paperclipWorkspace: { cwd: localCwd, source: "project_workspace", workspaceId: "workspace-1" },
|
||||
},
|
||||
executionTarget: {
|
||||
kind: "remote",
|
||||
transport: "sandbox",
|
||||
providerKey: "fake-plugin",
|
||||
remoteCwd,
|
||||
runner: createLocalSandboxRunner(),
|
||||
} as never,
|
||||
authToken: "real-run-jwt",
|
||||
onLog: async (_stream, chunk) => {
|
||||
loggedLines.push(chunk);
|
||||
},
|
||||
}),
|
||||
);
|
||||
|
||||
// Fail-open: the restore miss never changes the run's exit code or
|
||||
// status, and it surfaces as one allowlisted code — never the raw error.
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(result.resultJson?.workspaceRestoreFailure).toBe("restore_permission_denied");
|
||||
const allLogs = loggedLines.join("");
|
||||
expect(allLogs).not.toContain("SENTINEL-HOST-PATH-marker");
|
||||
expect(allLogs).not.toContain(localCwd);
|
||||
expect(allLogs).not.toContain("EACCES");
|
||||
expect(allLogs).toContain("permission denied");
|
||||
} finally {
|
||||
await fs.chmod(localCwd, 0o700).catch(() => undefined);
|
||||
}
|
||||
expect(result.exitCode).toBe(0);
|
||||
expect(mockCreateWorkspaceRestoreTeardown).toHaveBeenCalledWith(
|
||||
expect.objectContaining({
|
||||
startMessage: "[paperclip] Restoring workspace changes from the sandbox.\n",
|
||||
failurePrefix: "[paperclip] Gemini ACP teardown workspace restore failed",
|
||||
}),
|
||||
);
|
||||
});
|
||||
|
||||
it("does not persist an api-key auth selector from a host-only credential", async () => {
|
||||
|
||||
@@ -31,10 +31,7 @@ import {
|
||||
asString,
|
||||
parseObject,
|
||||
} from "@paperclipai/adapter-utils/server-utils";
|
||||
import {
|
||||
classifyWorkspaceRestoreFailure,
|
||||
describeWorkspaceRestoreFailure,
|
||||
} from "@paperclipai/adapter-utils/workspace-restore-merge";
|
||||
import { createWorkspaceRestoreTeardown } from "@paperclipai/adapter-utils/workspace-restore-teardown";
|
||||
import { DEFAULT_GEMINI_LOCAL_MODEL } from "../index.js";
|
||||
|
||||
const moduleDir = path.dirname(fileURLToPath(import.meta.url));
|
||||
@@ -166,24 +163,13 @@ async function prepareGeminiRemoteManagedHome(
|
||||
// the host. A restore miss is logged and never fails the run.
|
||||
const registerWorkspaceSyncBack = (
|
||||
stagedRuntime: AcpxRemoteManagedHomeResult["stagedRuntime"],
|
||||
): AcpxRemoteManagedHomeResult["teardown"] => async () => {
|
||||
try {
|
||||
await onLog("stdout", "[paperclip] Restoring workspace changes from the sandbox.\n");
|
||||
await stagedRuntime.restoreWorkspace((line) => onLog("stdout", line));
|
||||
return { ok: true };
|
||||
} catch (err) {
|
||||
// The run log is readable by any same-company actor, so it must never
|
||||
// carry the caught error's own message: that message can hold a host
|
||||
// filesystem path or a process id. Log only the fixed, allowlisted
|
||||
// diagnostic for the classified code.
|
||||
const code = classifyWorkspaceRestoreFailure(err);
|
||||
await onLog(
|
||||
"stderr",
|
||||
`[paperclip] Gemini ACP teardown workspace restore failed: ${describeWorkspaceRestoreFailure(code)}\n`,
|
||||
);
|
||||
return { ok: false, code };
|
||||
}
|
||||
};
|
||||
): AcpxRemoteManagedHomeResult["teardown"] =>
|
||||
createWorkspaceRestoreTeardown({
|
||||
stagedRuntime,
|
||||
onLog,
|
||||
startMessage: "[paperclip] Restoring workspace changes from the sandbox.\n",
|
||||
failurePrefix: "[paperclip] Gemini ACP teardown workspace restore failed",
|
||||
});
|
||||
const geminiSkillsHome = resolveGeminiSkillsHome(input.config);
|
||||
const stagedRuntime = await input.stage(
|
||||
geminiSkillsHome
|
||||
|
||||
Reference in new issue
Block a user