Merge pull request #2949 from Lempkey/fix/skip-system-prompt-on-resume

fix: skip --append-system-prompt-file on resumed claude sessions
2026-04-08 07:06:58 -05:00
parent 667d5a7384 0ff262ca0f
commit 8367c5f406
2 changed files with 348 additions and 29 deletions
@@ -335,12 +335,6 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
  const dangerouslySkipPermissions = asBoolean(config.dangerouslySkipPermissions, true);
  const instructionsFilePath = asString(config.instructionsFilePath, "").trim();
  const instructionsFileDir = instructionsFilePath ? `${path.dirname(instructionsFilePath)}/` : "";
-  const commandNotes = instructionsFilePath
-    ? [
-        `Injected agent instructions via --append-system-prompt-file ${instructionsFilePath} (with path directive appended)`,
-      ]
-    : [];
-
  const runtimeConfig = await buildClaudeRuntimeConfig({
    runId,
    agent,
@@ -369,11 +363,28 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
  const billingType = resolveClaudeBillingType(effectiveEnv);
  const skillsDir = await buildSkillsDir(config);

-  // When instructionsFilePath is configured, create a combined temp file that
-  // includes both the file content and the path directive, so we only need
-  // --append-system-prompt-file (Claude CLI forbids using both flags together).
-  let effectiveInstructionsFilePath: string | undefined = instructionsFilePath;
-  if (instructionsFilePath) {
+  const runtimeSessionParams = parseObject(runtime.sessionParams);
+  const runtimeSessionId = asString(runtimeSessionParams.sessionId, runtime.sessionId ?? "");
+  const runtimeSessionCwd = asString(runtimeSessionParams.cwd, "");
+  const canResumeSession =
+    runtimeSessionId.length > 0 &&
+    (runtimeSessionCwd.length === 0 || path.resolve(runtimeSessionCwd) === path.resolve(cwd));
+  const sessionId = canResumeSession ? runtimeSessionId : null;
+  if (runtimeSessionId && !canResumeSession) {
+    await onLog(
+      "stdout",
+      `[paperclip] Claude session "${runtimeSessionId}" was saved for cwd "${runtimeSessionCwd}" and will not be resumed in "${cwd}".\n`,
+    );
+  }
+
+  let effectiveInstructionsFilePath: string | undefined;
+  let preparedInstructionsFile = false;
+
+  const ensureEffectiveInstructionsFilePath = async (resumeSessionId: string | null) => {
+    if (resumeSessionId || !instructionsFilePath) return undefined;
+    if (preparedInstructionsFile) return effectiveInstructionsFilePath;
+
+    preparedInstructionsFile = true;
    try {
      const instructionsContent = await fs.readFile(instructionsFilePath, "utf-8");
      const pathDirective = `\nThe above agent instructions were loaded from ${instructionsFilePath}. Resolve any relative file references from ${instructionsFileDir}.`;
@@ -388,21 +399,9 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
      );
      effectiveInstructionsFilePath = undefined;
    }
-  }

-  const runtimeSessionParams = parseObject(runtime.sessionParams);
-  const runtimeSessionId = asString(runtimeSessionParams.sessionId, runtime.sessionId ?? "");
-  const runtimeSessionCwd = asString(runtimeSessionParams.cwd, "");
-  const canResumeSession =
-    runtimeSessionId.length > 0 &&
-    (runtimeSessionCwd.length === 0 || path.resolve(runtimeSessionCwd) === path.resolve(cwd));
-  const sessionId = canResumeSession ? runtimeSessionId : null;
-  if (runtimeSessionId && !canResumeSession) {
-    await onLog(
-      "stdout",
-      `[paperclip] Claude session "${runtimeSessionId}" was saved for cwd "${runtimeSessionCwd}" and will not be resumed in "${cwd}".\n`,
-    );
-  }
+    return effectiveInstructionsFilePath;
+  };
  const bootstrapPromptTemplate = asString(config.bootstrapPromptTemplate, "");
  const templateData = {
    agentId: agent.id,
@@ -435,7 +434,10 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
    heartbeatPromptChars: renderedPrompt.length,
  };

-  const buildClaudeArgs = (resumeSessionId: string | null) => {
+  const buildClaudeArgs = (
+    resumeSessionId: string | null,
+    attemptInstructionsFilePath: string | undefined,
+  ) => {
    const args = ["--print", "-", "--output-format", "stream-json", "--verbose"];
    if (resumeSessionId) args.push("--resume", resumeSessionId);
    if (dangerouslySkipPermissions) args.push("--dangerously-skip-permissions");
@@ -448,8 +450,11 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
    }
    if (effort) args.push("--effort", effort);
    if (maxTurns > 0) args.push("--max-turns", String(maxTurns));
-    if (effectiveInstructionsFilePath) {
-      args.push("--append-system-prompt-file", effectiveInstructionsFilePath);
+    // On resumed sessions the instructions are already in the session cache;
+    // re-injecting them via --append-system-prompt-file wastes 5-10K tokens
+    // per heartbeat and the Claude CLI may reject the combination outright.
+    if (attemptInstructionsFilePath && !resumeSessionId) {
+      args.push("--append-system-prompt-file", attemptInstructionsFilePath);
    }
    args.push("--add-dir", skillsDir);
    if (extraArgs.length > 0) args.push(...extraArgs);
@@ -473,7 +478,14 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
  };

  const runAttempt = async (resumeSessionId: string | null) => {
-    const args = buildClaudeArgs(resumeSessionId);
+    const attemptInstructionsFilePath = await ensureEffectiveInstructionsFilePath(resumeSessionId);
+    const args = buildClaudeArgs(resumeSessionId, attemptInstructionsFilePath);
+    const commandNotes =
+      attemptInstructionsFilePath && !resumeSessionId
+        ? [
+            `Injected agent instructions via --append-system-prompt-file ${instructionsFilePath} (with path directive appended)`,
+          ]
+        : [];
    if (onMeta) {
      await onMeta({
        adapterType: "claude_local",
@@ -9,10 +9,14 @@ async function writeFakeClaudeCommand(commandPath: string): Promise<void> {
 const fs = require("node:fs");

 const capturePath = process.env.PAPERCLIP_TEST_CAPTURE_PATH;
+const promptFileFlagIndex = process.argv.indexOf("--append-system-prompt-file");
+const appendedSystemPromptFilePath = promptFileFlagIndex >= 0 ? process.argv[promptFileFlagIndex + 1] : null;
 const payload = {
  argv: process.argv.slice(2),
  prompt: fs.readFileSync(0, "utf8"),
  claudeConfigDir: process.env.CLAUDE_CONFIG_DIR || null,
+  appendedSystemPromptFilePath,
+  appendedSystemPromptFileContents: appendedSystemPromptFilePath ? fs.readFileSync(appendedSystemPromptFilePath, "utf8") : null,
 };
 if (capturePath) {
  fs.writeFileSync(capturePath, JSON.stringify(payload), "utf8");
@@ -25,7 +29,310 @@ console.log(JSON.stringify({ type: "result", session_id: "claude-session-1", res
  await fs.chmod(commandPath, 0o755);
 }

+async function writeRetryThenSucceedClaudeCommand(commandPath: string): Promise<void> {
+  const script = `#!/usr/bin/env node
+const fs = require("node:fs");
+
+const capturePath = process.env.PAPERCLIP_TEST_CAPTURE_PATH;
+const statePath = process.env.PAPERCLIP_TEST_STATE_PATH;
+const promptFileFlagIndex = process.argv.indexOf("--append-system-prompt-file");
+const appendedSystemPromptFilePath = promptFileFlagIndex >= 0 ? process.argv[promptFileFlagIndex + 1] : null;
+const payload = {
+  argv: process.argv.slice(2),
+  prompt: fs.readFileSync(0, "utf8"),
+  claudeConfigDir: process.env.CLAUDE_CONFIG_DIR || null,
+  appendedSystemPromptFilePath,
+  appendedSystemPromptFileContents: appendedSystemPromptFilePath ? fs.readFileSync(appendedSystemPromptFilePath, "utf8") : null,
+};
+if (capturePath) {
+  const entries = fs.existsSync(capturePath) ? JSON.parse(fs.readFileSync(capturePath, "utf8")) : [];
+  entries.push(payload);
+  fs.writeFileSync(capturePath, JSON.stringify(entries), "utf8");
+}
+const resumed = process.argv.includes("--resume");
+const shouldFailResume = resumed && statePath && !fs.existsSync(statePath);
+if (shouldFailResume) {
+  fs.writeFileSync(statePath, "retried", "utf8");
+  console.log(JSON.stringify({
+    type: "result",
+    subtype: "error",
+    session_id: "claude-session-1",
+    result: "No conversation found with session id claude-session-1",
+    errors: ["No conversation found with session id claude-session-1"],
+  }));
+  process.exit(1);
+}
+console.log(JSON.stringify({ type: "system", subtype: "init", session_id: "claude-session-2", model: "claude-sonnet" }));
+console.log(JSON.stringify({ type: "assistant", session_id: "claude-session-2", message: { content: [{ type: "text", text: "hello" }] } }));
+console.log(JSON.stringify({ type: "result", session_id: "claude-session-2", result: "hello", usage: { input_tokens: 1, cache_read_input_tokens: 0, output_tokens: 1 } }));
+`;
+  await fs.writeFile(commandPath, script, "utf8");
+  await fs.chmod(commandPath, 0o755);
+}
+
+async function setupExecuteEnv(
+  root: string,
+  options?: { commandWriter?: (commandPath: string) => Promise<void> },
+) {
+  const workspace = path.join(root, "workspace");
+  const binDir = path.join(root, "bin");
+  const commandPath = path.join(binDir, "claude");
+  const capturePath = path.join(root, "capture.json");
+  const statePath = path.join(root, "state.txt");
+  await fs.mkdir(workspace, { recursive: true });
+  await fs.mkdir(binDir, { recursive: true });
+  await (options?.commandWriter ?? writeFakeClaudeCommand)(commandPath);
+  const previousHome = process.env.HOME;
+  const previousPath = process.env.PATH;
+  process.env.HOME = root;
+  process.env.PATH = `${binDir}${path.delimiter}${process.env.PATH ?? ""}`;
+  return {
+    workspace, commandPath, capturePath, statePath,
+    restore: () => {
+      if (previousHome === undefined) delete process.env.HOME;
+      else process.env.HOME = previousHome;
+      if (previousPath === undefined) delete process.env.PATH;
+      else process.env.PATH = previousPath;
+    },
+  };
+}
+
 describe("claude execute", () => {
+  /**
+   * Regression tests for https://github.com/paperclipai/paperclip/issues/2848
+   *
+   * --append-system-prompt-file should only be passed on fresh sessions.
+   * On resumed sessions the instructions are already in the session cache;
+   * re-injecting them wastes tokens and may be rejected by the CLI.
+   */
+  it("passes --append-system-prompt-file on a fresh session when instructionsFile is set", async () => {
+    const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-claude-exec-fresh-"));
+    const { workspace, commandPath, capturePath, restore } = await setupExecuteEnv(root);
+    const instructionsFile = path.join(root, "instructions.md");
+    await fs.writeFile(instructionsFile, "# Agent instructions", "utf-8");
+    try {
+      await execute({
+        runId: "run-fresh",
+        agent: { id: "agent-1", companyId: "co-1", name: "Test", adapterType: "claude_local", adapterConfig: {} },
+        runtime: { sessionId: null, sessionParams: null, sessionDisplayId: null, taskKey: null },
+        config: {
+          command: commandPath,
+          cwd: workspace,
+          env: { PAPERCLIP_TEST_CAPTURE_PATH: capturePath },
+          promptTemplate: "Do work.",
+          instructionsFilePath: instructionsFile,
+        },
+        context: {},
+        authToken: "tok",
+        onLog: async () => {},
+        onMeta: async () => {},
+      });
+      const captured = JSON.parse(await fs.readFile(capturePath, "utf-8"));
+      expect(captured.argv).toContain("--append-system-prompt-file");
+    } finally {
+      restore();
+      await fs.rm(root, { recursive: true, force: true });
+    }
+  });
+
+  it("omits --append-system-prompt-file on a resumed session even when instructionsFile is set", async () => {
+    const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-claude-exec-resume-"));
+    const { workspace, commandPath, capturePath, restore } = await setupExecuteEnv(root);
+    const instructionsFile = path.join(root, "instructions.md");
+    await fs.writeFile(instructionsFile, "# Agent instructions", "utf-8");
+    try {
+      await execute({
+        runId: "run-resume",
+        agent: { id: "agent-1", companyId: "co-1", name: "Test", adapterType: "claude_local", adapterConfig: {} },
+        runtime: { sessionId: "claude-session-1", sessionParams: null, sessionDisplayId: null, taskKey: null },
+        config: {
+          command: commandPath,
+          cwd: workspace,
+          env: { PAPERCLIP_TEST_CAPTURE_PATH: capturePath },
+          promptTemplate: "Do work.",
+          instructionsFilePath: instructionsFile,
+        },
+        context: {},
+        authToken: "tok",
+        onLog: async () => {},
+        onMeta: async () => {},
+      });
+      const captured = JSON.parse(await fs.readFile(capturePath, "utf-8"));
+      expect(captured.argv).not.toContain("--append-system-prompt-file");
+      expect(captured.argv).toContain("--resume");
+    } finally {
+      restore();
+      await fs.rm(root, { recursive: true, force: true });
+    }
+  });
+
+  /**
+   * Regression tests for commandNotes accuracy (Greptile P2).
+   *
+   * commandNotes should only claim instructions were injected when the flag
+   * was actually passed — i.e. on fresh sessions, not resumed ones.
+   */
+  it("commandNotes reports injection on a fresh session with instructionsFile", async () => {
+    const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-claude-exec-notes-fresh-"));
+    const { workspace, commandPath, restore } = await setupExecuteEnv(root);
+    const instructionsFile = path.join(root, "instructions.md");
+    await fs.writeFile(instructionsFile, "# Agent instructions", "utf-8");
+    let capturedNotes: string[] = [];
+    try {
+      await execute({
+        runId: "run-notes-fresh",
+        agent: { id: "agent-1", companyId: "co-1", name: "Test", adapterType: "claude_local", adapterConfig: {} },
+        runtime: { sessionId: null, sessionParams: null, sessionDisplayId: null, taskKey: null },
+        config: {
+          command: commandPath,
+          cwd: workspace,
+          env: {},
+          promptTemplate: "Do work.",
+          instructionsFilePath: instructionsFile,
+        },
+        context: {},
+        authToken: "tok",
+        onLog: async () => {},
+        onMeta: async (meta) => { capturedNotes = (meta.commandNotes as string[]) ?? []; },
+      });
+      expect(capturedNotes.some((n) => n.includes("--append-system-prompt-file"))).toBe(true);
+    } finally {
+      restore();
+      await fs.rm(root, { recursive: true, force: true });
+    }
+  });
+
+  it("commandNotes is empty on a resumed session even when instructionsFile is set", async () => {
+    const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-claude-exec-notes-resume-"));
+    const { workspace, commandPath, restore } = await setupExecuteEnv(root);
+    const instructionsFile = path.join(root, "instructions.md");
+    await fs.writeFile(instructionsFile, "# Agent instructions", "utf-8");
+    let capturedNotes: string[] = ["sentinel"];
+    try {
+      await execute({
+        runId: "run-notes-resume",
+        agent: { id: "agent-1", companyId: "co-1", name: "Test", adapterType: "claude_local", adapterConfig: {} },
+        runtime: { sessionId: "claude-session-1", sessionParams: null, sessionDisplayId: null, taskKey: null },
+        config: {
+          command: commandPath,
+          cwd: workspace,
+          env: {},
+          promptTemplate: "Do work.",
+          instructionsFilePath: instructionsFile,
+        },
+        context: {},
+        authToken: "tok",
+        onLog: async () => {},
+        onMeta: async (meta) => { capturedNotes = (meta.commandNotes as string[]) ?? []; },
+      });
+      expect(capturedNotes).toHaveLength(0);
+    } finally {
+      restore();
+      await fs.rm(root, { recursive: true, force: true });
+    }
+  });
+
+  /**
+   * Regression test for unnecessary file I/O on resumed sessions (Greptile P2).
+   *
+   * The combined agent-instructions.md temp file must NOT be written when
+   * resuming, since the instructions are already baked into the session cache.
+   */
+  it("does not write agent-instructions temp file on a resumed session", async () => {
+    const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-claude-exec-io-resume-"));
+    const { workspace, commandPath, restore } = await setupExecuteEnv(root);
+    const instructionsFile = path.join(root, "instructions.md");
+    await fs.writeFile(instructionsFile, "# Agent instructions", "utf-8");
+    try {
+      await execute({
+        runId: "run-io-resume",
+        agent: { id: "agent-1", companyId: "co-1", name: "Test", adapterType: "claude_local", adapterConfig: {} },
+        runtime: { sessionId: "claude-session-1", sessionParams: null, sessionDisplayId: null, taskKey: null },
+        config: {
+          command: commandPath,
+          cwd: workspace,
+          env: {},
+          promptTemplate: "Do work.",
+          instructionsFilePath: instructionsFile,
+        },
+        context: {},
+        authToken: "tok",
+        onLog: async () => {},
+        onMeta: async () => {},
+      });
+      // The skills dir lives under HOME/.paperclip/skills — verify no combined
+      // agent-instructions.md was written anywhere under root on a resume.
+      const allFiles = await fs.readdir(root, { recursive: true });
+      const tempInstructionsWritten = (allFiles as string[]).some((f) =>
+        f.includes("agent-instructions.md"),
+      );
+      expect(tempInstructionsWritten).toBe(false);
+    } finally {
+      restore();
+      await fs.rm(root, { recursive: true, force: true });
+    }
+  });
+
+  it("rebuilds the combined instructions file when an unknown resumed session falls back to fresh", async () => {
+    const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-claude-exec-resume-fallback-"));
+    const { workspace, commandPath, capturePath, statePath, restore } = await setupExecuteEnv(root, {
+      commandWriter: writeRetryThenSucceedClaudeCommand,
+    });
+    const instructionsFile = path.join(root, "instructions.md");
+    await fs.writeFile(instructionsFile, "# Agent instructions", "utf-8");
+    const metaEvents: Array<{ commandArgs: string[]; commandNotes: string[] }> = [];
+    try {
+      const result = await execute({
+        runId: "run-resume-fallback",
+        agent: { id: "agent-1", companyId: "co-1", name: "Test", adapterType: "claude_local", adapterConfig: {} },
+        runtime: { sessionId: "claude-session-1", sessionParams: null, sessionDisplayId: null, taskKey: null },
+        config: {
+          command: commandPath,
+          cwd: workspace,
+          env: {
+            PAPERCLIP_TEST_CAPTURE_PATH: capturePath,
+            PAPERCLIP_TEST_STATE_PATH: statePath,
+          },
+          promptTemplate: "Do work.",
+          instructionsFilePath: instructionsFile,
+        },
+        context: {},
+        authToken: "tok",
+        onLog: async () => {},
+        onMeta: async (meta) => {
+          metaEvents.push({
+            commandArgs: ((meta.commandArgs as string[]) ?? []).slice(),
+            commandNotes: ((meta.commandNotes as string[]) ?? []).slice(),
+          });
+        },
+      });
+      const captured = JSON.parse(await fs.readFile(capturePath, "utf-8")) as Array<{
+        argv: string[];
+        appendedSystemPromptFilePath: string | null;
+        appendedSystemPromptFileContents: string | null;
+      }>;
+      expect(captured).toHaveLength(2);
+      expect(captured[0]?.argv).toContain("--resume");
+      expect(captured[0]?.argv).not.toContain("--append-system-prompt-file");
+      expect(captured[1]?.argv).not.toContain("--resume");
+      expect(captured[1]?.argv).toContain("--append-system-prompt-file");
+      expect(captured[1]?.appendedSystemPromptFilePath).toContain("agent-instructions.md");
+      expect(captured[1]?.appendedSystemPromptFilePath).not.toBe(instructionsFile);
+      expect(captured[1]?.appendedSystemPromptFileContents).toContain("# Agent instructions");
+      expect(captured[1]?.appendedSystemPromptFileContents).toContain(
+        `The above agent instructions were loaded from ${instructionsFile}. Resolve any relative file references from ${path.dirname(instructionsFile)}/.`,
+      );
+      expect(metaEvents).toHaveLength(2);
+      expect(metaEvents[0]?.commandNotes).toHaveLength(0);
+      expect(metaEvents[1]?.commandNotes.some((note) => note.includes("--append-system-prompt-file"))).toBe(true);
+      expect(result.sessionId).toBe("claude-session-2");
+      expect(result.clearSession).toBe(false);
+    } finally {
+      restore();
+      await fs.rm(root, { recursive: true, force: true });
+    }
+  });
+
  it("logs HOME, CLAUDE_CONFIG_DIR, and the resolved executable path in invocation metadata", async () => {
    const root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-claude-execute-meta-"));
    const workspace = path.join(root, "workspace");