7fa0eeb43d
The shake-strategy post-shake threshold check was reading #estimatePendingPromptTokens([]) while #checkCompaction triggered on calculateContextTokens(assistantMessage.usage). The local estimator ignored block.thinkingSignature payloads (OpenAI Responses encrypted reasoning items, Anthropic signed thinking blocks, etc.), so on a thinking-heavy session the estimate sat ~0.9–2× below provider-reported usage. Once the two straddled the threshold, the #2119 dead-loop guard never fired, shake reported 'handled', and #scheduleAutoContinuePrompt re-injected the auto-continue developer prompt every turn — 53 injections in a real 25-minute repro session before an external timeout. Thread the trigger's provider-anchored contextTokens through #runAutoCompaction → #runAutoShake for the threshold and incomplete paths, then evaluate residual pressure as triggerContextTokens − result.tokensFreed with an 80% recovery-band hysteresis. Re-checking against the raw threshold (even on the corrected metric) would still let shake reclaim a trickle of the previous turn's elidable blocks and land just under the line every turn; the band closes that oscillation. As defense in depth, estimateTokens() now charges thinkingSignature and redactedThinking.data alongside the visible thinking text so every other site that uses the estimator (idle compaction, pre-prompt check, status line) tracks provider usage on replay. New regression test pins the contract; existing dispatch test bumped its mocked tokensFreed so its happy-path scenario lands inside the new recovery band. Fixes #2275
304 lines
12 KiB
TypeScript
304 lines
12 KiB
TypeScript
import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test";
|
||
import * as path from "node:path";
|
||
import { Agent } from "@oh-my-pi/pi-agent-core";
|
||
import type { AssistantMessage, ImageContent, ToolResultMessage } from "@oh-my-pi/pi-ai";
|
||
import { getBundledModel } from "@oh-my-pi/pi-catalog/models";
|
||
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||
import { AgentSession, type AgentSessionEvent } from "@oh-my-pi/pi-coding-agent/session/agent-session";
|
||
import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
|
||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||
|
||
const usage = {
|
||
input: 16,
|
||
output: 8,
|
||
cacheRead: 0,
|
||
cacheWrite: 0,
|
||
totalTokens: 24,
|
||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
|
||
};
|
||
|
||
describe("AgentSession shake", () => {
|
||
let tempDir: TempDir;
|
||
let session: AgentSession;
|
||
let sessionManager: SessionManager;
|
||
let authStorage: AuthStorage;
|
||
let modelRegistry: ModelRegistry;
|
||
let events: AgentSessionEvent[];
|
||
let apiInfo: { api: AssistantMessage["api"]; provider: AssistantMessage["provider"]; model: string };
|
||
|
||
beforeEach(async () => {
|
||
tempDir = TempDir.createSync("@pi-shake-");
|
||
authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db"));
|
||
authStorage.setRuntimeApiKey("anthropic", "test-key");
|
||
modelRegistry = new ModelRegistry(authStorage);
|
||
sessionManager = SessionManager.create(tempDir.path(), tempDir.path());
|
||
events = [];
|
||
|
||
const model = getBundledModel("anthropic", "claude-sonnet-4-5");
|
||
if (!model) throw new Error("Expected built-in anthropic model to exist");
|
||
apiInfo = { api: model.api, provider: model.provider, model: model.id };
|
||
|
||
const agent = new Agent({ initialState: { model, systemPrompt: ["Test"], tools: [], messages: [] } });
|
||
session = new AgentSession({
|
||
agent,
|
||
sessionManager,
|
||
settings: Settings.isolated({ "compaction.enabled": true, "compaction.autoContinue": false }),
|
||
modelRegistry,
|
||
});
|
||
session.subscribe(event => events.push(event));
|
||
});
|
||
|
||
afterEach(async () => {
|
||
if (session) await session.dispose();
|
||
authStorage.close();
|
||
try {
|
||
await tempDir.remove();
|
||
} catch {}
|
||
vi.restoreAllMocks();
|
||
});
|
||
|
||
/** Seed a user → assistant(toolCall) → toolResult turn carrying a heavy bash result. */
|
||
function seedHeavyToolResult(text: string, toolName = "bash"): void {
|
||
const toolCallId = `call_${toolName}_${Math.random().toString(36).slice(2)}`;
|
||
sessionManager.appendMessage({
|
||
role: "user",
|
||
content: [{ type: "text", text: "do it" }],
|
||
timestamp: Date.now() - 3,
|
||
});
|
||
sessionManager.appendMessage({
|
||
role: "assistant",
|
||
content: [
|
||
{ type: "text", text: "working" },
|
||
{ type: "toolCall", id: toolCallId, name: toolName, arguments: { command: "ls" } },
|
||
],
|
||
...apiInfo,
|
||
stopReason: "toolUse",
|
||
usage,
|
||
timestamp: Date.now() - 2,
|
||
});
|
||
sessionManager.appendMessage({
|
||
role: "toolResult",
|
||
toolCallId,
|
||
toolName,
|
||
content: [{ type: "text", text }],
|
||
isError: false,
|
||
timestamp: Date.now() - 1,
|
||
});
|
||
}
|
||
|
||
function branchToolResults(): ToolResultMessage[] {
|
||
return sessionManager
|
||
.getBranch()
|
||
.filter(e => e.type === "message" && (e.message as { role?: string }).role === "toolResult")
|
||
.map(e => (e as { message: ToolResultMessage }).message);
|
||
}
|
||
|
||
describe("elide", () => {
|
||
it("drops the tool result, offloads to an artifact, and embeds the recovery link", async () => {
|
||
seedHeavyToolResult("X".repeat(4000));
|
||
const replaceSpy = vi.spyOn(session.agent, "replaceMessages");
|
||
|
||
const result = await session.shake("elide");
|
||
|
||
expect(result.mode).toBe("elide");
|
||
expect(result.toolResultsDropped).toBe(1);
|
||
expect(result.tokensFreed).toBeGreaterThan(0);
|
||
expect(result.artifactId).toBeDefined();
|
||
expect(replaceSpy).toHaveBeenCalled();
|
||
|
||
const [tr] = branchToolResults();
|
||
expect(tr.prunedAt).toBeGreaterThan(0);
|
||
const text = tr.content.map(b => (b.type === "text" ? b.text : "")).join("");
|
||
expect(text).toContain(`artifact://${result.artifactId}`);
|
||
expect(text).toContain("shaken");
|
||
});
|
||
|
||
it("returns zero counts for an empty branch", async () => {
|
||
const result = await session.shake("elide");
|
||
expect(result.toolResultsDropped).toBe(0);
|
||
expect(result.blocksDropped).toBe(0);
|
||
expect(result.tokensFreed).toBe(0);
|
||
});
|
||
});
|
||
|
||
describe("images", () => {
|
||
it("mirrors dropImages and reports the removed image count", async () => {
|
||
const png: ImageContent = { type: "image", data: "iVBORw0KGgo", mimeType: "image/png" };
|
||
sessionManager.appendMessage({
|
||
role: "user",
|
||
content: [{ type: "text", text: "look" }, png],
|
||
timestamp: Date.now(),
|
||
});
|
||
|
||
const result = await session.shake("images");
|
||
|
||
expect(result.mode).toBe("images");
|
||
expect(result.imagesDropped).toBe(1);
|
||
const branch = sessionManager.getBranch();
|
||
const userMsg = branch.find(e => e.type === "message" && (e.message as { role?: string }).role === "user");
|
||
const content = (userMsg as { message: { content: unknown } }).message.content as Array<{ type: string }>;
|
||
expect(content.some(b => b.type === "image")).toBe(false);
|
||
});
|
||
});
|
||
|
||
describe("protected tools", () => {
|
||
it("never shakes skill results", async () => {
|
||
seedHeavyToolResult("S".repeat(4000), "skill");
|
||
const result = await session.shake("elide");
|
||
expect(result.toolResultsDropped).toBe(0);
|
||
});
|
||
});
|
||
|
||
describe("auto-shake strategy", () => {
|
||
it("dispatches the elide path and emits a shake action for threshold maintenance", async () => {
|
||
session.settings.set("compaction.strategy", "shake");
|
||
session.settings.set("compaction.thresholdPercent", 1);
|
||
session.settings.set("contextPromotion.enabled", false);
|
||
|
||
// Reclaim enough that the corrected (provider − tokensFreed) figure lands
|
||
// inside the 80% recovery band — otherwise the #2275 post-shake check would
|
||
// (correctly) declare pressure unresolved and fall back to context-full.
|
||
const shakeSpy = vi
|
||
.spyOn(session, "shake")
|
||
.mockResolvedValue({ mode: "elide", toolResultsDropped: 1, blocksDropped: 0, tokensFreed: 10_000 });
|
||
|
||
const assistantMessage: AssistantMessage = {
|
||
role: "assistant",
|
||
content: [{ type: "text", text: "trigger" }],
|
||
...apiInfo,
|
||
stopReason: "stop",
|
||
usage: {
|
||
input: 10_000,
|
||
output: 1_000,
|
||
cacheRead: 0,
|
||
cacheWrite: 0,
|
||
totalTokens: 11_000,
|
||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
|
||
},
|
||
timestamp: Date.now(),
|
||
};
|
||
session.agent.emitExternalEvent({ type: "message_end", message: assistantMessage });
|
||
session.agent.emitExternalEvent({ type: "agent_end", messages: [assistantMessage] });
|
||
await Bun.sleep(20);
|
||
|
||
expect(shakeSpy).toHaveBeenCalledWith("elide", expect.anything());
|
||
const start = events.filter(e => e.type === "auto_compaction_start");
|
||
expect(start).toHaveLength(1);
|
||
expect(start[0]).toMatchObject({ type: "auto_compaction_start", reason: "threshold", action: "shake" });
|
||
const end = events.filter(e => e.type === "auto_compaction_end");
|
||
expect(end).toHaveLength(1);
|
||
expect(end[0]).toMatchObject({ type: "auto_compaction_end", action: "shake" });
|
||
});
|
||
|
||
it("falls back to context-full when shake cannot drop context below the threshold (regression #2119)", async () => {
|
||
session.settings.set("compaction.strategy", "shake");
|
||
session.settings.set("compaction.thresholdPercent", 1);
|
||
session.settings.set("contextPromotion.enabled", false);
|
||
|
||
// Seed agent state so the post-shake estimate is well above the 1% threshold
|
||
// (~2K tokens for a 200K window). The mocked shake returns reclaimed=true but
|
||
// does not modify state, mimicking the dead-loop scenario where shake removes
|
||
// nothing material yet the threshold check stays positive.
|
||
session.agent.replaceMessages([
|
||
{
|
||
role: "user",
|
||
content: [{ type: "text", text: "x".repeat(40000) }],
|
||
timestamp: Date.now(),
|
||
} as never,
|
||
]);
|
||
|
||
const shakeSpy = vi
|
||
.spyOn(session, "shake")
|
||
.mockResolvedValue({ mode: "elide", toolResultsDropped: 1, blocksDropped: 0, tokensFreed: 10 });
|
||
|
||
const assistantMessage: AssistantMessage = {
|
||
role: "assistant",
|
||
content: [{ type: "text", text: "trigger" }],
|
||
...apiInfo,
|
||
stopReason: "stop",
|
||
usage: {
|
||
input: 10_000,
|
||
output: 1_000,
|
||
cacheRead: 0,
|
||
cacheWrite: 0,
|
||
totalTokens: 11_000,
|
||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
|
||
},
|
||
timestamp: Date.now(),
|
||
};
|
||
session.agent.emitExternalEvent({ type: "message_end", message: assistantMessage });
|
||
session.agent.emitExternalEvent({ type: "agent_end", messages: [assistantMessage] });
|
||
await Bun.sleep(50);
|
||
|
||
// Shake fires once. The pre-fix bug auto-continued, which would re-trigger shake
|
||
// on the next agent_end. The fix replaces that loop with a one-shot fallback.
|
||
expect(shakeSpy).toHaveBeenCalledTimes(1);
|
||
|
||
const shakeEnd = events.find(
|
||
e => e.type === "auto_compaction_end" && (e as { action?: string }).action === "shake",
|
||
) as { errorMessage?: string; skipped?: boolean } | undefined;
|
||
expect(shakeEnd).toBeDefined();
|
||
expect(shakeEnd?.errorMessage).toMatch(/falling back to context-full/i);
|
||
|
||
// Fallback enters the context-full path so the situation actually resolves.
|
||
const fullStart = events.find(
|
||
e => e.type === "auto_compaction_start" && (e as { action?: string }).action === "context-full",
|
||
);
|
||
expect(fullStart).toBeDefined();
|
||
});
|
||
|
||
it("falls back when provider-reported usage stays above the threshold even though the local estimate is below it (regression #2275)", async () => {
|
||
session.settings.set("compaction.strategy", "shake");
|
||
session.settings.set("compaction.thresholdTokens", 5_000);
|
||
session.settings.set("contextPromotion.enabled", false);
|
||
|
||
// Agent state holds almost no content, so #estimatePendingPromptTokens reads
|
||
// well below the 5K threshold. The pre-fix post-shake check trusted that
|
||
// estimate and treated the pressure as resolved, even though the assistant
|
||
// message's provider-reported usage (11K) was well above the threshold.
|
||
// This is the metric-divergence dead loop from #2275: thinking-heavy
|
||
// sessions hit it for real (thinkingSignature payloads aren't counted by
|
||
// the estimator), and an empty-state probe mimics it deterministically.
|
||
session.agent.replaceMessages([]);
|
||
|
||
const shakeSpy = vi
|
||
.spyOn(session, "shake")
|
||
.mockResolvedValue({ mode: "elide", toolResultsDropped: 1, blocksDropped: 0, tokensFreed: 10 });
|
||
|
||
const assistantMessage: AssistantMessage = {
|
||
role: "assistant",
|
||
content: [{ type: "text", text: "trigger" }],
|
||
...apiInfo,
|
||
stopReason: "stop",
|
||
usage: {
|
||
input: 10_000,
|
||
output: 1_000,
|
||
cacheRead: 0,
|
||
cacheWrite: 0,
|
||
totalTokens: 11_000,
|
||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
|
||
},
|
||
timestamp: Date.now(),
|
||
};
|
||
session.agent.emitExternalEvent({ type: "message_end", message: assistantMessage });
|
||
session.agent.emitExternalEvent({ type: "agent_end", messages: [assistantMessage] });
|
||
await Bun.sleep(50);
|
||
|
||
expect(shakeSpy).toHaveBeenCalledTimes(1);
|
||
|
||
const shakeEnd = events.find(
|
||
e => e.type === "auto_compaction_end" && (e as { action?: string }).action === "shake",
|
||
) as { errorMessage?: string; skipped?: boolean } | undefined;
|
||
expect(shakeEnd).toBeDefined();
|
||
expect(shakeEnd?.errorMessage).toMatch(/falling back to context-full/i);
|
||
|
||
const fullStart = events.find(
|
||
e => e.type === "auto_compaction_start" && (e as { action?: string }).action === "context-full",
|
||
);
|
||
expect(fullStart).toBeDefined();
|
||
});
|
||
});
|
||
});
|