diff --git a/packages/adapter-utils/src/acpx-engine/execute.test.ts b/packages/adapter-utils/src/acpx-engine/execute.test.ts index e9c4f0a576..5981cc79cf 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.test.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.test.ts @@ -3368,6 +3368,279 @@ describe("ACPX engine remote managed-home seam (PR 2: per-adapter home seed)", ( }); }); +describe("ACPX engine Claude skill bundle staging (remote ACP lane)", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + async function setupRemoteSandbox() { + const root = await makeTempRoot(); + const stateDir = path.join(root, "state"); + 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 }); + const executionTarget = { + kind: "remote", + transport: "sandbox", + providerKey: "fake-plugin", + remoteCwd, + runner: createLocalSandboxRunner(), + }; + return { root, stateDir, localCwd, remoteCwd, executionTarget }; + } + + // A stand-in for the real Claude seam (`claude-local/server/acp.ts`): stage + // the bundle the engine hands it, at the same asset key and + // `followSymlinks` value the real seam uses. This isolates the ENGINE's own + // contract — computing and threading `skillsBundleDir`, then rewriting the + // prompt/identity once staging resolves — from the real seam, which has its + // own test in `claude-local/server/acp.test.ts`. + function stagingClaudeSeam(): AcpxEngineExecutorOptions["prepareRemoteManagedHome"] { + return async (input) => { + const stagedRuntime = await input.stage( + input.skillsBundleDir + ? [{ key: "skills", localDir: input.skillsBundleDir, followSymlinks: false }] + : [], + ); + return { stagedRuntime }; + }; + } + + it("rewrites the prompt and skill identity onto the in-sandbox skill root once the bundle is staged", async () => { + const { stateDir, localCwd, executionTarget } = await setupRemoteSandbox(); + const skill = await createSkill(path.join(localCwd, "skills"), "review"); + + const { meta, result } = await runExecutor( + { + agent: "claude", + agentCommand: "node ./fake-acp.js", + stateDir, + cwd: localCwd, + paperclipRuntimeSkills: [skill], + paperclipSkillSync: { desiredSkills: [skill.key] }, + }, + { authToken: "real-run-jwt", executionTarget, prepareRemoteManagedHome: stagingClaudeSeam() }, + ); + + const hostBundleDir = await onlyChildDir(path.join(stateDir, "runtime-skills", "claude")); + const hostSkillsHome = path.join(hostBundleDir, ".claude", "skills"); + + const prompt = String(meta[0]?.prompt ?? ""); + expect(prompt).toMatch(/Skill root: (\S+)/); + expect(prompt).not.toContain(hostSkillsHome); + + const inSandboxSkillsRoot = prompt.match(/Skill root: (\S+)/)![1]!; + expect(inSandboxSkillsRoot).not.toBe(hostSkillsHome); + await expect( + fs.readFile(path.join(inSandboxSkillsRoot, skill.runtimeName, "SKILL.md"), "utf8"), + ).resolves.toContain("# review"); + + const skillsIdentity = result.sessionParams?.skills as { skillRoot?: string } | undefined; + expect(skillsIdentity?.skillRoot).toBe(inSandboxSkillsRoot); + }); + + it("keeps the session fingerprint stable across two different in-sandbox skill roots", async () => { + // Same session (same execution target, same config) both times, so the + // fingerprint's other 16 fields cannot explain a difference — only the + // seam's reported in-sandbox skill path varies, by direct override. + const { stateDir, localCwd, executionTarget } = await setupRemoteSandbox(); + const skill = await createSkill(path.join(localCwd, "skills"), "review"); + const baseConfig = { + agent: "claude", + agentCommand: "node ./fake-acp.js", + stateDir, + cwd: localCwd, + paperclipRuntimeSkills: [skill], + paperclipSkillSync: { desiredSkills: [skill.key] }, + }; + const seamWithOverriddenSkillsDir = ( + overridePath: string, + ): AcpxEngineExecutorOptions["prepareRemoteManagedHome"] => + async (input) => { + const stagedRuntime = await input.stage( + input.skillsBundleDir + ? [{ key: "skills", localDir: input.skillsBundleDir, followSymlinks: false }] + : [], + ); + stagedRuntime.assetDirs.skills = overridePath; + return { stagedRuntime }; + }; + + const runA = await runExecutor(baseConfig, { + authToken: "real-run-jwt", + executionTarget, + prepareRemoteManagedHome: seamWithOverriddenSkillsDir("/sandbox/path-a/skills"), + }); + const runB = await runExecutor(baseConfig, { + authToken: "real-run-jwt", + executionTarget, + prepareRemoteManagedHome: seamWithOverriddenSkillsDir("/sandbox/path-b/skills"), + }); + + expect(String(runA.meta[0]?.prompt ?? "")).toContain("Skill root: /sandbox/path-a/skills"); + expect(String(runB.meta[0]?.prompt ?? "")).toContain("Skill root: /sandbox/path-b/skills"); + // The session fingerprint, which only ever saw the host-independent skill + // identity, stays the same across the two different in-sandbox paths. + expect(runA.result.sessionParams?.configFingerprint).toBe(runB.result.sessionParams?.configFingerprint); + }); + + it("stages no skills asset and leaves the prompt untouched when no skill is selected", async () => { + const { stateDir, localCwd, executionTarget } = await setupRemoteSandbox(); + + const { meta } = await runExecutor( + { + agent: "claude", + agentCommand: "node ./fake-acp.js", + stateDir, + cwd: localCwd, + paperclipRuntimeSkills: [], + paperclipSkillSync: { desiredSkills: [] }, + }, + { authToken: "real-run-jwt", executionTarget, prepareRemoteManagedHome: stagingClaudeSeam() }, + ); + + const stageArgs = vi.mocked(prepareAdapterExecutionTargetRuntime).mock.calls[0]![0]; + expect((stageArgs.assets ?? []).some((asset) => asset.key === "skills")).toBe(false); + expect(String(meta[0]?.prompt ?? "")).not.toContain("Skill root:"); + }); + + it("drops a skill that fails to materialize from the prompt, the identity, and the staged bundle", async () => { + const { stateDir, localCwd, executionTarget } = await setupRemoteSandbox(); + const skillsRoot = path.join(localCwd, "skills"); + const review = await createSkill(skillsRoot, "review"); + + // A skill whose source is a symlink. `materializePaperclipSkillCopy` + // refuses a symlinked skill root, so this skill's copy fails while its + // source still exists (it does not hit the separate missing-source + // filter). + const linkedTarget = path.join(skillsRoot, "broken-target"); + await fs.mkdir(linkedTarget, { recursive: true }); + await fs.writeFile(path.join(linkedTarget, "SKILL.md"), "# broken\n", "utf8"); + const brokenSource = path.join(skillsRoot, "broken"); + await fs.symlink(linkedTarget, brokenSource, "dir"); + const broken = { + key: "paperclipai/test/broken", + runtimeName: "broken", + source: brokenSource, + required: false, + }; + + const { meta, result } = await runExecutor( + { + agent: "claude", + agentCommand: "node ./fake-acp.js", + stateDir, + cwd: localCwd, + paperclipRuntimeSkills: [review, broken], + paperclipSkillSync: { desiredSkills: [review.key, broken.key] }, + }, + { authToken: "real-run-jwt", executionTarget, prepareRemoteManagedHome: stagingClaudeSeam() }, + ); + + const prompt = String(meta[0]?.prompt ?? ""); + expect(prompt).toContain("Selected skills: review"); + expect(prompt).not.toContain("broken"); + + const skillsIdentity = result.sessionParams?.skills as { selectedSkills?: string[] } | undefined; + expect(skillsIdentity?.selectedSkills).toEqual(["review"]); + + const hostBundleDir = await onlyChildDir(path.join(stateDir, "runtime-skills", "claude")); + const hostSkillsHome = path.join(hostBundleDir, ".claude", "skills"); + await expect(pathExists(path.join(hostSkillsHome, "broken"))).resolves.toBe(false); + }); + + it("stages no skills asset when every selected skill fails to materialize", async () => { + const { stateDir, localCwd, executionTarget } = await setupRemoteSandbox(); + const skillsRoot = path.join(localCwd, "skills"); + const linkedTarget = path.join(skillsRoot, "broken-target"); + await fs.mkdir(linkedTarget, { recursive: true }); + await fs.writeFile(path.join(linkedTarget, "SKILL.md"), "# broken\n", "utf8"); + const brokenSource = path.join(skillsRoot, "broken"); + await fs.symlink(linkedTarget, brokenSource, "dir"); + const broken = { + key: "paperclipai/test/broken", + runtimeName: "broken", + source: brokenSource, + required: false, + }; + + const { meta } = await runExecutor( + { + agent: "claude", + agentCommand: "node ./fake-acp.js", + stateDir, + cwd: localCwd, + paperclipRuntimeSkills: [broken], + paperclipSkillSync: { desiredSkills: [broken.key] }, + }, + { authToken: "real-run-jwt", executionTarget, prepareRemoteManagedHome: stagingClaudeSeam() }, + ); + + const stageArgs = vi.mocked(prepareAdapterExecutionTargetRuntime).mock.calls[0]![0]; + expect((stageArgs.assets ?? []).some((asset) => asset.key === "skills")).toBe(false); + expect(String(meta[0]?.prompt ?? "")).not.toContain("Skill root:"); + }); + + it("drops a skill whose staged copy has no usable SKILL.md from the prompt, the identity, and the staged bundle", async () => { + const { stateDir, localCwd, executionTarget } = await setupRemoteSandbox(); + const skillsRoot = path.join(localCwd, "skills"); + const review = await createSkill(skillsRoot, "review"); + + // A skill root that is a real directory (so the copy itself does not + // throw), but whose `SKILL.md` is a symlink. `materializePaperclipSkillCopy` + // skips a symlinked file entry instead of copying it, so the staged + // directory ends up with no `SKILL.md`. + const linkedSkillMdTarget = path.join(skillsRoot, "linked-skill-md-target.md"); + await fs.writeFile(linkedSkillMdTarget, "# linked\n", "utf8"); + const symlinkedSkillMdSource = path.join(skillsRoot, "symlinked-skill-md"); + await fs.mkdir(symlinkedSkillMdSource, { recursive: true }); + await fs.symlink(linkedSkillMdTarget, path.join(symlinkedSkillMdSource, "SKILL.md"), "file"); + const symlinkedSkillMd = { + key: "paperclipai/test/symlinked-skill-md", + runtimeName: "symlinked-skill-md", + source: symlinkedSkillMdSource, + required: false, + }; + + // A skill root that is a real directory with no `SKILL.md` at all. + const noSkillMdSource = path.join(skillsRoot, "no-skill-md"); + await fs.mkdir(noSkillMdSource, { recursive: true }); + await fs.writeFile(path.join(noSkillMdSource, "notes.md"), "# notes\n", "utf8"); + const noSkillMd = { + key: "paperclipai/test/no-skill-md", + runtimeName: "no-skill-md", + source: noSkillMdSource, + required: false, + }; + + const { meta, result } = await runExecutor( + { + agent: "claude", + agentCommand: "node ./fake-acp.js", + stateDir, + cwd: localCwd, + paperclipRuntimeSkills: [review, symlinkedSkillMd, noSkillMd], + paperclipSkillSync: { desiredSkills: [review.key, symlinkedSkillMd.key, noSkillMd.key] }, + }, + { authToken: "real-run-jwt", executionTarget, prepareRemoteManagedHome: stagingClaudeSeam() }, + ); + + const prompt = String(meta[0]?.prompt ?? ""); + expect(prompt).toContain("Selected skills: review"); + expect(prompt).not.toContain("symlinked-skill-md"); + expect(prompt).not.toContain("no-skill-md"); + + const skillsIdentity = result.sessionParams?.skills as { selectedSkills?: string[] } | undefined; + expect(skillsIdentity?.selectedSkills).toEqual(["review"]); + + const hostBundleDir = await onlyChildDir(path.join(stateDir, "runtime-skills", "claude")); + const hostSkillsHome = path.join(hostBundleDir, ".claude", "skills"); + await expect(pathExists(path.join(hostSkillsHome, "symlinked-skill-md"))).resolves.toBe(false); + await expect(pathExists(path.join(hostSkillsHome, "no-skill-md"))).resolves.toBe(false); + }); +}); + describe("ACPX engine remote session-lifecycle re-staging (PR 3: stage once / reuse on compatible resume)", () => { beforeEach(() => { vi.clearAllMocks(); diff --git a/packages/adapter-utils/src/acpx-engine/execute.ts b/packages/adapter-utils/src/acpx-engine/execute.ts index 3f537fedd1..e7168640dd 100644 --- a/packages/adapter-utils/src/acpx-engine/execute.ts +++ b/packages/adapter-utils/src/acpx-engine/execute.ts @@ -300,6 +300,19 @@ export interface AcpxRemoteManagedHomeContext { env: Record; onLog: AdapterExecutionContext["onLog"]; onRuntimeProgress: AdapterExecutionContext["onRuntimeProgress"]; + /** + * The host directory that holds this run's skill bundle. This field is + * `null` when the agent does not support on-demand skills, when no skill + * is selected, and when every selected skill fails to materialize. + * + * A seam stages the bundle as an asset. When the bundle holds an owned + * copy with no symbolic link, the seam sets `followSymlinks: false` (see + * `claude-local/src/server/acp.ts` for a worked example). The + * `followSymlinks: true` value makes the archive step carry a symbolic + * link's target content instead of a dangling link. A seam needs that + * value only when its own bundle holds symbolic links. + */ + skillsBundleDir: string | null; /** * Runs the shared workspace+assets staging seam and returns the prepared * runtime. The seam passes its per-adapter home `assets` here; the returned @@ -1077,6 +1090,15 @@ async function prepareClaudeSkillRuntime(input: { identity: Record; promptInstructions: string; commandNotes: string[]; + /** + * The host directory that directly holds the materialized skill + * directories (`//SKILL.md`). This field is `null` + * when no skill is selected, or when every selected skill failed to + * materialize. A remote run stages this directory into the sandbox and + * rewrites the prompt onto the in-sandbox copy. See + * `AcpxRemoteManagedHomeContext.skillsBundleDir`. + */ + bundleDir: string | null; }> { const { allSkills, selectedSkills, desiredSkillNames } = await resolveSelectedRuntimeSkills(input.config, input.moduleDir); const skillSetKey = await buildSkillSetKey({ skills: selectedSkills, label: "claude" }); @@ -1084,10 +1106,28 @@ async function prepareClaudeSkillRuntime(input: { const skillsHome = path.join(bundleRoot, ".claude", "skills"); await fs.mkdir(skillsHome, { recursive: true }); + // A failed materialization, or a materialized copy with no usable + // `SKILL.md`, must drop the skill from every advertised list below. + // Otherwise the prompt and the session identity still name a skill whose + // `SKILL.md` is not in `skillsHome` — either the whole copy failed, or the + // copy skipped a symlinked `SKILL.md` — and the agent's read of that file + // fails with a missing-file error, the same symptom this bundle exists to + // fix. + const materializedNames: string[] = []; for (const entry of selectedSkills) { const target = path.join(skillsHome, entry.runtimeName); try { const result = await materializePaperclipSkillCopy(entry.source, target); + const skillMdStat = await fs.stat(path.join(target, "SKILL.md")).catch(() => null); + if (!skillMdStat?.isFile()) { + await fs.rm(target, { recursive: true, force: true }); + await input.onLog( + "stderr", + `[paperclip] Skipped ACPX Claude skill "${entry.key}": the staged copy at ${target} has no usable SKILL.md.\n`, + ); + continue; + } + materializedNames.push(entry.runtimeName); if (result.skippedSymlinks.length > 0) { await input.onLog( "stdout", @@ -1102,14 +1142,14 @@ async function prepareClaudeSkillRuntime(input: { } } - const selectedNames = selectedSkills.map((entry) => entry.runtimeName).sort(); - const promptInstructions = selectedSkills.length > 0 + const selectedNames = materializedNames.sort(); + const promptInstructions = selectedNames.length > 0 ? [ "Paperclip has materialized selected runtime skills for this ACPX Claude session.", `Skill root: ${skillsHome}`, - selectedNames.length > 0 ? `Selected skills: ${selectedNames.join(", ")}` : "", + `Selected skills: ${selectedNames.join(", ")}`, "When a task calls for one of these skills, read its SKILL.md from that root and follow it.", - ].filter(Boolean).join("\n") + ].join("\n") : ""; return { @@ -1118,12 +1158,13 @@ async function prepareClaudeSkillRuntime(input: { skillSetKey, desiredSkillNames, selectedSkills: selectedNames, - skillRoot: selectedSkills.length > 0 ? skillsHome : null, + skillRoot: selectedNames.length > 0 ? skillsHome : null, }, promptInstructions, - commandNotes: selectedSkills.length > 0 - ? [`Materialized ${selectedSkills.length} Paperclip skill(s) for ACPX Claude at ${skillsHome}.`] + commandNotes: selectedNames.length > 0 + ? [`Materialized ${selectedNames.length} Paperclip skill(s) for ACPX Claude at ${skillsHome}.`] : [], + bundleDir: selectedNames.length > 0 ? skillsHome : null, }; } @@ -1955,6 +1996,12 @@ async function buildRuntime(input: { let skillPromptInstructions = ""; let skillsIdentity: Record = { mode: "unsupported" }; const skillCommandNotes: string[] = []; + // The host directory a remote run stages as the `skills` asset. The engine + // uses it to rewrite `skillPromptInstructions` and `skillsIdentity` onto + // the in-sandbox copy, once `stagedRuntime` is known (see the rewrite + // below, after `placeWorkspace` returns). This field is `null` for every + // non-Claude agent, and for a Claude run with no skill selected. + let claudeSkillsBundleDir: string | null = null; let paperclipClaudeSettings: PaperclipClaudeSettingsResult | null = null; if (acpxAgent === "claude") { const preparedSkills = await prepareClaudeSkillRuntime({ @@ -1966,6 +2013,7 @@ async function buildRuntime(input: { skillPromptInstructions = preparedSkills.promptInstructions; skillsIdentity = preparedSkills.identity; skillCommandNotes.push(...preparedSkills.commandNotes); + claudeSkillsBundleDir = preparedSkills.bundleDir; paperclipClaudeSettings = await writePaperclipClaudeSettings({ cwd, stateDir, @@ -2231,6 +2279,7 @@ async function buildRuntime(input: { env, onLog: input.ctx.onLog, onRuntimeProgress: input.ctx.onRuntimeProgress, + skillsBundleDir: claudeSkillsBundleDir, stage, }); return { @@ -2327,6 +2376,36 @@ async function buildRuntime(input: { remoteStagingEnvDelta = placedStaged?.envDelta ?? null; sessionStagingLeaseRelease = sandboxSite.stagingLeaseRelease; } + // Once the skill bundle is staged, rewrite the prompt and the identity onto + // the in-sandbox copy. This code runs here, after `placeWorkspace` resolves + // `stagedRuntime`. It runs on every invocation, both a fresh stage and a + // compatible resume. It never runs inside the `prepareRemoteManagedHome` + // seam, because a compatible resume never calls that seam again. + // `skillPromptInstructions` and `skillsIdentity` already fed `fingerprint` + // above, with the host-independent identity. So this rewrite never reaches + // the fingerprint: it only replaces the host bundle path with the + // in-sandbox path, in the local copies used for the returned prompt, + // identity, and command notes. + if (acpxAgent === "claude" && stagedRuntime && claudeSkillsBundleDir) { + const inSandboxSkillsRoot = + stagedRuntime.assetDirs.skills ?? + path.posix.join( + stagedRuntime.runtimeRootDir ?? + path.posix.join(stagedRuntime.workspaceRemoteDir ?? cwd, ".paperclip-runtime", acpxAgent), + "skills", + ); + const rebaseToSandbox = (value: string) => value.split(claudeSkillsBundleDir!).join(inSandboxSkillsRoot); + skillPromptInstructions = rebaseToSandbox(skillPromptInstructions); + skillsIdentity = { + ...skillsIdentity, + skillRoot: typeof skillsIdentity.skillRoot === "string" + ? rebaseToSandbox(skillsIdentity.skillRoot) + : skillsIdentity.skillRoot, + }; + for (let i = 0; i < skillCommandNotes.length; i += 1) { + skillCommandNotes[i] = rebaseToSandbox(skillCommandNotes[i]!); + } + } // Both bridge starts run under one try so a failure at EITHER — including the // paperclip callback bridge — fires the same abandon-path cleanup. The // paperclip bridge starts after the workspace + managed home were already diff --git a/packages/adapter-utils/src/skills-staging-follow-symlinks.test.ts b/packages/adapter-utils/src/skills-staging-follow-symlinks.test.ts new file mode 100644 index 0000000000..fe1a1d64a5 --- /dev/null +++ b/packages/adapter-utils/src/skills-staging-follow-symlinks.test.ts @@ -0,0 +1,145 @@ +import fs from "node:fs/promises"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +const __moduleDir = path.dirname(fileURLToPath(import.meta.url)); +const repoAdaptersDir = path.resolve(__moduleDir, "../../adapters"); + +// One flat object literal, such as an asset entry in an `assets: [...]` +// array. `[^{}]*` stops at the object's own closing brace, so a match never +// bleeds into a sibling asset (e.g. `mcp-config`) later in the same array. +const OBJECT_LITERAL = /\{[^{}]*\}/g; +const KEY_IS_SKILLS = /key:\s*"skills"/; +// Property order inside the object is not significant: `key: "skills"` can +// come before or after `followSymlinks`, so each is matched on its own +// against the whole block instead of one property chained after the other. +const FOLLOW_SYMLINKS_VALUE = /followSymlinks:\s*(true|false)/; + +/** + * The `followSymlinks` value of every `key: "skills"` object literal in a + * source string, regardless of the order its properties appear in. + */ +function parseSkillsAssetFollowSymlinksValues(source: string): boolean[] { + const followSymlinksValues: boolean[] = []; + for (const [block] of source.matchAll(OBJECT_LITERAL)) { + if (!KEY_IS_SKILLS.test(block)) continue; + const followSymlinksMatch = FOLLOW_SYMLINKS_VALUE.exec(block); + if (!followSymlinksMatch) continue; + followSymlinksValues.push(followSymlinksMatch[1] === "true"); + } + return followSymlinksValues; +} + +/** + * Every real occurrence of a `key: "skills"` staging asset under + * `packages/adapters`, keyed by its repo-relative path. This walks the real + * adapter source tree, so it catches a new staging site as soon as it lands + * — a test that only checks a hand-picked list, or a fabricated string, + * cannot. It also catches a site whose `key` and `followSymlinks` + * properties appear in either order. + */ +async function findSkillsStagingSites(): Promise> { + const sites = new Map(); + + async function walk(dir: string): Promise { + const entries = await fs.readdir(dir, { withFileTypes: true }); + for (const entry of entries) { + if (entry.name === "node_modules" || entry.name === "dist") continue; + const fullPath = path.join(dir, entry.name); + if (entry.isDirectory()) { + await walk(fullPath); + continue; + } + if (!entry.isFile()) continue; + if (!fullPath.endsWith(".ts") || fullPath.endsWith(".test.ts")) continue; + + const source = await fs.readFile(fullPath, "utf8"); + const followSymlinksValues = parseSkillsAssetFollowSymlinksValues(source); + if (followSymlinksValues.length === 0) continue; + + const relativePath = path + .relative(repoAdaptersDir, fullPath) + .split(path.sep) + .join("/"); + sites.set(relativePath, followSymlinksValues); + } + } + + await walk(repoAdaptersDir); + return sites; +} + +// The known `key: "skills"` staging sites and their reviewed +// `followSymlinks` setting, as of this test's authorship. Each site's value +// is a deliberate choice: the remote Claude ACP lane stages an +// already-materialized, owned copy of the skill bundle (never a symlink), +// so it must set `followSymlinks: false` — following a link there turns +// into tar's `-h` flag (`sandbox-managed-runtime.ts`) and dereferences a +// symlink planted in the bundle directory after it was built. Every other +// listed site still stages the user's live skills directory directly and +// relies on `followSymlinks: true` to resolve that directory's own +// symlinks. A change to any of these values, or a new unlisted site, must +// fail this test and force a deliberate review. +const EXPECTED_SKILLS_STAGING_SITES: Record = { + "claude-local/src/server/acp.ts": false, + "claude-local/src/server/execute.ts": true, + "cursor-local/src/server/execute.ts": true, + "gemini-local/src/server/acp.ts": true, + "gemini-local/src/server/execute.ts": true, + "kimi-local/src/server/execute.ts": true, + "opencode-local/src/server/execute.ts": true, + "pi-local/src/server/execute.ts": true, +}; + +describe("remote skills staging sites (real source scan)", () => { + it("matches the reviewed followSymlinks setting at every known site, and finds no unlisted site", async () => { + const discoveredSites = await findSkillsStagingSites(); + + for (const [relativePath, followSymlinksValues] of discoveredSites) { + expect( + relativePath in EXPECTED_SKILLS_STAGING_SITES, + `Found an unreviewed "skills" staging site at ${relativePath}. ` + + "Add it to EXPECTED_SKILLS_STAGING_SITES with a deliberate " + + "followSymlinks choice.", + ).toBe(true); + for (const value of followSymlinksValues) { + expect( + value, + `${relativePath} sets followSymlinks: ${value}, but the reviewed ` + + `value is ${EXPECTED_SKILLS_STAGING_SITES[relativePath]}.`, + ).toBe(EXPECTED_SKILLS_STAGING_SITES[relativePath]); + } + } + + for (const relativePath of Object.keys(EXPECTED_SKILLS_STAGING_SITES)) { + expect( + discoveredSites.has(relativePath), + `Expected a "skills" staging site at ${relativePath}; it is missing ` + + "or no longer sets followSymlinks in that object literal.", + ).toBe(true); + } + }); + + it("stages the remote Claude ACP skill bundle without following a symlink", async () => { + // The one site this change owns: the materialized skill bundle must + // never be staged with tar's `-h` flag. A regression here would let a + // symlink planted in the bundle directory after it was built escape + // into the sandbox. + const discoveredSites = await findSkillsStagingSites(); + const claudeAcpValues = discoveredSites.get( + "claude-local/src/server/acp.ts", + ); + expect(claudeAcpValues).toEqual([false]); + }); + + it("finds a skills asset's followSymlinks value in either property order", () => { + const keyFirst = `{ key: "skills", localDir: dir, followSymlinks: false }`; + const followSymlinksFirst = `{ followSymlinks: true, localDir: dir, key: "skills" }`; + + expect(parseSkillsAssetFollowSymlinksValues(keyFirst)).toEqual([false]); + expect(parseSkillsAssetFollowSymlinksValues(followSymlinksFirst)).toEqual([ + true, + ]); + }); +}); diff --git a/packages/adapters/claude-local/src/server/acp.test.ts b/packages/adapters/claude-local/src/server/acp.test.ts index 34ead8a952..a0441be380 100644 --- a/packages/adapters/claude-local/src/server/acp.test.ts +++ b/packages/adapters/claude-local/src/server/acp.test.ts @@ -1,9 +1,23 @@ 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"; + +// Wrap the shared staging seam in a call-recording spy that still delegates to +// the real implementation (a runner-backed sandbox test exercises it end to +// end against the local sandbox stand-in). This lets a test assert the exact +// `assets` the Claude remote managed-home seam sends it without changing any +// real behavior for the other tests. +vi.mock("@paperclipai/adapter-utils/execution-target", async (importActual) => { + const actual = await importActual(); + return { + ...actual, + prepareAdapterExecutionTargetRuntime: vi.fn(actual.prepareAdapterExecutionTargetRuntime), + }; +}); +import { prepareAdapterExecutionTargetRuntime } from "@paperclipai/adapter-utils/execution-target"; import { buildClaudeAcpConfig, createClaudeAcpExecutor, @@ -536,6 +550,193 @@ describe("claude_local ACP lane", () => { expect(settings.permissions.allow).toEqual(expect.arrayContaining(["Bash(curl:*)", "Bash(env)"])); }); + it("stages the skill bundle as a no-follow-symlinks asset for a remote ACP run, and points the prompt at the in-sandbox skill root", async () => { + vi.mocked(prepareAdapterExecutionTargetRuntime).mockClear(); + const root = await makeTempRoot("paperclip-claude-acp-skills-remote-"); + const skill = await createRuntimeSkill(root); + 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 }); + + const runtimes: FakeRuntime[] = []; + const execute = createClaudeAcpExecutor({ + createRuntime: (options: FakeRuntimeOptions) => { + const runtime = new FakeRuntime(options); + runtimes.push(runtime); + return runtime 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.", + paperclipRuntimeSkills: [skill], + paperclipSkillSync: { desiredSkills: [skill.key] }, + }, + context: { + issueId: "issue-1", + paperclipTaskMarkdown: "Task context", + 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", + }), + ); + + expect(result.exitCode).toBe(0); + + // The real seam sent the bundle to the shared staging call with + // `followSymlinks: false`: the bundle holds a plain copy of each skill's + // files, so staging never needs to carry a symbolic link's target + // content, and a link planted in the bundle after materialization must + // not cross into the sandbox. + const stageArgs = vi.mocked(prepareAdapterExecutionTargetRuntime).mock.calls[0]![0]; + const skillsAsset = stageArgs.assets?.find((asset) => asset.key === "skills"); + expect(skillsAsset).toMatchObject({ followSymlinks: false }); + + // The prompt names the in-sandbox skill root, not the host bundle dir... + const prompt = String(runtimes[0]?.startInputs[0]?.text ?? ""); + const skillRootMatch = prompt.match(/Skill root: (\S+)/); + expect(skillRootMatch).toBeTruthy(); + const inSandboxSkillRoot = skillRootMatch![1]!; + expect(inSandboxSkillRoot).not.toBe(skillsAsset!.localDir); + expect(prompt).not.toContain(String(skillsAsset!.localDir)); + // ...and it really landed there (local runner extracts to the asset dir). + await expect( + fs.readFile(path.join(inSandboxSkillRoot, "review", "SKILL.md"), "utf8"), + ).resolves.toContain("review skill"); + }); + + it("stages no skills asset for a remote ACP run with no selected skill", async () => { + vi.mocked(prepareAdapterExecutionTargetRuntime).mockClear(); + const root = await makeTempRoot("paperclip-claude-acp-skills-remote-empty-"); + // An available-but-undesired skill, so the run resolves a real (empty) + // selection instead of falling back to the package's own default skill + // set (that fallback only fires when `paperclipRuntimeSkills` is absent). + const skill = await createRuntimeSkill(root); + 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 }); + + const runtimes: FakeRuntime[] = []; + const execute = createClaudeAcpExecutor({ + createRuntime: (options: FakeRuntimeOptions) => { + const runtime = new FakeRuntime(options); + runtimes.push(runtime); + return runtime 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.", + paperclipRuntimeSkills: [skill], + paperclipSkillSync: { desiredSkills: [] }, + }, + 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", + }), + ); + + expect(result.exitCode).toBe(0); + const stageArgs = vi.mocked(prepareAdapterExecutionTargetRuntime).mock.calls[0]![0]; + expect((stageArgs.assets ?? []).some((asset) => asset.key === "skills")).toBe(false); + expect(String(runtimes[0]?.startInputs[0]?.text ?? "")).not.toContain("Skill root:"); + }); + + it("stages the skill bundle inside the sandbox but never syncs it back into the host workspace", async () => { + // The staged skill bundle lives under `.paperclip-runtime/claude/skills`, + // inside the same in-sandbox directory the workspace restore reads. The + // restore excludes the whole `.paperclip-runtime` tree + // (`sandbox-managed-runtime.ts`'s `restoreExclude` list) for every asset + // key alike, so this proves it for the new "skills" asset specifically. + const root = await makeTempRoot("paperclip-claude-acp-skills-no-syncback-"); + const skill = await createRuntimeSkill(root); + 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"); + + const runtimes: FakeRuntime[] = []; + const execute = createClaudeAcpExecutor({ + createRuntime: (options: FakeRuntimeOptions) => { + const runtime = new FakeRuntime(options); + runtimes.push(runtime); + return runtime 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.", + paperclipRuntimeSkills: [skill], + paperclipSkillSync: { desiredSkills: [skill.key] }, + }, + 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", + }), + ); + + expect(result.exitCode).toBe(0); + // Positive control: the bundle really did land in the sandbox stand-in + // during the run, under the in-sandbox skill root the prompt names. + const prompt = String(runtimes[0]?.startInputs[0]?.text ?? ""); + const inSandboxSkillRoot = prompt.match(/Skill root: (\S+)/)![1]!; + expect(inSandboxSkillRoot).toContain(path.join(remoteCwd, ".paperclip-runtime")); + await expect( + fs.readFile(path.join(inSandboxSkillRoot, "review", "SKILL.md"), "utf8"), + ).resolves.toContain("review skill"); + // After the run's workspace restore, the host worktree carries the file the + // run wrote inside the workspace proper... + await expect(fs.readFile(path.join(localCwd, "hello.txt"), "utf8")).resolves.toBe("hi"); + // ...but not the staged runtime directory the skill bundle staged into. + await expect(fs.access(path.join(localCwd, ".paperclip-runtime"))).rejects.toThrow(); + }); + it("passes the exact configured Fable 5.1 ID through ANTHROPIC_MODEL on the ACP lane", async () => { const root = await makeTempRoot("paperclip-claude-acp-fable51-"); const meta: AdapterInvocationMeta[] = []; diff --git a/packages/adapters/claude-local/src/server/acp.ts b/packages/adapters/claude-local/src/server/acp.ts index b9f8724215..951a9419e7 100644 --- a/packages/adapters/claude-local/src/server/acp.ts +++ b/packages/adapters/claude-local/src/server/acp.ts @@ -268,8 +268,21 @@ async function prepareClaudeRemoteManagedHome( // Content-addressed sanitized seed (managed cache under the instance root, not // a temp dir — reused across runs, so no teardown cleanup). const claudeConfigSeedDir = await prepareClaudeConfigSeed(process.env, onLog, input.companyId); + // Ship the per-run skill bundle, staged only when the run selected at + // least one skill. The bundle directory holds a plain copy of each + // selected skill's files (`materializePaperclipSkillCopy` never copies a + // symbolic link, at the root or at any depth). So the bundle asset stages + // with `followSymlinks: false`: staging never needs to carry a symbolic + // link's target content, and refusing to follow one stops a link planted + // in the bundle directory after materialization (for example by a + // concurrent writer) from pulling an arbitrary host file into the sandbox. + // The engine rewrites the prompt onto the in-sandbox copy once this asset + // is staged. const stagedRuntime = await input.stage([ { key: "config-seed", localDir: claudeConfigSeedDir, followSymlinks: true }, + ...(input.skillsBundleDir + ? [{ key: "skills", localDir: input.skillsBundleDir, followSymlinks: false }] + : []), ]); const remoteClaudeRuntimeRoot =