fix session cleanup

.changeset/thin-heads-train.md

@@ -0,0 +1,6 @@
+---
+"@kilocode/cli": patch
+"kilo-code": patch
+---
+
+fixes session cleanup race conditions

cli/src/commands/__tests__/session.test.ts

@@ -687,7 +687,7 @@ describe("sessionCommand", () => {
 			await sessionCommand.handler(mockContext)
 
 			expect(SessionManager.init).toHaveBeenCalled()
-			expect(mockSessionManager.renameSession).toHaveBeenCalledWith("My New Session Name")
+			expect(mockSessionManager.renameSession).toHaveBeenCalledWith("test-session-123", "My New Session Name")
 			expect(mockContext.addMessage).toHaveBeenCalledTimes(1)
 			const message = (mockContext.addMessage as ReturnType<typeof vi.fn>).mock.calls[0][0]
 			expect(message.type).toBe("system")
@@ -701,7 +701,7 @@ describe("sessionCommand", () => {
 
 			await sessionCommand.handler(mockContext)
 
-			expect(mockSessionManager.renameSession).toHaveBeenCalledWith("SingleWord")
+			expect(mockSessionManager.renameSession).toHaveBeenCalledWith("test-session-123", "SingleWord")
 			const message = (mockContext.addMessage as ReturnType<typeof vi.fn>).mock.calls[0][0]
 			expect(message.type).toBe("system")
 			expect(message.content).toContain("SingleWord")
@@ -720,7 +720,7 @@ describe("sessionCommand", () => {
 		})
 
 		it("should handle rename error when no active session", async () => {
-			mockSessionManager.renameSession = vi.fn().mockRejectedValue(new Error("No active session"))
+			mockSessionManager.sessionId = null
 			mockContext.args = ["rename", "New", "Name"]
 
 			await sessionCommand.handler(mockContext)
@@ -750,10 +750,11 @@ describe("sessionCommand", () => {
 
 			await sessionCommand.handler(mockContext)
 
-			expect(mockSessionManager.renameSession).toHaveBeenCalledWith("New Name")
+			expect(mockSessionManager.renameSession).toHaveBeenCalledWith("test-session-123", "New Name")
 		})
 
 		it("should handle backend error gracefully", async () => {
+			mockSessionManager.sessionId = "test-session-123"
 			mockSessionManager.renameSession = vi.fn().mockRejectedValue(new Error("Network error"))
 			mockContext.args = ["rename", "New", "Name"]
 

cli/src/commands/session.ts

@@ -313,7 +313,11 @@ async function renameSession(context: CommandContext, newName: string): Promise<
 	}
 
 	try {
-		await sessionService.renameSession(newName)
+		if (!sessionService.sessionId) {
+			throw new Error("No active session to rename")
+		}
+
+		await sessionService.renameSession(sessionService.sessionId, newName)
 
 		addMessage({
 			...generateMessage(),

src/shared/kilocode/cli-sessions/core/SessionManager.ts

@@ -313,12 +313,11 @@ export class SessionManager {
 		})
 	}
 
-	async renameSession(newTitle: string) {
+	async renameSession(sessionId: string, newTitle: string) {
 		if (!this.sessionClient) {
 			throw new Error("SessionManager used before initialization")
 		}
 
-		const sessionId = this.sessionId
 		if (!sessionId) {
 			throw new Error("No active session")
 		}
@@ -406,6 +405,7 @@ export class SessionManager {
 		this.logger?.debug("Destroying SessionManager", "SessionManager", {
 			sessionId: this.sessionId,
 			isSyncing: this.isSyncing,
+			currentTaskId: this.currentTaskId,
 		})
 
 		if (this.timer) {
@@ -423,7 +423,9 @@ export class SessionManager {
 
 		this.paths = { ...defaultPaths }
 		this.sessionId = null
+		this.currentTaskId = null
 		this.sessionTitle = null
+		this.sessionGitUrl = null
 		this.isSyncing = false
 
 		this.logger?.debug("SessionManager flushed", "SessionManager")
@@ -455,6 +457,8 @@ export class SessionManager {
 		}
 
 		this.isSyncing = true
+		// capture the sessionId at the start of the sync
+		let capturedSessionId = this.sessionId
 
 		try {
 			if (!this.platform || !this.sessionClient || !this.sessionPersistenceManager) {
@@ -488,28 +492,29 @@ export class SessionManager {
 				})
 			}
 
-			if (!this.sessionId && this.currentTaskId) {
+			if (!capturedSessionId && this.currentTaskId) {
 				const existingSessionId = this.sessionPersistenceManager.getSessionForTask(this.currentTaskId)
 
 				if (existingSessionId) {
 					this.sessionId = existingSessionId
+					capturedSessionId = existingSessionId
 				}
 			}
 
-			if (this.sessionId) {
+			if (capturedSessionId) {
 				const gitUrlChanged = gitInfo?.repoUrl && gitInfo.repoUrl !== this.sessionGitUrl
 
 				if (gitUrlChanged) {
-					this.logger?.debug("Updating existing session", "SessionManager", { sessionId: this.sessionId })
-
 					await this.sessionClient.update({
-						session_id: this.sessionId,
+						session_id: capturedSessionId,
 						...basePayload,
 					})
 
 					this.sessionGitUrl = gitInfo?.repoUrl || null
 
-					this.logger?.debug("Session updated successfully", "SessionManager", { sessionId: this.sessionId })
+					this.logger?.debug("Session updated successfully", "SessionManager", {
+						sessionId: capturedSessionId,
+					})
 				}
 			} else {
 				this.logger?.debug("Creating new session", "SessionManager")
@@ -528,29 +533,35 @@ export class SessionManager {
 				})
 
 				this.sessionId = session.session_id
+				capturedSessionId = session.session_id
+
 				this.sessionGitUrl = gitInfo?.repoUrl || null
 
-				this.logger?.info("Session created successfully", "SessionManager", { sessionId: this.sessionId })
+				this.logger?.info("Session created successfully", "SessionManager", { sessionId: capturedSessionId })
 
-				this.sessionPersistenceManager.setLastSession(this.sessionId)
+				this.sessionPersistenceManager.setLastSession(capturedSessionId)
 
 				this.onSessionCreated?.({
 					timestamp: Date.now(),
 					event: "session_created",
-					sessionId: this.sessionId,
+					sessionId: capturedSessionId,
 				})
 			}
 
 			if (this.currentTaskId) {
-				this.sessionPersistenceManager.setSessionForTask(this.currentTaskId, this.sessionId)
+				this.sessionPersistenceManager.setSessionForTask(this.currentTaskId, capturedSessionId)
 			}
 
 			const blobUploads: Array<Promise<void>> = []
 
 			if (rawPayload.apiConversationHistoryPath && this.hasBlobChanged("apiConversationHistory")) {
 				blobUploads.push(
 					this.sessionClient
-						.uploadBlob(this.sessionId, "api_conversation_history", rawPayload.apiConversationHistoryPath)
+						.uploadBlob(
+							capturedSessionId,
+							"api_conversation_history",
+							rawPayload.apiConversationHistoryPath,
+						)
 						.then(() => {
 							this.markBlobSynced("apiConversationHistory")
 							this.logger?.debug("Uploaded api_conversation_history blob", "SessionManager")
@@ -566,7 +577,7 @@ export class SessionManager {
 			if (rawPayload.taskMetadataPath && this.hasBlobChanged("taskMetadata")) {
 				blobUploads.push(
 					this.sessionClient
-						.uploadBlob(this.sessionId, "task_metadata", rawPayload.taskMetadataPath)
+						.uploadBlob(capturedSessionId, "task_metadata", rawPayload.taskMetadataPath)
 						.then(() => {
 							this.markBlobSynced("taskMetadata")
 							this.logger?.debug("Uploaded task_metadata blob", "SessionManager")
@@ -582,7 +593,7 @@ export class SessionManager {
 			if (rawPayload.uiMessagesPath && this.hasBlobChanged("uiMessages")) {
 				blobUploads.push(
 					this.sessionClient
-						.uploadBlob(this.sessionId, "ui_messages", rawPayload.uiMessagesPath)
+						.uploadBlob(capturedSessionId, "ui_messages", rawPayload.uiMessagesPath)
 						.then(() => {
 							this.markBlobSynced("uiMessages")
 							this.logger?.debug("Uploaded ui_messages blob", "SessionManager")
@@ -610,7 +621,7 @@ export class SessionManager {
 					if (this.hasBlobChanged("gitState")) {
 						blobUploads.push(
 							this.sessionClient
-								.uploadBlob(this.sessionId, "git_state", gitStateData)
+								.uploadBlob(capturedSessionId, "git_state", gitStateData)
 								.then(() => {
 									this.markBlobSynced("gitState")
 									this.logger?.debug("Uploaded git_state blob", "SessionManager")
@@ -630,8 +641,8 @@ export class SessionManager {
 			if (!this.sessionTitle && rawPayload.uiMessagesPath) {
 				this.generateTitle(rawPayload.uiMessagesPath as ClineMessage[])
 					.then((generatedTitle) => {
-						if (generatedTitle) {
-							return this.renameSession(generatedTitle)
+						if (capturedSessionId && generatedTitle) {
+							return this.renameSession(capturedSessionId, generatedTitle)
 						}
 
 						return null
@@ -645,7 +656,7 @@ export class SessionManager {
 		} catch (error) {
 			this.logger?.error("Failed to sync session", "SessionManager", {
 				error: error instanceof Error ? error.message : String(error),
-				sessionId: this.sessionId,
+				sessionId: capturedSessionId,
 				hasApiHistory: !!this.paths.apiConversationHistoryPath,
 				hasUiMessages: !!this.paths.uiMessagesPath,
 				hasTaskMetadata: !!this.paths.taskMetadataPath,

src/shared/kilocode/cli-sessions/core/__tests__/SessionManager.test.ts

@@ -434,26 +434,23 @@ describe("SessionManager", () => {
 
 	describe("renameSession", () => {
 		it("should throw error when no active session", async () => {
-			sessionManager.sessionId = null
-
-			await expect(sessionManager.renameSession("New Title")).rejects.toThrow("No active session")
+			await expect(sessionManager.renameSession("", "New Title")).rejects.toThrow("No active session")
 		})
 
 		it("should throw error when title is empty", async () => {
-			sessionManager.sessionId = "session-123"
-
-			await expect(sessionManager.renameSession("   ")).rejects.toThrow("Session title cannot be empty")
+			await expect(sessionManager.renameSession("session-123", "   ")).rejects.toThrow(
+				"Session title cannot be empty",
+			)
 		})
 
 		it("should rename the session", async () => {
-			sessionManager.sessionId = "session-123"
 			vi.mocked(sessionManager.sessionClient!.update).mockResolvedValue({
 				session_id: "session-123",
 				title: "New Title",
 				updated_at: new Date().toISOString(),
 			})
 
-			await sessionManager.renameSession("New Title")
+			await sessionManager.renameSession("session-123", "New Title")
 
 			expect(sessionManager.sessionClient!.update).toHaveBeenCalledWith({
 				session_id: "session-123",
@@ -463,14 +460,13 @@ describe("SessionManager", () => {
 		})
 
 		it("should trim the title", async () => {
-			sessionManager.sessionId = "session-123"
 			vi.mocked(sessionManager.sessionClient!.update).mockResolvedValue({
 				session_id: "session-123",
 				title: "Trimmed Title",
 				updated_at: new Date().toISOString(),
 			})
 
-			await sessionManager.renameSession("  Trimmed Title  ")
+			await sessionManager.renameSession("session-123", "  Trimmed Title  ")
 
 			expect(sessionManager.sessionClient!.update).toHaveBeenCalledWith({
 				session_id: "session-123",
@@ -636,6 +632,32 @@ describe("SessionManager", () => {
 
 			expect(sessionManager["isSyncing"]).toBe(false)
 		})
+
+		it("should clear currentTaskId to prevent session ID clobbering across tasks", async () => {
+			sessionManager.setPath("task-A", "apiConversationHistoryPath", "/path/to/taskA/api.json")
+			sessionManager.sessionId = "session-A"
+			vi.mocked(sessionManager.sessionPersistenceManager!.getSessionForTask).mockReturnValue("session-A")
+			vi.mocked(sessionManager.sessionClient!.create).mockResolvedValue({
+				session_id: "session-B",
+				title: "Task B",
+				created_at: new Date().toISOString(),
+				updated_at: new Date().toISOString(),
+			})
+
+			await sessionManager.destroy()
+
+			expect(sessionManager["currentTaskId"]).toBeNull()
+			expect(sessionManager.sessionId).toBeNull()
+
+			sessionManager.setPath("task-B", "apiConversationHistoryPath", "/path/to/taskB/api.json")
+
+			vi.mocked(sessionManager.sessionPersistenceManager!.getSessionForTask).mockReturnValue(undefined)
+
+			await sessionManager["syncSession"]()
+
+			expect(sessionManager.sessionId).toBe("session-B")
+			expect(sessionManager.sessionId).not.toBe("session-A")
+		})
 	})
 
 	describe("getFirstMessageText", () => {