mirror of
https://github.com/paperclipai/paperclip
synced 2026-04-25 17:25:15 +02:00
fix(pi-local): prepend installed skill bin/ dirs to child PATH (#4331)
## Thinking Path
> - Paperclip orchestrates AI agents; each agent runs under an adapter
that spawns a model CLI as a child process.
> - The pi-local adapter (`packages/adapters/pi-local`) spawns `pi` and
inherits the child's shell environment — including `PATH`, which
determines what the child's bash tool can execute by name.
> - Paperclip skills ship executable helpers under `<skill>/bin/` (e.g.
`paperclip-get-issue`) and Reviewer/QA-style `AGENTS.md` files invoke
them by name via the agent's bash tool.
> - Pi-local builds its runtime env with `ensurePathInEnv({
...process.env, ...env })` only — it never adds the installed skills'
`bin/` dirs to PATH. The pi CLI's `--skill` arg loads each skill's
SKILL.md but does not augment PATH.
> - Consequence: every bash invocation of a skill helper fails with
`exit 127: command not found`. The agent then spends its heartbeat
guessing (re-reading SKILL.md, trying `find`, inventing command paths)
and either times out or gives up.
> - This PR prepends each injected skill's `bin/` directory to the child
PATH immediately before runtimeEnv is constructed.
> - The benefit: pi_local agents whose AGENTS.md uses any `paperclip-*`
skill helper can actually run those helpers.
## What Changed
- `packages/adapters/pi-local/src/server/execute.ts`: compute
`skillBinDirs` from the already-resolved `piSkillEntries`, dedupe
against the existing PATH, prepend them to whichever of `PATH` / `Path`
the merged env uses, then build `runtimeEnv`. No new helpers, no
adapter-utils changes.
## Verification
Manual repro before the fix:
1. Create a pi_local agent wired to a paperclip skill (e.g.
paperclip-control).
2. Wake the agent on an in_review issue with an AGENTS.md that starts
with `paperclip-get-issue "$PAPERCLIP_TASK_ID"`.
3. Session file: `{ "role": "toolResult", "isError": true, "content": [{
"text": "/bin/bash: paperclip-get-issue: command not found\n\nCommand
exited with code 127" }] }`.
After the fix: same wake; `paperclip-get-issue` resolves and returns the
issue JSON; agent proceeds.
Local commands:
```
pnpm --filter @paperclipai/adapter-pi-local typecheck # clean
pnpm --filter @paperclipai/adapter-pi-local build # clean
pnpm --filter @paperclipai/server exec vitest run \
src/__tests__/pi-local-execute.test.ts \
src/__tests__/pi-local-adapter-environment.test.ts \
src/__tests__/pi-local-skill-sync.test.ts
# 5/5 passing
```
No new tests: the existing `pi-local-skill-sync.test.ts` covers skill
symlink injection (upstream of the PATH step), and
`pi-local-execute.test.ts` covers the spawn path; this change only
augments env on the same spawn path.
## Risks
Low. Pure PATH augmentation on the child env. Edge cases:
- Zero skills installed → no PATH change (guarded by
`skillBinDirs.length > 0`).
- Duplicate bin dirs already on PATH → deduped; no pollution on re-runs.
- Windows `Path` casing → falls back correctly when merged env uses
`Path` instead of `PATH`.
- Skill dir without `bin/` subdir → joined path simply won't resolve;
harmless.
No behavioral change for pi_local agents that don't use skill-provided
commands.
## Model Used
- Claude, `claude-opus-4-7` (1M context), extended thinking enabled,
tool use enabled. Walked pi-local/cursor-local/claude-local and
adapter-utils to isolate the gap, wrote the inlined fix, and ran
typecheck/build/test locally.
## Checklist
- [x] Thinking path from project context to this change
- [x] Model used specified
- [x] Checked ROADMAP.md — no overlap
- [x] Tests run locally, passing
- [x] Tests added — new case in
`server/src/__tests__/pi-local-execute.test.ts`; verified it fails when
the fix is reverted
- [ ] UI screenshots — N/A (backend adapter change)
- [x] Docs updated — N/A (internal adapter, no user-facing docs)
- [x] Risks documented
- [x] Will address reviewer comments before merge
This commit is contained in:
@@ -204,8 +204,31 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
||||
env.PAPERCLIP_API_KEY = authToken;
|
||||
}
|
||||
|
||||
// Prepend installed skill `bin/` dirs to PATH so an agent's bash tool can
|
||||
// invoke skill binaries (e.g. `paperclip-get-issue`) by name. Without this,
|
||||
// any pi_local agent whose AGENTS.md calls a skill command via bash hits
|
||||
// exit 127 "command not found". Only include skills that ensurePiSkillsInjected
|
||||
// actually linked — otherwise non-injected skills' binaries would be reachable
|
||||
// to the agent.
|
||||
const injectedSkillKeys = new Set(desiredPiSkillNames);
|
||||
const skillBinDirs = piSkillEntries
|
||||
.filter((entry) => injectedSkillKeys.has(entry.key) && entry.source.length > 0)
|
||||
.map((entry) => path.join(entry.source, "bin"));
|
||||
const mergedEnv = ensurePathInEnv({ ...process.env, ...env });
|
||||
const pathKey =
|
||||
typeof mergedEnv.Path === "string" && mergedEnv.Path.length > 0 && !mergedEnv.PATH
|
||||
? "Path"
|
||||
: "PATH";
|
||||
const basePath = mergedEnv[pathKey] ?? "";
|
||||
if (skillBinDirs.length > 0) {
|
||||
const existing = basePath.split(path.delimiter).filter(Boolean);
|
||||
const additions = skillBinDirs.filter((dir) => !existing.includes(dir));
|
||||
if (additions.length > 0) {
|
||||
mergedEnv[pathKey] = [...additions, basePath].filter(Boolean).join(path.delimiter);
|
||||
}
|
||||
}
|
||||
const runtimeEnv = Object.fromEntries(
|
||||
Object.entries(ensurePathInEnv({ ...process.env, ...env })).filter(
|
||||
Object.entries(mergedEnv).filter(
|
||||
(entry): entry is [string, string] => typeof entry[1] === "string",
|
||||
),
|
||||
);
|
||||
|
||||
@@ -27,6 +27,25 @@ process.exit(0);
|
||||
await fs.chmod(commandPath, 0o755);
|
||||
}
|
||||
|
||||
async function writeEnvDumpPiCommand(commandPath: string, envDumpPath: string): Promise<void> {
|
||||
const script = `#!/usr/bin/env node
|
||||
const fs = require("node:fs");
|
||||
if (process.argv.includes("--list-models")) {
|
||||
console.log("provider model");
|
||||
console.log("google gemini-3-flash-preview");
|
||||
process.exit(0);
|
||||
}
|
||||
fs.writeFileSync(${JSON.stringify(envDumpPath)}, process.env.PATH || "");
|
||||
console.log(JSON.stringify({ type: "agent_start" }));
|
||||
console.log(JSON.stringify({ type: "turn_start" }));
|
||||
console.log(JSON.stringify({ type: "turn_end", message: { role: "assistant", content: "" }, toolResults: [] }));
|
||||
console.log(JSON.stringify({ type: "agent_end", messages: [] }));
|
||||
process.exit(0);
|
||||
`;
|
||||
await fs.writeFile(commandPath, script, "utf8");
|
||||
await fs.chmod(commandPath, 0o755);
|
||||
}
|
||||
|
||||
describe("pi_local execute", () => {
|
||||
it("fails the run when Pi exhausts automatic retries despite exiting 0", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-pi-execute-"));
|
||||
@@ -73,4 +92,116 @@ describe("pi_local execute", () => {
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("prepends installed skill bin/ dirs to the spawned Pi child PATH", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-pi-path-"));
|
||||
const workspace = path.join(root, "workspace");
|
||||
const commandPath = path.join(root, "pi");
|
||||
const skillDir = path.join(root, "skills", "demo-skill");
|
||||
const skillBinDir = path.join(skillDir, "bin");
|
||||
const envDumpPath = path.join(root, "captured-path.txt");
|
||||
await fs.mkdir(workspace, { recursive: true });
|
||||
await fs.mkdir(skillBinDir, { recursive: true });
|
||||
await fs.writeFile(path.join(skillDir, "SKILL.md"), "# demo-skill\n", "utf8");
|
||||
await writeEnvDumpPiCommand(commandPath, envDumpPath);
|
||||
|
||||
const previousHome = process.env.HOME;
|
||||
process.env.HOME = root;
|
||||
|
||||
try {
|
||||
await execute({
|
||||
runId: "run-pi-skill-path",
|
||||
agent: {
|
||||
id: "agent-skill-path",
|
||||
companyId: "company-skill-path",
|
||||
name: "Pi Agent",
|
||||
adapterType: "pi_local",
|
||||
adapterConfig: {},
|
||||
},
|
||||
runtime: {
|
||||
sessionId: null,
|
||||
sessionParams: null,
|
||||
sessionDisplayId: null,
|
||||
taskKey: null,
|
||||
},
|
||||
config: {
|
||||
command: commandPath,
|
||||
cwd: workspace,
|
||||
model: "google/gemini-3-flash-preview",
|
||||
promptTemplate: "Keep working.",
|
||||
paperclipRuntimeSkills: [
|
||||
{ key: "demo-skill", runtimeName: "demo-skill", source: skillDir, required: true },
|
||||
],
|
||||
},
|
||||
context: {},
|
||||
authToken: "run-jwt-token",
|
||||
onLog: async () => {},
|
||||
});
|
||||
|
||||
const capturedPath = await fs.readFile(envDumpPath, "utf8");
|
||||
const entries = capturedPath.split(path.delimiter);
|
||||
expect(entries[0]).toBe(skillBinDir);
|
||||
expect(entries.filter((entry) => entry === skillBinDir)).toHaveLength(1);
|
||||
} finally {
|
||||
if (previousHome === undefined) delete process.env.HOME;
|
||||
else process.env.HOME = previousHome;
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("does not expose bin/ dirs from skills that are not injected", async () => {
|
||||
const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-pi-path-neg-"));
|
||||
const workspace = path.join(root, "workspace");
|
||||
const commandPath = path.join(root, "pi");
|
||||
const nonInjectedSkillDir = path.join(root, "skills", "not-injected");
|
||||
const nonInjectedBinDir = path.join(nonInjectedSkillDir, "bin");
|
||||
const envDumpPath = path.join(root, "captured-path.txt");
|
||||
await fs.mkdir(workspace, { recursive: true });
|
||||
await fs.mkdir(nonInjectedBinDir, { recursive: true });
|
||||
await fs.writeFile(path.join(nonInjectedSkillDir, "SKILL.md"), "# not-injected\n", "utf8");
|
||||
await writeEnvDumpPiCommand(commandPath, envDumpPath);
|
||||
|
||||
const previousHome = process.env.HOME;
|
||||
process.env.HOME = root;
|
||||
|
||||
try {
|
||||
await execute({
|
||||
runId: "run-pi-skill-path-neg",
|
||||
agent: {
|
||||
id: "agent-skill-path-neg",
|
||||
companyId: "company-skill-path-neg",
|
||||
name: "Pi Agent",
|
||||
adapterType: "pi_local",
|
||||
adapterConfig: {},
|
||||
},
|
||||
runtime: {
|
||||
sessionId: null,
|
||||
sessionParams: null,
|
||||
sessionDisplayId: null,
|
||||
taskKey: null,
|
||||
},
|
||||
config: {
|
||||
command: commandPath,
|
||||
cwd: workspace,
|
||||
model: "google/gemini-3-flash-preview",
|
||||
promptTemplate: "Keep working.",
|
||||
// required:false with no explicit paperclipSkillSync preference →
|
||||
// resolvePaperclipDesiredSkillNames returns [] → skill is not injected.
|
||||
paperclipRuntimeSkills: [
|
||||
{ key: "not-injected", runtimeName: "not-injected", source: nonInjectedSkillDir, required: false },
|
||||
],
|
||||
},
|
||||
context: {},
|
||||
authToken: "run-jwt-token",
|
||||
onLog: async () => {},
|
||||
});
|
||||
|
||||
const capturedPath = await fs.readFile(envDumpPath, "utf8");
|
||||
expect(capturedPath.split(path.delimiter)).not.toContain(nonInjectedBinDir);
|
||||
} finally {
|
||||
if (previousHome === undefined) delete process.env.HOME;
|
||||
else process.env.HOME = previousHome;
|
||||
await fs.rm(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user