mirror of
https://github.com/earendil-works/pi.git
synced 2026-10-02 00:35:27 +08:00
fix(coding-agent): disable tools during summarization
This commit is contained in:
@@ -357,6 +357,9 @@ export async function generateBranchSummary(
|
||||
if (response.stopReason === "error") {
|
||||
return { error: response.errorMessage || "Summarization failed" };
|
||||
}
|
||||
if (response.content.some((block) => block.type === "toolCall")) {
|
||||
return { error: "Branch summarization attempted to call a tool" };
|
||||
}
|
||||
|
||||
let summary = contentText(response.content);
|
||||
|
||||
|
||||
@@ -589,6 +589,7 @@ export async function completeSummarization(
|
||||
...options,
|
||||
cacheRetention: "none",
|
||||
sessionId: options.sessionId ?? uuidv7(),
|
||||
toolChoice: "none",
|
||||
};
|
||||
const produce = async (): Promise<AssistantMessage> =>
|
||||
streamFn
|
||||
@@ -708,6 +709,9 @@ export async function generateSummaryWithUsage(
|
||||
if (response.stopReason === "error") {
|
||||
throw new Error(`Summarization failed: ${response.errorMessage || "Unknown error"}`);
|
||||
}
|
||||
if (response.content.some((block) => block.type === "toolCall")) {
|
||||
throw new Error("Summarization attempted to call a tool");
|
||||
}
|
||||
|
||||
const textContent = contentText(response.content);
|
||||
|
||||
@@ -996,6 +1000,9 @@ async function generateTurnPrefixSummary(
|
||||
if (response.stopReason === "error") {
|
||||
throw new Error(`Turn prefix summarization failed: ${response.errorMessage || "Unknown error"}`);
|
||||
}
|
||||
if (response.content.some((block) => block.type === "toolCall")) {
|
||||
throw new Error("Turn prefix summarization attempted to call a tool");
|
||||
}
|
||||
|
||||
return {
|
||||
text: contentText(response.content),
|
||||
|
||||
@@ -0,0 +1,90 @@
|
||||
import type { StreamFn } from "@earendil-works/pi-agent-core";
|
||||
import {
|
||||
type AssistantMessage,
|
||||
createAssistantMessageEventStream,
|
||||
fauxAssistantMessage,
|
||||
type Model,
|
||||
type SimpleStreamOptions,
|
||||
} from "@earendil-works/pi-ai";
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { generateBranchSummary } from "../src/core/compaction/index.ts";
|
||||
import type { SessionEntry } from "../src/core/session-manager.ts";
|
||||
|
||||
const model: Model<"anthropic-messages"> = {
|
||||
id: "test-model",
|
||||
name: "Test Model",
|
||||
api: "anthropic-messages",
|
||||
provider: "anthropic",
|
||||
baseUrl: "https://api.anthropic.com",
|
||||
reasoning: false,
|
||||
input: ["text"],
|
||||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 },
|
||||
contextWindow: 200000,
|
||||
maxTokens: 8192,
|
||||
};
|
||||
|
||||
const entries: SessionEntry[] = [
|
||||
{
|
||||
type: "message",
|
||||
id: "branch-user",
|
||||
parentId: null,
|
||||
timestamp: new Date(1).toISOString(),
|
||||
message: { role: "user", content: "Abandoned request", timestamp: 1 },
|
||||
},
|
||||
];
|
||||
|
||||
function response(content: AssistantMessage["content"]): AssistantMessage {
|
||||
return {
|
||||
...fauxAssistantMessage(""),
|
||||
content,
|
||||
api: model.api,
|
||||
provider: model.provider,
|
||||
model: model.id,
|
||||
};
|
||||
}
|
||||
|
||||
describe("branch summarization", () => {
|
||||
it("disables tools for branch summaries", async () => {
|
||||
let requestOptions: SimpleStreamOptions | undefined;
|
||||
const streamFn: StreamFn = (_model, _context, options) => {
|
||||
requestOptions = options;
|
||||
const stream = createAssistantMessageEventStream();
|
||||
queueMicrotask(() =>
|
||||
stream.push({ type: "done", reason: "stop", message: response([{ type: "text", text: "summary" }]) }),
|
||||
);
|
||||
return stream;
|
||||
};
|
||||
|
||||
await generateBranchSummary(entries, {
|
||||
model,
|
||||
signal: new AbortController().signal,
|
||||
streamFn,
|
||||
});
|
||||
|
||||
expect(requestOptions?.toolChoice).toBe("none");
|
||||
});
|
||||
|
||||
it("rejects tool calls from branch summaries", async () => {
|
||||
const streamFn: StreamFn = () => {
|
||||
const stream = createAssistantMessageEventStream();
|
||||
queueMicrotask(() =>
|
||||
stream.push({
|
||||
type: "done",
|
||||
reason: "toolUse",
|
||||
message: response([
|
||||
{ type: "toolCall", id: "tool-call-1", name: "read", arguments: { path: "README.md" } },
|
||||
]),
|
||||
}),
|
||||
);
|
||||
return stream;
|
||||
};
|
||||
|
||||
const result = await generateBranchSummary(entries, {
|
||||
model,
|
||||
signal: new AbortController().signal,
|
||||
streamFn,
|
||||
});
|
||||
|
||||
expect(result.error).toBe("Branch summarization attempted to call a tool");
|
||||
});
|
||||
});
|
||||
@@ -59,6 +59,12 @@ const mockSummaryResponse: AssistantMessage = {
|
||||
timestamp: Date.now(),
|
||||
};
|
||||
|
||||
const mockToolCallResponse: AssistantMessage = {
|
||||
...mockSummaryResponse,
|
||||
content: [{ type: "toolCall", id: "tool-call-1", name: "read", arguments: { path: "README.md" } }],
|
||||
stopReason: "toolUse",
|
||||
};
|
||||
|
||||
const messages: AgentMessage[] = [{ role: "user", content: "Summarize this.", timestamp: Date.now() }];
|
||||
|
||||
describe("generateSummary reasoning options", () => {
|
||||
@@ -103,6 +109,7 @@ describe("generateSummary reasoning options", () => {
|
||||
const requestOptions = completeSimpleMock.mock.calls.map((call) => call[2]);
|
||||
expect(requestOptions).toHaveLength(2);
|
||||
expect(requestOptions.every((options) => options?.cacheRetention === "none")).toBe(true);
|
||||
expect(requestOptions.every((options) => options?.toolChoice === "none")).toBe(true);
|
||||
|
||||
const sessionIds = requestOptions.map((options) => options?.sessionId);
|
||||
expect(sessionIds[0]).not.toBe(sessionIds[1]);
|
||||
@@ -112,15 +119,41 @@ describe("generateSummary reasoning options", () => {
|
||||
await completeSummarization(
|
||||
createModel(false),
|
||||
{ systemPrompt: "Summarize", messages: [] },
|
||||
{ sessionId: "current-routing-session", cacheRetention: "long" },
|
||||
{ sessionId: "current-routing-session", cacheRetention: "long", toolChoice: "auto" },
|
||||
);
|
||||
|
||||
expect(completeSimpleMock.mock.calls[0][2]).toMatchObject({
|
||||
sessionId: "current-routing-session",
|
||||
cacheRetention: "none",
|
||||
toolChoice: "none",
|
||||
});
|
||||
});
|
||||
|
||||
it("rejects tool calls from conversation summaries", async () => {
|
||||
completeSimpleMock.mockResolvedValueOnce(mockToolCallResponse);
|
||||
|
||||
await expect(generateSummaryWithUsage(messages, createModel(false), 2000, "test-key")).rejects.toThrow(
|
||||
"Summarization attempted to call a tool",
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects tool calls from split-turn summaries", async () => {
|
||||
completeSimpleMock.mockResolvedValueOnce(mockToolCallResponse);
|
||||
const preparation: CompactionPreparation = {
|
||||
firstKeptEntryId: "entry-keep",
|
||||
messagesToSummarize: [],
|
||||
turnPrefixMessages: messages,
|
||||
isSplitTurn: true,
|
||||
tokensBefore: 100,
|
||||
fileOps: { read: new Set(), written: new Set(), edited: new Set() },
|
||||
settings: { enabled: true, reserveTokens: 2000, keepRecentTokens: 20 },
|
||||
};
|
||||
|
||||
await expect(compact(preparation, createModel(false), "test-key")).rejects.toThrow(
|
||||
"Turn prefix summarization attempted to call a tool",
|
||||
);
|
||||
});
|
||||
|
||||
it("does not set reasoning when thinking is off", async () => {
|
||||
await generateSummary(
|
||||
messages,
|
||||
|
||||
Reference in New Issue
Block a user