smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix /move session cleanup ownership

Browse Source
This commit is contained in:
can1357
2026-06-27 01:24:28 +02:00
parent bc2d3c2c86
commit 3273d0de68
5 changed files with 105 additions and 106 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/coding-agent/src/modes/controllers/command-controller.ts
+39 -23
View File
@@ -39,7 +39,6 @@ import { computeContextBreakdown, renderContextUsage } from "../../modes/utils/c
import { buildHotkeysMarkdown } from "../../modes/utils/hotkeys-markdown";
import { buildToolsMarkdown } from "../../modes/utils/tools-markdown";
import type { AsyncJobSnapshotItem } from "../../session/agent-session";
import { markMoveSession } from "../../session/agent-session";
import type { AuthStorage, OAuthAccountIdentity } from "../../session/auth-storage";
import type { CompactMode } from "../../session/compact-modes";
import type { NewSessionOptions } from "../../session/session-entries";
@@ -983,35 +982,52 @@ export class CommandController {
}
}
let newSessionFile: string | undefined;
try {
// Create a fresh empty session file in the target directory's session
// folder, then switch to it. The current session is left behind and
// remains resumable via /resume.
const newSessionFile = SessionManager.createEmptySessionFile(resolvedPath);
await this.ctx.session.switchSession(newSessionFile);
markMoveSession(newSessionFile);
await this.ctx.applyCwdChange(resolvedPath);
this.ctx.chatContainer.clear();
this.ctx.pendingMessagesContainer.clear();
this.ctx.compactionQueuedMessages = [];
this.ctx.streamingComponent = undefined;
this.ctx.streamingMessage = undefined;
this.ctx.pendingTools.clear();
this.ctx.statusLine.invalidate();
this.ctx.statusLine.setSessionStartTime(Date.now());
this.ctx.updateEditorTopBorder();
this.ctx.updateEditorBorderColor();
await this.ctx.reloadTodos();
this.ctx.ui.requestRender(true, { clearScrollback: true });
this.ctx.present([
new Spacer(1),
new Text(`${theme.fg("accent", `${theme.status.success} Moved to ${resolvedPath}`)}`, 1, 1),
]);
newSessionFile = SessionManager.createEmptySessionFile(resolvedPath);
const switched = await this.ctx.session.switchSession(newSessionFile);
if (!switched) {
await this.ctx.sessionManager.dropSession(newSessionFile);
return;
}
} catch (err) {
if (newSessionFile) {
try {
await this.ctx.sessionManager.dropSession(newSessionFile);
} catch (dropErr) {
this.ctx.showError(
`Move failed: ${err instanceof Error ? err.message : String(err)}; failed to remove empty session: ${dropErr instanceof Error ? dropErr.message : String(dropErr)}`,
);
return;
}
}
this.ctx.showError(`Move failed: ${err instanceof Error ? err.message : String(err)}`);
return;
}
this.ctx.session.markMovedFromEmptySessionFile(newSessionFile!);
await this.ctx.applyCwdChange(resolvedPath);
this.ctx.chatContainer.clear();
this.ctx.pendingMessagesContainer.clear();
this.ctx.compactionQueuedMessages = [];
this.ctx.streamingComponent = undefined;
this.ctx.streamingMessage = undefined;
this.ctx.pendingTools.clear();
this.ctx.statusLine.invalidate();
this.ctx.statusLine.setSessionStartTime(Date.now());
this.ctx.updateEditorTopBorder();
this.ctx.updateEditorBorderColor();
await this.ctx.reloadTodos();
this.ctx.ui.requestRender(true, { clearScrollback: true });
this.ctx.present([
new Spacer(1),
new Text(`${theme.fg("accent", `${theme.status.success} Moved to ${resolvedPath}`)}`, 1, 1),
]);
}
async handleRenameCommand(title: string): Promise<void> {
packages/coding-agent/src/session/agent-session.ts
+10 -54
View File
@@ -321,7 +321,7 @@ import { formatSessionDumpText } from "./session-dump-format";
import type { BranchSummaryEntry, CompactionEntry, NewSessionOptions } from "./session-entries";
import { EPHEMERAL_MODEL_CHANGE_ROLE } from "./session-entries";
import { formatSessionHistoryMarkdown } from "./session-history-format";
import type { SessionManager } from "./session-manager";
import { cleanupEmptyMoveSession, type SessionManager } from "./session-manager";
import type { ShakeMode, ShakeResult } from "./shake-types";
import { ToolChoiceQueue } from "./tool-choice-queue";
import { classifyUnexpectedStop, isUnexpectedStopCandidate } from "./unexpected-stop-classifier";
@@ -1235,6 +1235,8 @@ export class AgentSession {
#allowAcpAgentInitiatedTurns = false;
/** Per-session memory of allow_always / reject_always decisions for gated tools. */
#acpPermissionDecisions: Map<string, "allow_always" | "reject_always"> = new Map();
/** Session file created by this session's `/move`; removed on dispose if it stayed empty. */
#movedFromEmptySessionFile?: string;
// Compaction state
#compactionAbortController: AbortController | undefined = undefined;
@@ -4641,6 +4643,10 @@ export class AgentSession {
return this.#isDisposed;
}
markMovedFromEmptySessionFile(sessionFile: string): void {
this.#movedFromEmptySessionFile = path.resolve(sessionFile);
}
/**
* Synchronously mark the session as disposing so new work is rejected
* immediately: eval starts throw, queued asides are dropped, and the
@@ -4714,8 +4720,9 @@ export class AgentSession {
await disposeJuliaKernelSessionsByOwner(this.#evalKernelOwnerId);
await shutdownTinyTitleClient();
this.#releasePowerAssertion();
// Clean up empty sessions created by /move so they don't accumulate.
await cleanupEmptyMoveSession(this.sessionManager);
// Clean up an empty session created by this session's /move so it doesn't accumulate.
await cleanupEmptyMoveSession(this.sessionManager, this.#movedFromEmptySessionFile);
this.#movedFromEmptySessionFile = undefined;
await this.sessionManager.close();
// beginDispose() stopped the advisor and captured its recorder close; await
// it so the final advisor turn is flushed before the process may exit.
@@ -13903,54 +13910,3 @@ export class AgentSession {
}
}
// ---------------------------------------------------------------------------
// Empty move-session cleanup
// ---------------------------------------------------------------------------
/**
* Session files created by `/move` that should be cleaned up on shutdown if
* they never received any real user/assistant messages. This prevents empty
* session files from accumulating when the user moves to a directory and quits
* without chatting.
*/
const moveSessionFiles = new Set<string>();
/** Mark a session file as created by `/move` so it can be cleaned up on exit. */
export function markMoveSession(sessionFile: string): void {
moveSessionFiles.add(path.resolve(sessionFile));
}
/** Remove a session file from the move-session cleanup tracking. */
export function unmarkMoveSession(sessionFile: string): void {
moveSessionFiles.delete(path.resolve(sessionFile));
}
/** Check whether a session file was created by `/move` and is still tracked. */
export function isMoveSession(sessionFile: string): boolean {
return moveSessionFiles.has(path.resolve(sessionFile));
}
/**
* If the current session was created by `/move` and contains no real
* user/assistant messages, delete it so empty move sessions don't accumulate.
* Called during `dispose()`.
*/
async function cleanupEmptyMoveSession(sessionManager: SessionManager): Promise<void> {
const sessionFile = sessionManager.getSessionFile();
if (!sessionFile || !moveSessionFiles.has(path.resolve(sessionFile))) return;
const entries = sessionManager.getEntries();
const hasRealMessages = entries.some(
e => e.type === "message" && (e.message.role === "user" || e.message.role === "assistant"),
);
if (hasRealMessages) {
// The session has real content — keep it and stop tracking it as a move session.
moveSessionFiles.delete(path.resolve(sessionFile));
return;
}
try {
await sessionManager.dropSession(sessionFile);
} catch (err) {
logger.warn("Failed to clean up empty move session", { sessionFile, error: String(err) });
}
moveSessionFiles.delete(path.resolve(sessionFile));
}
packages/coding-agent/src/session/session-manager.ts
+23
View File
@@ -1771,3 +1771,26 @@ export class SessionManager {
return listAllSessions(storage);
}
}
/**
* If the current session was created by `/move` and contains no real
* user/assistant messages, delete it so empty move sessions don't accumulate.
*/
export async function cleanupEmptyMoveSession(
sessionManager: SessionManager,
movedFromEmptySessionFile: string | undefined,
): Promise<void> {
const sessionFile = sessionManager.getSessionFile();
if (!sessionFile || !movedFromEmptySessionFile) return;
if (path.resolve(sessionFile) !== path.resolve(movedFromEmptySessionFile)) return;
const entries = sessionManager.getEntries();
const hasRealMessages = entries.some(
e => e.type === "message" && (e.message.role === "user" || e.message.role === "assistant"),
);
if (hasRealMessages) return;
try {
await sessionManager.dropSession(sessionFile);
} catch (err) {
logger.warn("Failed to clean up empty move session", { sessionFile, error: String(err) });
}
}
packages/coding-agent/src/slash-commands/builtin-registry.ts
+19 -10
View File
@@ -26,7 +26,6 @@ import { describeLoopLimitRuntime } from "../modes/loop-limit";
import { theme } from "../modes/theme/theme";
import type { InteractiveModeContext } from "../modes/types";
import type { AgentSession, FreshSessionResult } from "../session/agent-session";
import { markMoveSession } from "../session/agent-session";
import { COMPACT_MODES, parseCompactArgs } from "../session/compact-modes";
import { resolveResumableSession } from "../session/session-listing";
import { SessionManager } from "../session/session-manager";
@@ -1607,20 +1606,30 @@ const BUILTIN_SLASH_COMMAND_REGISTRY: ReadonlyArray<SlashCommandSpec> = [
return usage(`Not a directory: ${resolvedPath}`, runtime);
}
} catch {
// Directory doesn't exist — create it (no interactive confirm in ACP).
try {
await fs.mkdir(resolvedPath, { recursive: true });
} catch (err) {
return usage(`Failed to create directory: ${errorMessage(err)}`, runtime);
}
return usage(`Directory does not exist: ${resolvedPath}`, runtime);
}
let newSessionFile: string | undefined;
try {
const newSessionFile = SessionManager.createEmptySessionFile(resolvedPath);
await runtime.session.switchSession(newSessionFile);
markMoveSession(newSessionFile);
newSessionFile = SessionManager.createEmptySessionFile(resolvedPath);
const switched = await runtime.session.switchSession(newSessionFile);
if (!switched) {
await runtime.sessionManager.dropSession(newSessionFile);
return usage("Move cancelled.", runtime);
}
} catch (err) {
if (newSessionFile) {
try {
await runtime.sessionManager.dropSession(newSessionFile);
} catch (dropErr) {
return usage(
`Move failed: ${errorMessage(err)}; failed to remove empty session: ${errorMessage(dropErr)}`,
runtime,
);
}
}
return usage(`Move failed: ${errorMessage(err)}`, runtime);
}
runtime.session.markMovedFromEmptySessionFile(newSessionFile!);
setProjectDir(resolvedPath);
// Reload plugin/capability caches so the next prompt sees commands and
// capabilities scoped to the new cwd.
packages/coding-agent/test/session-manager/move-session-cleanup.test.ts
+14 -19
View File
@@ -3,8 +3,7 @@ import * as fs from "node:fs";
import * as fsp from "node:fs/promises";
import * as os from "node:os";
import * as path from "node:path";
import { isMoveSession, markMoveSession, unmarkMoveSession } from "@oh-my-pi/pi-coding-agent/session/agent-session";
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
import { cleanupEmptyMoveSession, SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
import { getConfigRootDir, setAgentDir } from "@oh-my-pi/pi-utils";
import { makeAssistantMessage } from "./helpers";
@@ -31,22 +30,21 @@ describe("move-session cleanup tracking", () => {
await fsp.rm(testAgentDir, { recursive: true, force: true });
});
it("markMoveSession / isMoveSession / unmarkMoveSession round-trip", () => {
const file = path.resolve(cwd, "session.jsonl");
expect(isMoveSession(file)).toBe(false);
markMoveSession(file);
expect(isMoveSession(file)).toBe(true);
unmarkMoveSession(file);
expect(isMoveSession(file)).toBe(false);
it("does not delete an empty session file without the owning move marker", async () => {
const file = SessionManager.createEmptySessionFile(cwd);
const manager = SessionManager.create(cwd);
await manager.setSessionFile(file);
await cleanupEmptyMoveSession(manager, undefined);
expect(fs.existsSync(file)).toBe(true);
await manager.dropSession(file);
});
it("createEmptySessionFile + markMoveSession + dispose deletes empty session file", async () => {
it("createEmptySessionFile + cleanupEmptyMoveSession deletes an empty move session file", async () => {
const file = SessionManager.createEmptySessionFile(cwd);
expect(fs.existsSync(file)).toBe(true);
markMoveSession(file);
// Simulate what dispose() does: load the session, then call cleanupEmptyMoveSession.
// We test the contract: an empty (header-only) move session file is deleted.
const manager = SessionManager.create(cwd);
await manager.setSessionFile(file);
@@ -57,16 +55,12 @@ describe("move-session cleanup tracking", () => {
);
expect(hasRealMessages).toBe(false);
await manager.dropSession(file);
await cleanupEmptyMoveSession(manager, file);
expect(fs.existsSync(file)).toBe(false);
expect(isMoveSession(file)).toBe(true); // tracking not auto-cleared by dropSession
unmarkMoveSession(file);
expect(isMoveSession(file)).toBe(false);
});
it("a move session that received real messages is NOT deleted", async () => {
const file = SessionManager.createEmptySessionFile(cwd);
markMoveSession(file);
const manager = SessionManager.create(cwd);
await manager.setSessionFile(file);
@@ -81,7 +75,8 @@ describe("move-session cleanup tracking", () => {
);
expect(hasRealMessages).toBe(true);
await cleanupEmptyMoveSession(manager, file);
expect(fs.existsSync(file)).toBe(true);
unmarkMoveSession(file);
await manager.dropSession(file);
});
});