diff --git a/packages/coding-agent/src/modes/controllers/command-controller.ts b/packages/coding-agent/src/modes/controllers/command-controller.ts index e40c9eaba..67f0463aa 100644 --- a/packages/coding-agent/src/modes/controllers/command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/command-controller.ts @@ -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 { diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 24ab18723..f000317b7 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -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 = 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(); - -/** 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 { - 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)); -} diff --git a/packages/coding-agent/src/session/session-manager.ts b/packages/coding-agent/src/session/session-manager.ts index 38878cbf6..041cedafa 100644 --- a/packages/coding-agent/src/session/session-manager.ts +++ b/packages/coding-agent/src/session/session-manager.ts @@ -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 { + 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) }); + } +} diff --git a/packages/coding-agent/src/slash-commands/builtin-registry.ts b/packages/coding-agent/src/slash-commands/builtin-registry.ts index 44fd73ab6..4d4c2c5a0 100644 --- a/packages/coding-agent/src/slash-commands/builtin-registry.ts +++ b/packages/coding-agent/src/slash-commands/builtin-registry.ts @@ -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 = [ 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. diff --git a/packages/coding-agent/test/session-manager/move-session-cleanup.test.ts b/packages/coding-agent/test/session-manager/move-session-cleanup.test.ts index 12730f309..54da3eb6e 100644 --- a/packages/coding-agent/test/session-manager/move-session-cleanup.test.ts +++ b/packages/coding-agent/test/session-manager/move-session-cleanup.test.ts @@ -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); }); });