Merge PR #8283: fix(coding-agent): skip idle mid-turn persistence waits (@ethancawse)

docs/extensions.md

@@ -251,7 +251,7 @@ Cancelable pre-events:
 - `agent_start` / `agent_end` — agent loop lifecycle notification; `agent_end` remains notification-only
 - `session_stop` — main-session stop hook, awaited before settle; may continue with `{ continue: true, additionalContext }` or `{ decision: "block", reason }`; capped at 8 consecutive continuations and never fires for task/subagent sessions
 - `turn_start` / `turn_end`
-- `message_start` / `message_update` / `message_end`
+- `message_start` / `message_update` / `message_end` — lifecycle notifications; `message_end` receives a detached message snapshot, so use `tool_result` or `context` when an extension needs to change provider context
 
 ### Tool lifecycle
 

packages/coding-agent/CHANGELOG.md

@@ -42,6 +42,9 @@
 - Fixed agent-facing prompts mentioning tools that may be absent from the session catalog: `todo`/`grep` workflow guidance in the system prompt, `glob`/`read`/`edit` drill-in hints in the project prompt, `ask` directives in plan mode, and the orchestrate notice's `task`/`edit`/`write`/`lsp`/`bash`/`todo` budget are now gated on tool availability.
 - Fixed a `/skill:<name>` token embedded in a `/plan` or `/vibe` inline prompt being sent to the agent as literal text instead of loading the skill; mode-command inline prompts now dispatch skill invocations through the same custom-message path as the editor submit flow ([#8137](https://github.com/can1357/oh-my-pi/issues/8137)).
 - Fixed Codex reset fireworks triggering on ordinary weekly-usage decreases when the provider had not advanced the quota reset deadline.
+### Fixed
+
+- Fixed below-threshold tool turns waiting for asynchronous session persistence when no mid-run compaction will run, while preserving journal writes when a `message_end` listener fails and isolating notification-only handler payloads from late context mutations ([#8283](https://github.com/can1357/oh-my-pi/pull/8283) by [@ethancawse](https://github.com/ethancawse)).
 
 ## [17.2.15] - 2026-08-12
 

packages/coding-agent/src/extensibility/extensions/types.ts

@@ -733,7 +733,10 @@ export interface MessageUpdateEvent {
 	assistantMessageEvent: AssistantMessageEvent;
 }
 
-/** Fired when a message ends */
+/**
+ * Fired when a message ends. Notification-only: the message is a detached
+ * snapshot, so in-place changes do not rewrite agent or provider context.
+ */
 export interface MessageEndEvent {
 	type: "message_end";
 	message: AgentMessage;

packages/coding-agent/src/session/agent-session.ts

@@ -423,6 +423,35 @@ type SetSessionNameWithTrigger = (
 const kPersistedSessionEntryId = Symbol("persistedSessionEntryId");
 type PersistedAssistantMessage = AssistantMessage & { [kPersistedSessionEntryId]?: string };
 
+/**
+ * Clone one top-level notification field without ever returning an object owned
+ * by the live session. Most values take the lossless structured-clone path. If
+ * a third-party metadata object contains functions or other unsupported values,
+ * JSON sanitization drops those values; a cyclic/non-JSON value finally degrades
+ * to a descriptive string rather than retaining a shared mutable reference.
+ */
+function cloneMessageEndNotificationField(value: unknown): unknown {
+	try {
+		return structuredClone(value);
+	} catch {}
+	try {
+		const json = JSON.stringify(value);
+		if (json !== undefined) return JSON.parse(json) as unknown;
+	} catch {}
+	return String(value);
+}
+
+/** Build a detached, notification-only snapshot of an AgentMessage. */
+function cloneMessageEndNotification(message: AgentMessage): AgentMessage {
+	const snapshot: Record<PropertyKey, unknown> = {};
+	for (const key of Reflect.ownKeys(message)) {
+		const descriptor = Object.getOwnPropertyDescriptor(message, key);
+		if (!descriptor?.enumerable) continue;
+		snapshot[key] = cloneMessageEndNotificationField(Reflect.get(message, key));
+	}
+	return snapshot as unknown as AgentMessage;
+}
+
 export class AgentSession {
 	readonly agent: Agent;
 	readonly sessionManager: SessionManager;
@@ -2297,6 +2326,27 @@ export class AgentSession {
 		}
 	}
 
+	#persistMessageEnd(message: AgentMessage): void {
+		if (message.role === "hookMessage" || message.role === "custom") {
+			// Prewalk's plan nudge is a one-run steering instruction. Persisting it would
+			// resurrect the consumed prompt on resume, fork, or any context rebuild.
+			if (!isPrewalkPlanNudge(message)) {
+				this.sessionManager.appendCustomMessageEntry(
+					message.customType,
+					message.content,
+					message.display,
+					message.details,
+					message.attribution ?? "agent",
+				);
+			}
+			if (message.role === "custom" && message.customType === "ttsr-injection") {
+				this.#ttsr.markInjectedFromDetails(message.details);
+			}
+			return;
+		}
+		this.#persistSessionMessageIfMissing(message);
+	}
+
 	/**
 	 * On a user-interrupted (`Esc`) abort, copy the trailing thinking run into a
 	 * hidden `display: false` continuity message for the next turn WITHOUT
@@ -2469,7 +2519,17 @@ export class AgentSession {
 			try {
 				await this.#emitSessionEvent(displayEvent);
 			} catch (error) {
-				messageEndPersistence?.release();
+				if (event.type === "message_end") {
+					const persistMessageEnd = () => this.#persistMessageEnd(event.message);
+					try {
+						if (messageEndPersistence) await messageEndPersistence.persist(persistMessageEnd);
+						else persistMessageEnd();
+					} catch (persistenceError) {
+						logger.warn("Failed to persist message after session event emission failed", {
+							error: String(persistenceError),
+						});
+					}
+				}
 				throw error;
 			}
 		}
@@ -2527,27 +2587,7 @@ export class AgentSession {
 
 		// Handle session persistence
 		if (event.type === "message_end") {
-			const persistMessageEnd = () => {
-				// Check if this is a hook/custom message
-				if (event.message.role === "hookMessage" || event.message.role === "custom") {
-					// Prewalk's plan nudge is a one-run steering instruction. Persisting it would
-					// resurrect the consumed prompt on resume, fork, or any context rebuild.
-					if (!isPrewalkPlanNudge(event.message)) {
-						this.sessionManager.appendCustomMessageEntry(
-							event.message.customType,
-							event.message.content,
-							event.message.display,
-							event.message.details,
-							event.message.attribution ?? "agent",
-						);
-					}
-					if (event.message.role === "custom" && event.message.customType === "ttsr-injection") {
-						this.#ttsr.markInjectedFromDetails(event.message.details);
-					}
-				} else {
-					this.#persistSessionMessageIfMissing(event.message);
-				}
-			};
+			const persistMessageEnd = () => this.#persistMessageEnd(event.message);
 			if (messageEndPersistence) {
 				await messageEndPersistence.persist(persistMessageEnd);
 			} else {
@@ -3426,9 +3466,16 @@ export class AgentSession {
 			};
 			await this.#extensionRunner.emit(extensionEvent);
 		} else if (event.type === "message_end") {
+			// `message_end` is a notification, not a context-rewrite hook. Detach its
+			// payload from agent-owned history so an async observer that mutates the
+			// event after an `await` cannot race mid-run maintenance and enlarge (or
+			// otherwise rewrite) the next provider request after its threshold check.
+			// Explicit `tool_result` / `context` hooks remain the supported mutation
+			// surfaces. Third-party metadata that is not structured-cloneable is
+			// sanitized field-by-field without retaining nested live references.
 			const extensionEvent: MessageEndEvent = {
 				type: "message_end",
-				message: event.message,
+				message: cloneMessageEndNotification(event.message),
 			};
 			await this.#extensionRunner.emit(extensionEvent);
 		} else if (event.type === "tool_execution_start") {

packages/coding-agent/src/session/session-maintenance.ts

@@ -1097,6 +1097,16 @@ export class SessionMaintenance {
 			.find((message): message is AssistantMessage => message.role === "assistant");
 		if (!lastAssistant || lastAssistant.stopReason === "aborted" || lastAssistant.stopReason === "error") return;
 
+		// Decide from the live agent context before waiting for the asynchronous
+		// session journal. The persistence barrier is required only when maintenance
+		// will actually rewrite history; awaiting it on every ordinary tool turn lets
+		// a slow message_end listener leave the TUI "generating" with no provider
+		// request or tool running.
+		const billedContextTokens = calculateContextTokens(lastAssistant.usage);
+		const storedContextTokens = this.#estimateStoredContextTokens();
+		const contextTokens = compactionContextTokens(billedContextTokens, storedContextTokens);
+		if (!shouldCompact(contextTokens, contextWindow, compactionSettings)) return;
+
 		if (!(await this.#host.persistTurnMessagesForMidRunCompaction(context))) return;
 		if (this.#midTurnCompactionDeadEnds.has(activeMessages)) {
 			// A prior boundary already ran the dead-end rescue and could not reduce
@@ -1118,11 +1128,6 @@ export class SessionMaintenance {
 			this.#midTurnDeadEndPendingPrePrompt = false;
 		}
 
-		const billedContextTokens = calculateContextTokens(lastAssistant.usage);
-		const storedContextTokens = this.#estimateStoredContextTokens();
-		const contextTokens = compactionContextTokens(billedContextTokens, storedContextTokens);
-		if (!shouldCompact(contextTokens, contextWindow, compactionSettings)) return;
-
 		// Promote to a larger-context sibling before compacting, mirroring the
 		// pre-prompt (runPrePromptCompactionIfNeeded) and post-turn threshold
 		// (checkCompaction) paths. Without this, a long mid-turn tool loop that

packages/coding-agent/test/agent-session-goal-midrun-compaction.test.ts

@@ -78,7 +78,12 @@ describe("AgentSession mid-run threshold compaction", () => {
 
 	async function createHarness(
 		settingsOverride: Record<string, unknown> = {},
-		options: { extensionRunner?: ExtensionRunner } = {},
+		options: {
+			extensionRunner?: ExtensionRunner;
+			onProviderCall?: (index: number) => void;
+			configureAgent?: (agent: Agent) => void;
+			toolResultDetails?: unknown;
+		} = {},
 	): Promise<{
 		session: AgentSession;
 		observedContexts: string[][];
@@ -108,7 +113,10 @@ describe("AgentSession mid-run threshold compaction", () => {
 			label: "Bash",
 			description: "Mock bash tool",
 			parameters: type({}),
-			execute: async () => ({ content: [{ type: "text" as const, text: "tool output" }] }),
+			execute: async () => ({
+				content: [{ type: "text" as const, text: "tool output" }],
+				...(options.toolResultDetails === undefined ? {} : { details: options.toolResultDetails }),
+			}),
 		};
 
 		let call = 0;
@@ -118,6 +126,7 @@ describe("AgentSession mid-run threshold compaction", () => {
 			convertToLlm,
 			streamFn: (_model, context) => {
 				const index = call++;
+				options.onProviderCall?.(index);
 				observedContexts.push(context.messages.map(message => JSON.stringify(message)));
 				const stream = new AssistantMessageEventStream();
 				const isToolTurn = index === 0;
@@ -151,6 +160,7 @@ describe("AgentSession mid-run threshold compaction", () => {
 				return stream;
 			},
 		});
+		options.configureAgent?.(agent);
 
 		const session = new AgentSession({
 			agent,
@@ -212,6 +222,162 @@ describe("AgentSession mid-run threshold compaction", () => {
 		expect(observedContexts[1].join("\n")).toContain("HANDOFF-MID-RUN-COMPACTED-IN-PLACE");
 	});
 
+	it("does not wait for message persistence below the mid-run threshold", async () => {
+		const releaseMessageEnd = Promise.withResolvers<void>();
+		const messageEndEntered = Promise.withResolvers<void>();
+		const nextProviderCall = Promise.withResolvers<void>();
+		const extensionRunner = {
+			hasHandlers: vi.fn((eventType: string) => eventType === "message_end"),
+			emitBeforeAgentStart: vi.fn(async () => undefined),
+			emit: vi.fn(async (event: { type: string; message?: AgentMessage }) => {
+				if (
+					event.type === "message_end" &&
+					event.message?.role === "assistant" &&
+					event.message.stopReason === "toolUse"
+				) {
+					messageEndEntered.resolve();
+					await releaseMessageEnd.promise;
+				}
+			}),
+		} as unknown as ExtensionRunner;
+		const { session } = await createHarness(
+			{ "compaction.thresholdTokens": 100_000 },
+			{
+				extensionRunner,
+				onProviderCall: index => {
+					if (index === 1) nextProviderCall.resolve();
+				},
+			},
+		);
+		const compactSpy = mockCompaction("SHOULD-NOT-RUN");
+
+		const prompt = session.prompt("work below the maintenance threshold");
+		const messageEndOutcome = await Promise.race([
+			messageEndEntered.promise.then(() => "entered" as const),
+			Bun.sleep(2_000).then(() => "blocked" as const),
+		]);
+		const providerOutcome =
+			messageEndOutcome === "entered"
+				? await Promise.race([
+						nextProviderCall.promise.then(() => "dispatched" as const),
+						Bun.sleep(2_000).then(() => "blocked" as const),
+					])
+				: "blocked";
+		releaseMessageEnd.resolve();
+		const promptOutcome = await Promise.race([
+			prompt.then(() => "settled" as const),
+			Bun.sleep(2_000).then(() => "blocked" as const),
+		]);
+
+		expect(messageEndOutcome).toBe("entered");
+		expect(providerOutcome).toBe("dispatched");
+		expect(promptOutcome).toBe("settled");
+		expect(compactSpy).not.toHaveBeenCalled();
+	});
+
+	it("persists a tool result when its message_end listener rejects below the mid-run threshold", async () => {
+		let rejected = false;
+		const extensionRunner = {
+			hasHandlers: vi.fn((eventType: string) => eventType === "message_end"),
+			emitBeforeAgentStart: vi.fn(async () => undefined),
+			emit: vi.fn(async (event: { type: string; message?: AgentMessage }) => {
+				if (!rejected && event.type === "message_end" && event.message?.role === "toolResult") {
+					rejected = true;
+					throw new Error("intentional message_end failure");
+				}
+			}),
+		} as unknown as ExtensionRunner;
+		const { session, sessionManager } = await createHarness(
+			{ "compaction.thresholdTokens": 100_000 },
+			{ extensionRunner },
+		);
+
+		await session.prompt("work below the maintenance threshold");
+
+		const persistedToolResults = sessionManager
+			.getBranch()
+			.filter(entry => entry.type === "message" && entry.message.role === "toolResult");
+		expect(rejected).toBe(true);
+		expect(persistedToolResults).toHaveLength(1);
+	});
+
+	it("isolates late message_end mutations from the next provider request", async () => {
+		const releaseMutation = Promise.withResolvers<void>();
+		const mutationApplied = Promise.withResolvers<void>();
+		const toolResultHookEntered = Promise.withResolvers<void>();
+		const secondModelCallEntered = Promise.withResolvers<void>();
+		const releaseSecondModelCall = Promise.withResolvers<void>();
+		const mutationMarker = `LATE-MESSAGE-END-MUTATION-${"x".repeat(500_000)}`;
+		const liveDetails = {
+			nested: { state: "original" },
+			nonCloneable: () => "third-party callback",
+		};
+		let interceptedToolResult = false;
+		const extensionRunner = {
+			hasHandlers: vi.fn((eventType: string) => eventType === "message_end"),
+			emitBeforeAgentStart: vi.fn(async () => undefined),
+			emit: vi.fn(async (event: { type: string; message?: AgentMessage }) => {
+				if (interceptedToolResult || event.type !== "message_end" || event.message?.role !== "toolResult") return;
+				interceptedToolResult = true;
+				toolResultHookEntered.resolve();
+				await releaseMutation.promise;
+				event.message.content = [{ type: "text", text: mutationMarker }];
+				(event.message.details as { nested: { state: string } }).nested.state = "mutated";
+				mutationApplied.resolve();
+			}),
+		} as unknown as ExtensionRunner;
+		let modelCall = 0;
+		const { session, observedContexts } = await createHarness(
+			{ "compaction.thresholdTokens": 100_000 },
+			{
+				extensionRunner,
+				toolResultDetails: liveDetails,
+				configureAgent: agent => {
+					agent.addBeforeModelCallHook(async () => {
+						if (modelCall++ !== 1) return;
+						secondModelCallEntered.resolve();
+						await releaseSecondModelCall.promise;
+					});
+				},
+			},
+		);
+
+		const prompt = session.prompt("keep notification mutations out of live context");
+		const toolResultHookOutcome = await Promise.race([
+			toolResultHookEntered.promise.then(() => "entered" as const),
+			Bun.sleep(2_000).then(() => "blocked" as const),
+		]);
+		const secondModelCallOutcome =
+			toolResultHookOutcome === "entered"
+				? await Promise.race([
+						secondModelCallEntered.promise.then(() => "dispatched" as const),
+						Bun.sleep(2_000).then(() => "blocked" as const),
+					])
+				: "blocked";
+		releaseMutation.resolve();
+		const mutationOutcome = await Promise.race([
+			mutationApplied.promise.then(() => "applied" as const),
+			Bun.sleep(2_000).then(() => "blocked" as const),
+		]);
+		releaseSecondModelCall.resolve();
+		const promptOutcome = await Promise.race([
+			prompt.then(() => "settled" as const),
+			Bun.sleep(2_000).then(() => "blocked" as const),
+		]);
+
+		expect(toolResultHookOutcome).toBe("entered");
+		expect(secondModelCallOutcome).toBe("dispatched");
+		expect(mutationOutcome).toBe("applied");
+		expect(promptOutcome).toBe("settled");
+		expect(observedContexts).toHaveLength(2);
+		expect(observedContexts[1].join("\n")).not.toContain("LATE-MESSAGE-END-MUTATION");
+		expect(JSON.stringify(session.messages)).not.toContain("LATE-MESSAGE-END-MUTATION");
+		expect(liveDetails.nested.state).toBe("original");
+		const storedToolResult = session.messages.find(message => message.role === "toolResult");
+		if (!storedToolResult) throw new Error("Expected a stored tool result");
+		expect((storedToolResult.details as { nested: { state: string } }).nested.state).toBe("original");
+	});
+
 	it("preserves the just-finished tool turn when message_end hooks are still pending", async () => {
 		const releaseMessageEnd = Promise.withResolvers<void>();
 		const messageEndEntered = Promise.withResolvers<void>();
@@ -277,7 +443,7 @@ describe("AgentSession mid-run threshold compaction", () => {
 		expect(persistedToolTurnRoles).toEqual(["assistant", "toolResult"]);
 	});
 
-	it("treats same-key assistant content variants as persisted before mid-run compaction", async () => {
+	it("keeps synchronous message_end mutations notification-local during mid-run compaction", async () => {
 		const extensionRuntime = new ExtensionRuntime();
 		const extension = await loadExtensionFromFactory(
 			pi => {
@@ -308,6 +474,7 @@ describe("AgentSession mid-run threshold compaction", () => {
 		expect(compactSpy).toHaveBeenCalledTimes(1);
 		expect(observedContexts.length).toBeGreaterThanOrEqual(2);
 		expect(observedContexts[1].join("\n")).toContain("MID-RUN-COMPACTED-WITH-CONTENT-VARIANT");
+		expect(JSON.stringify(session.messages)).not.toContain("display-variant");
 	});
 
 	it("does not compact mid-run outside goal mode when disabled", async () => {