merge #3847: skip rebuilding previous session context on different-session switch
This commit is contained in:
@@ -12,6 +12,9 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed snapcompact preflight to use the same font-aware renderability probe as compaction, including prior preserved archive text, so CJK history remains renderable through per-glyph Silver fallback across repeated compactions.
|
||||
### 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
|
||||
|
||||
|
||||
@@ -13171,7 +13171,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.
|
||||
@@ -13210,7 +13218,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 });
|
||||
@@ -13326,7 +13334,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) {
|
||||
|
||||
@@ -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 },
|
||||
]);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user