diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c68409f79..c1d43ea68 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed in-TUI `/resume` materializing the previous session's display context (snapcompact archives + OpenAI Responses replay payloads) before switching files, which could exhaust memory on huge pre-fix sessions. `AgentSession.switchSession` now only snapshots the prior context for same-session reloads, where it is needed for rollback comparison. ([#3846](https://github.com/can1357/oh-my-pi/issues/3846)) + ## [16.2.6] - 2026-06-29 ### Changed diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 049d8ac03..bc23e2dd7 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -13150,7 +13150,15 @@ export class AgentSession { // Flush pending writes before switching so restore snapshots reflect committed state. await this.sessionManager.flush(); const previousSessionState = this.sessionManager.captureState(); - const previousSessionContext = this.buildDisplaySessionContext(); + // Only same-session reloads compare against the prior context to detect + // rollback edits (`#didSessionMessagesChange` below). Building it for a + // different-session switch is a pure waste — and on huge pre-fix sessions + // it materializes every persisted snapcompact frame plus the + // `openaiRemoteCompaction.replacementHistory` payload into messages, + // blowing the heap before the new session even loads (issue #3846). The + // error-recovery path rebuilds the context on demand from the restored + // state instead. + const previousSessionContext = switchingToDifferentSession ? undefined : this.buildDisplaySessionContext(); // switchSession replaces these arrays wholesale during load/rollback, so retaining // the existing message objects is sufficient and avoids structured-clone failures for // extension/custom metadata that is valid to persist but not cloneable. @@ -13189,7 +13197,7 @@ export class AgentSession { const sessionContext = this.buildDisplaySessionContext(); const didReloadConversationChange = - !switchingToDifferentSession && + previousSessionContext !== undefined && this.#didSessionMessagesChange(previousSessionContext.messages, sessionContext.messages); const fallbackSelectedMCPToolNames = this.#getSessionDefaultSelectedMCPToolNames(sessionPath); await this.#restoreMCPSelectionsForSessionContext(sessionContext, { fallbackSelectedMCPToolNames }); @@ -13305,7 +13313,12 @@ export class AgentSession { this.#rekeyMnemopiMemoryForCurrentSessionId(); let restoreMcpError: unknown; try { - await this.#restoreMCPSelectionsForSessionContext(previousSessionContext, { + // `previousSessionContext` was skipped on different-session switches to + // avoid materializing the previous session's heavy compaction payload + // in the success path; rebuild it here on demand from the restored + // state so MCP selection restoration still has its inputs. + const mcpRestoreContext = previousSessionContext ?? this.buildDisplaySessionContext(); + await this.#restoreMCPSelectionsForSessionContext(mcpRestoreContext, { fallbackSelectedMCPToolNames: previousFallbackSelectedMCPToolNames, }); } catch (mcpError) { diff --git a/packages/coding-agent/test/agent-session-switch-prev-context.test.ts b/packages/coding-agent/test/agent-session-switch-prev-context.test.ts new file mode 100644 index 000000000..4fa599d8e --- /dev/null +++ b/packages/coding-agent/test/agent-session-switch-prev-context.test.ts @@ -0,0 +1,161 @@ +import { afterAll, afterEach, beforeAll, describe, expect, it } from "bun:test"; +import * as path from "node:path"; +import { Agent } from "@oh-my-pi/pi-agent-core"; +import type { Model } 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 } from "@oh-my-pi/pi-coding-agent/session/agent-session"; +import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; +import type { BuildSessionContextOptions, SessionContext } from "@oh-my-pi/pi-coding-agent/session/session-context"; +import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager"; +import { TempDir } from "@oh-my-pi/pi-utils"; + +/** + * Regression for issue #3846: in-TUI `/resume` rebuilt the *previous* + * session's display context before switching files. That call expands persisted + * snapcompact archives and `openaiRemoteCompaction.replacementHistory` payloads + * into messages, which can OOM on huge pre-fix sessions even though the loader + * itself streams. The previous context is only needed for same-session reloads + * (where `#didSessionMessagesChange` compares against the freshly rebuilt one); + * different-session switches MUST skip that work. + */ +describe("AgentSession.switchSession previous-context build", () => { + let sharedDir: TempDir; + let authStorage: AuthStorage; + let modelRegistry: ModelRegistry; + let model: Model; + const tempDirs: TempDir[] = []; + const sessions: AgentSession[] = []; + + beforeAll(async () => { + sharedDir = TempDir.createSync("@pi-switch-prev-ctx-shared-"); + authStorage = await AuthStorage.create(path.join(sharedDir.path(), "testauth.db")); + authStorage.setRuntimeApiKey("anthropic", "test-key"); + modelRegistry = new ModelRegistry(authStorage); + const bundled = getBundledModel("anthropic", "claude-sonnet-4-5"); + if (!bundled) throw new Error("Expected built-in anthropic model to exist"); + model = bundled; + }); + + afterAll(async () => { + authStorage.close(); + try { + await sharedDir.remove(); + } catch {} + }); + + afterEach(async () => { + while (sessions.length > 0) { + await sessions.pop()?.dispose(); + } + for (const dir of tempDirs.splice(0)) { + try { + await dir.remove(); + } catch {} + } + }); + + function buildSession(tempDir: TempDir): { session: AgentSession; sessionManager: SessionManager } { + const sessionManager = SessionManager.create(tempDir.path(), tempDir.path()); + const agent = new Agent({ + initialState: { + model, + systemPrompt: ["Test"], + tools: [], + messages: [], + }, + }); + const session = new AgentSession({ + agent, + sessionManager, + settings: Settings.isolated({ "compaction.enabled": false }), + modelRegistry, + }); + sessions.push(session); + return { session, sessionManager }; + } + + /** Wrap `sessionManager.buildSessionContext` so each call's caller-visible + * state (the manager's currently-loaded session file) is recorded in + * invocation order. The constructor itself calls `buildSessionContext` + * once; spying *after* construction means only switchSession-driven calls + * are observed. */ + function instrumentBuildSessionContext(sessionManager: SessionManager): { + calls: Array<{ sessionFile: string | undefined; transcript: boolean | undefined }>; + restore: () => void; + } { + const calls: Array<{ sessionFile: string | undefined; transcript: boolean | undefined }> = []; + const original = sessionManager.buildSessionContext.bind(sessionManager); + const patched = (options?: BuildSessionContextOptions): SessionContext => { + calls.push({ sessionFile: sessionManager.getSessionFile(), transcript: options?.transcript }); + return original(options); + }; + sessionManager.buildSessionContext = patched as SessionManager["buildSessionContext"]; + return { + calls, + restore: () => { + sessionManager.buildSessionContext = original; + }, + }; + } + + it("skips building the previous display context when switching to a different session", async () => { + const tempDir = TempDir.createSync("@pi-switch-prev-ctx-different-"); + tempDirs.push(tempDir); + + const { session, sessionManager } = buildSession(tempDir); + sessionManager.appendMessage({ role: "user", content: "previous", timestamp: 1 }); + await sessionManager.flush(); + const previousSessionFile = sessionManager.getSessionFile(); + expect(previousSessionFile).toBeString(); + + const otherManager = SessionManager.create(tempDir.path(), tempDir.path()); + otherManager.appendMessage({ role: "user", content: "target", timestamp: 2 }); + await otherManager.flush(); + const targetSessionFile = otherManager.getSessionFile(); + expect(targetSessionFile).toBeString(); + expect(targetSessionFile).not.toBe(previousSessionFile); + await otherManager.close(); + + const { calls, restore } = instrumentBuildSessionContext(sessionManager); + try { + const switched = await session.switchSession(targetSessionFile!); + expect(switched).toBe(true); + expect(session.sessionFile).toBe(targetSessionFile); + } finally { + restore(); + } + + // The previous session's display context MUST NOT be materialized. Only + // the new target context (post-`setSessionFile`) should be built. + expect(calls).toEqual([{ sessionFile: targetSessionFile!, transcript: undefined }]); + }); + + it("builds the previous display context for same-session reloads", async () => { + const tempDir = TempDir.createSync("@pi-switch-prev-ctx-reload-"); + tempDirs.push(tempDir); + + const { session, sessionManager } = buildSession(tempDir); + sessionManager.appendMessage({ role: "user", content: "current", timestamp: 1 }); + await sessionManager.flush(); + const sessionFile = sessionManager.getSessionFile(); + expect(sessionFile).toBeString(); + + const { calls, restore } = instrumentBuildSessionContext(sessionManager); + try { + const switched = await session.switchSession(sessionFile!); + expect(switched).toBe(true); + expect(session.sessionFile).toBe(sessionFile); + } finally { + restore(); + } + + // Same-session reload must snapshot the pre-reload context so + // `#didSessionMessagesChange` can detect rollback edits. + expect(calls).toEqual([ + { sessionFile: sessionFile!, transcript: undefined }, + { sessionFile: sessionFile!, transcript: undefined }, + ]); + }); +});