Merge PR #7327: fix(compaction): stop shake from re-eliding artifact recovery reads (@jwmacd)

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
 

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;
 

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,

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
 

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";

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);
 	});

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<string, unknown>, 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", () => {