mirror of
https://github.com/alkimake/paperclip.git
synced 2026-06-17 03:10:38 +09:00
fix: gate instructions file I/O and commandNotes on fresh sessions only
On resumed sessions, skipping --append-system-prompt-file (the original fix) left two secondary issues: - commandNotes still claimed the flag was injected, producing misleading onMeta logs on every resumed heartbeat - The instructions file was still read from disk and a combined temp file written on every resume, even though effectiveInstructionsFilePath was never consumed Hoist canResumeSession before the I/O block and gate both the disk operations and commandNotes construction on !canResumeSession / !sessionId. Adds three regression tests: commandNotes is populated on fresh sessions, empty on resume; and no agent-instructions.md is written on resume.
This commit is contained in:
parent
3cfbc350a0
commit
e3804f792d
2 changed files with 132 additions and 20 deletions
|
|
@ -335,12 +335,6 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
||||||
const dangerouslySkipPermissions = asBoolean(config.dangerouslySkipPermissions, true);
|
const dangerouslySkipPermissions = asBoolean(config.dangerouslySkipPermissions, true);
|
||||||
const instructionsFilePath = asString(config.instructionsFilePath, "").trim();
|
const instructionsFilePath = asString(config.instructionsFilePath, "").trim();
|
||||||
const instructionsFileDir = instructionsFilePath ? `${path.dirname(instructionsFilePath)}/` : "";
|
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({
|
const runtimeConfig = await buildClaudeRuntimeConfig({
|
||||||
runId,
|
runId,
|
||||||
agent,
|
agent,
|
||||||
|
|
@ -369,11 +363,27 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
||||||
const billingType = resolveClaudeBillingType(effectiveEnv);
|
const billingType = resolveClaudeBillingType(effectiveEnv);
|
||||||
const skillsDir = await buildSkillsDir(config);
|
const skillsDir = await buildSkillsDir(config);
|
||||||
|
|
||||||
|
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`,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
// When instructionsFilePath is configured, create a combined temp file that
|
// When instructionsFilePath is configured, create a combined temp file that
|
||||||
// includes both the file content and the path directive, so we only need
|
// includes both the file content and the path directive, so we only need
|
||||||
// --append-system-prompt-file (Claude CLI forbids using both flags together).
|
// --append-system-prompt-file (Claude CLI forbids using both flags together).
|
||||||
|
// Skipped on resumed sessions — the instructions are already baked into the
|
||||||
|
// session cache and re-injecting them wastes tokens.
|
||||||
let effectiveInstructionsFilePath: string | undefined = instructionsFilePath;
|
let effectiveInstructionsFilePath: string | undefined = instructionsFilePath;
|
||||||
if (instructionsFilePath) {
|
if (instructionsFilePath && !canResumeSession) {
|
||||||
try {
|
try {
|
||||||
const instructionsContent = await fs.readFile(instructionsFilePath, "utf-8");
|
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}.`;
|
const pathDirective = `\nThe above agent instructions were loaded from ${instructionsFilePath}. Resolve any relative file references from ${instructionsFileDir}.`;
|
||||||
|
|
@ -388,21 +398,16 @@ export async function execute(ctx: AdapterExecutionContext): Promise<AdapterExec
|
||||||
);
|
);
|
||||||
effectiveInstructionsFilePath = undefined;
|
effectiveInstructionsFilePath = undefined;
|
||||||
}
|
}
|
||||||
|
} else if (canResumeSession) {
|
||||||
|
effectiveInstructionsFilePath = undefined;
|
||||||
}
|
}
|
||||||
|
|
||||||
const runtimeSessionParams = parseObject(runtime.sessionParams);
|
const commandNotes =
|
||||||
const runtimeSessionId = asString(runtimeSessionParams.sessionId, runtime.sessionId ?? "");
|
instructionsFilePath && !sessionId
|
||||||
const runtimeSessionCwd = asString(runtimeSessionParams.cwd, "");
|
? [
|
||||||
const canResumeSession =
|
`Injected agent instructions via --append-system-prompt-file ${instructionsFilePath} (with path directive appended)`,
|
||||||
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`,
|
|
||||||
);
|
|
||||||
}
|
|
||||||
const bootstrapPromptTemplate = asString(config.bootstrapPromptTemplate, "");
|
const bootstrapPromptTemplate = asString(config.bootstrapPromptTemplate, "");
|
||||||
const templateData = {
|
const templateData = {
|
||||||
agentId: agent.id,
|
agentId: agent.id,
|
||||||
|
|
|
||||||
|
|
@ -117,6 +117,113 @@ describe("claude execute", () => {
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* 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("logs HOME, CLAUDE_CONFIG_DIR, and the resolved executable path in invocation metadata", async () => {
|
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 root = await fs.mkdtemp(path.join(os.tmpdir(), "paperclip-claude-execute-meta-"));
|
||||||
const workspace = path.join(root, "workspace");
|
const workspace = path.join(root, "workspace");
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue