diff --git a/packages/agent/CHANGELOG.md b/packages/agent/CHANGELOG.md index c42866b0f..7c6e6f119 100644 --- a/packages/agent/CHANGELOG.md +++ b/packages/agent/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixed - The error→toolUse salvage in the agent loop (`recoverTransientErrorToolTurn`) now recognizes Anthropic stream-envelope truncation errors, so a turn cut after streaming complete tool calls runs those calls instead of ending the run with an error. +- Shake no longer elides artifact recovery reads; the compaction dead-end rescue uses a dedicated `RESCUE_SHAKE_CONFIG`. ## [17.2.4] - 2026-08-01 diff --git a/packages/agent/src/compaction/shake.ts b/packages/agent/src/compaction/shake.ts index db95486de..3d5289b7d 100644 --- a/packages/agent/src/compaction/shake.ts +++ b/packages/agent/src/compaction/shake.ts @@ -18,6 +18,7 @@ import type { CustomMessageEntry, SessionEntry, SessionMessageEntry } from "./en import { invalidateMessageCache } from "./message-cache"; import { collectToolCallsById, + isArtifactRecoveryToolResult, isProtectedToolResult, isSkillReadToolResult, type ProtectedToolMatcher, @@ -46,11 +47,14 @@ export interface ShakeConfig { export const DEFAULT_SHAKE_CONFIG: ShakeConfig = { protectTokens: 16_000, minSavings: 4_000, - protectedTools: ["skill", isSkillReadToolResult], + protectedTools: ["skill", isSkillReadToolResult, isArtifactRecoveryToolResult], fenceMinTokens: 400, }; -/** Manual `/shake`: aggressive — drops every eligible region across history. */ +/** + * Manual `/shake`: aggressive — drops every eligible region across history, + * artifact recovery reads included (the user's full escape hatch). + */ export const AGGRESSIVE_SHAKE_CONFIG: ShakeConfig = { protectTokens: 0, minSavings: 0, @@ -58,6 +62,12 @@ export const AGGRESSIVE_SHAKE_CONFIG: ShakeConfig = { fenceMinTokens: 400, }; +/** Compaction dead-end rescue: aggressive reach, but artifact recovery reads stay protected. */ +export const RESCUE_SHAKE_CONFIG: ShakeConfig = { + ...AGGRESSIVE_SHAKE_CONFIG, + protectedTools: [...AGGRESSIVE_SHAKE_CONFIG.protectedTools, isArtifactRecoveryToolResult], +}; + /** Rough token cost of a placeholder line; used only for the savings gate. */ const PLACEHOLDER_TOKEN_ESTIMATE = 16; diff --git a/packages/agent/src/compaction/tool-protection.ts b/packages/agent/src/compaction/tool-protection.ts index 3ac5ca74e..259fbf95b 100644 --- a/packages/agent/src/compaction/tool-protection.ts +++ b/packages/agent/src/compaction/tool-protection.ts @@ -39,6 +39,16 @@ export function isSkillReadToolResult(context: ProtectedToolContext): boolean { return getReadToolPath(context)?.startsWith(SKILL_INTERNAL_URL_PREFIX) ?? false; } +const ARTIFACT_INTERNAL_URL_PREFIX = "artifact://"; + +/** Recovery reads of session artifacts — eliding one only mints another artifact and can repeat indefinitely. */ +export function isArtifactRecoveryToolResult(context: ProtectedToolContext): boolean { + if (getReadToolPath(context)?.startsWith(ARTIFACT_INTERNAL_URL_PREFIX)) return true; + const meta = (context.toolResult.details as { meta?: { source?: { type?: string; value?: string } } } | undefined) + ?.meta; + return meta?.source?.type === "internal" && (meta.source.value?.startsWith(ARTIFACT_INTERNAL_URL_PREFIX) ?? false); +} + export function isProtectedToolResult( toolResult: ToolResultMessage, toolCall: AgentToolCall | undefined, diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c051a7347..fe0326ed6 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -26,6 +26,9 @@ - Fixed macOS background keyboard input silently reaching a different window in the same application; ambiguous multi-window delivery now fails with `BackgroundUnavailable`, and foreground retries establish the addressed window as the app's main/focused window before typing. - Fixed macOS Chrome accessibility snapshots exposing only browser chrome by activating Chromium's renderer accessibility tree before resolving a window. - Fixed unnamed macOS accessibility controls such as Chrome's Back, Forward, and Reload buttons by using `AXDescription` when `AXTitle` is empty in snapshots and title queries. +### Fixed + +- Fixed shake re-eliding artifact recovery reads into a new artifact indefinitely. ## [17.2.4] - 2026-08-01 diff --git a/packages/coding-agent/src/session/session-maintenance.ts b/packages/coding-agent/src/session/session-maintenance.ts index e1ccc43a5..bb424ab07 100644 --- a/packages/coding-agent/src/session/session-maintenance.ts +++ b/packages/coding-agent/src/session/session-maintenance.ts @@ -28,6 +28,7 @@ import { estimateTokens, NativeCompactionError, prepareCompaction, + RESCUE_SHAKE_CONFIG, resolveBudgetReserveTokens, resolveThresholdTokens, type ShakeConfig, @@ -1859,7 +1860,7 @@ export class SessionMaintenance { let elideSink = "placeholders"; if (!options.skipElide) { try { - const result = await this.#host.shake("elide", { signal }); + const result = await this.#host.shake("elide", { config: RESCUE_SHAKE_CONFIG, signal }); elided = result.toolResultsDropped + result.blocksDropped; elidedTokens = result.tokensFreed; if (result.artifactId) elideSink = "an artifact"; diff --git a/packages/coding-agent/test/agent-session-snapcompact-frame-dead-end.test.ts b/packages/coding-agent/test/agent-session-snapcompact-frame-dead-end.test.ts index d07bd7c2e..ef1f63730 100644 --- a/packages/coding-agent/test/agent-session-snapcompact-frame-dead-end.test.ts +++ b/packages/coding-agent/test/agent-session-snapcompact-frame-dead-end.test.ts @@ -1,7 +1,7 @@ import { afterEach, describe, expect, it, vi } from "bun:test"; import * as fs from "node:fs"; import * as path from "node:path"; -import { Agent } from "@oh-my-pi/pi-agent-core"; +import { Agent, RESCUE_SHAKE_CONFIG } from "@oh-my-pi/pi-agent-core"; import * as compactionModule from "@oh-my-pi/pi-agent-core/compaction"; import { getBundledModel } from "@oh-my-pi/pi-catalog/models"; import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; @@ -389,7 +389,7 @@ describe("AgentSession snapcompact frame dead-end rescue", () => { const notices = collectNotices(); await triggerMaintenance(); - expect(shakeSpy).toHaveBeenCalledWith("elide", expect.anything()); + expect(shakeSpy).toHaveBeenCalledWith("elide", expect.objectContaining({ config: RESCUE_SHAKE_CONFIG })); const noProgress = notices.filter(n => n.source === NOTICE_SOURCE && n.message.includes(NO_PROGRESS_FRAGMENT)); expect(noProgress.length).toBe(1); expect(noProgress[0].level).toBe("warning"); @@ -431,7 +431,7 @@ describe("AgentSession snapcompact frame dead-end rescue", () => { await triggerMaintenance(); expect(compactSpy).not.toHaveBeenCalled(); - expect(shakeSpy).toHaveBeenCalledWith("elide", expect.anything()); + expect(shakeSpy).toHaveBeenCalledWith("elide", expect.objectContaining({ config: RESCUE_SHAKE_CONFIG })); expect(sessionManager.getBranch().at(-1)?.type).not.toBe("compaction"); }); @@ -455,7 +455,7 @@ describe("AgentSession snapcompact frame dead-end rescue", () => { await triggerMaintenance(); expect(compactSpy).not.toHaveBeenCalled(); - expect(shakeSpy).toHaveBeenCalledWith("elide", expect.anything()); + expect(shakeSpy).toHaveBeenCalledWith("elide", expect.objectContaining({ config: RESCUE_SHAKE_CONFIG })); }); it("leaves an oversized non-archive tail to the elide tiers instead of rescuing the archive", async () => { @@ -492,7 +492,7 @@ describe("AgentSession snapcompact frame dead-end rescue", () => { // The archive was NOT rebuilt; the elide tier got its shot at the tail. expect(compactSpy).not.toHaveBeenCalled(); - expect(shakeSpy).toHaveBeenCalledWith("elide", expect.anything()); + expect(shakeSpy).toHaveBeenCalledWith("elide", expect.objectContaining({ config: RESCUE_SHAKE_CONFIG })); const lastEntry = sessionManager.getBranch().at(-1); expect(lastEntry?.type).not.toBe("compaction"); const noProgress = notices.filter(n => n.source === NOTICE_SOURCE && n.message.includes(NO_PROGRESS_FRAGMENT)); @@ -539,7 +539,7 @@ describe("AgentSession snapcompact frame dead-end rescue", () => { // Text-only model: no frame re-render; existing tiers still run and the // existing dead-end warning is preserved. expect(compactSpy).not.toHaveBeenCalled(); - expect(shakeSpy).toHaveBeenCalledWith("elide", expect.anything()); + expect(shakeSpy).toHaveBeenCalledWith("elide", expect.objectContaining({ config: RESCUE_SHAKE_CONFIG })); const noProgress = notices.filter(n => n.source === NOTICE_SOURCE && n.message.includes(NO_PROGRESS_FRAGMENT)); expect(noProgress.length).toBe(1); }); diff --git a/packages/coding-agent/test/shake.test.ts b/packages/coding-agent/test/shake.test.ts index 69047423c..975ddadf8 100644 --- a/packages/coding-agent/test/shake.test.ts +++ b/packages/coding-agent/test/shake.test.ts @@ -1,7 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; import * as path from "node:path"; import { scheduler } from "node:timers/promises"; -import { Agent, type AgentMessage } from "@oh-my-pi/pi-agent-core"; +import { Agent, type AgentMessage, RESCUE_SHAKE_CONFIG } from "@oh-my-pi/pi-agent-core"; import * as compactionModule from "@oh-my-pi/pi-agent-core/compaction"; import type { AssistantMessage, ImageContent, ToolResultMessage } from "@oh-my-pi/pi-ai"; import { getBundledModel } from "@oh-my-pi/pi-catalog/models"; @@ -151,6 +151,62 @@ describe("AgentSession shake", () => { const result = await session.shake("elide"); expect(result.toolResultsDropped).toBe(0); }); + + /** Seed a user → assistant(read toolCall) → toolResult turn recovering an artifact. */ + function seedArtifactRecoveryResult(text: string, args: Record, details?: unknown): void { + const toolCallId = `call_read_${Math.random().toString(36).slice(2)}`; + sessionManager.appendMessage({ + role: "user", + content: [{ type: "text", text: "recover it" }], + timestamp: Date.now() - 3, + }); + sessionManager.appendMessage({ + role: "assistant", + content: [ + { type: "text", text: "recovering" }, + { type: "toolCall", id: toolCallId, name: "read", arguments: args }, + ], + ...apiInfo, + stopReason: "toolUse", + usage, + timestamp: Date.now() - 2, + }); + sessionManager.appendMessage({ + role: "toolResult", + toolCallId, + toolName: "read", + content: [{ type: "text", text }], + ...(details === undefined ? {} : { details }), + isError: false, + timestamp: Date.now() - 1, + }); + } + + it("rescue config never re-elides artifact recovery reads, by path or by source meta", async () => { + seedArtifactRecoveryResult("R".repeat(4000), { path: "artifact://0" }); + seedArtifactRecoveryResult( + "F".repeat(4000), + { path: "/tmp/artifacts/3.shake.log" }, + { + meta: { source: { type: "internal", value: "artifact://3" } }, + }, + ); + const result = await session.shake("elide", { config: RESCUE_SHAKE_CONFIG }); + expect(result.toolResultsDropped).toBe(0); + const texts = branchToolResults().map(m => (m.content[0] as { text: string }).text); + expect(texts.some(t => t.startsWith("R"))).toBe(true); + expect(texts.some(t => t.startsWith("F"))).toBe(true); + }); + + it("rescue config still elides ordinary oversized results", async () => { + seedHeavyToolResult("B".repeat(4000)); + seedArtifactRecoveryResult("R".repeat(4000), { path: "artifact://0" }); + const result = await session.shake("elide", { config: RESCUE_SHAKE_CONFIG }); + expect(result.toolResultsDropped).toBe(1); + const texts = branchToolResults().map(m => (m.content[0] as { text: string }).text); + expect(texts.some(t => t.startsWith("B"))).toBe(false); + expect(texts.some(t => t.startsWith("R"))).toBe(true); + }); }); describe("auto-shake strategy", () => {