From 1a744424abbe7edd72309a3da51dd24dfd5cf04a Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 16 Feb 2026 00:02:44 +0900 Subject: [PATCH] fix: address all Cubic P2 review issues - session-utils: log SDK errors instead of silent swallow - opencode-message-dir: fix indentation, improve error log format - storage: use session.list for sessionExists (handles empty sessions) - storage.test: use resetStorageClient for proper SDK client cleanup - todo-sync: add content-based fallback for id-less todo removal --- src/shared/opencode-message-dir.ts | 8 ++++---- src/shared/session-utils.ts | 6 +++--- src/tools/session-manager/storage.test.ts | 15 +++++---------- src/tools/session-manager/storage.ts | 6 +++--- src/tools/task/todo-sync.ts | 9 +++++++-- 5 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/shared/opencode-message-dir.ts b/src/shared/opencode-message-dir.ts index 14736e4b7..f330e84fa 100644 --- a/src/shared/opencode-message-dir.ts +++ b/src/shared/opencode-message-dir.ts @@ -21,10 +21,10 @@ export function getMessageDir(sessionID: string): string | null { return sessionPath } } -} catch (error) { - log(`Error reading message directory: ${error}`) - return null -} + } catch (error) { + log("[opencode-message-dir] Failed to scan message directories", { sessionID, error: String(error) }) + return null + } return null } \ No newline at end of file diff --git a/src/shared/session-utils.ts b/src/shared/session-utils.ts index ce2283617..5a9d33065 100644 --- a/src/shared/session-utils.ts +++ b/src/shared/session-utils.ts @@ -1,22 +1,22 @@ import { findNearestMessageWithFields, findNearestMessageWithFieldsFromSDK } from "../features/hook-message-injector" import { getMessageDir } from "./opencode-message-dir" import { isSqliteBackend } from "./opencode-storage-detection" +import { log } from "./logger" import type { PluginInput } from "@opencode-ai/plugin" export async function isCallerOrchestrator(sessionID?: string, client?: PluginInput["client"]): Promise { if (!sessionID) return false - // Beta mode: use SDK if client provided if (isSqliteBackend() && client) { try { const nearest = await findNearestMessageWithFieldsFromSDK(client, sessionID) return nearest?.agent?.toLowerCase() === "atlas" - } catch { + } catch (error) { + log("[session-utils] SDK orchestrator check failed", { sessionID, error: String(error) }) return false } } - // Stable mode: use JSON files const messageDir = getMessageDir(sessionID) if (!messageDir) return false const nearest = findNearestMessageWithFields(messageDir) diff --git a/src/tools/session-manager/storage.test.ts b/src/tools/session-manager/storage.test.ts index 81e3048e5..d4fe7b50f 100644 --- a/src/tools/session-manager/storage.test.ts +++ b/src/tools/session-manager/storage.test.ts @@ -469,23 +469,18 @@ describe("session-manager storage - SDK path (beta mode)", () => { }) test("SDK path returns empty array when client is not set", async () => { - // given - beta mode enabled but no client set + //#given beta mode enabled but no client set mock.module("../../shared/opencode-storage-detection", () => ({ isSqliteBackend: () => true, resetSqliteBackendCache: () => {}, })) - // Reset SDK client to ensure "client not set" case is exercised - const { setStorageClient } = await import("./storage") - setStorageClient(null as any) - - // Re-import without setting client - const { readSessionMessages } = await import("./storage") - - // when - calling readSessionMessages without client set + //#when client is explicitly cleared and messages are requested + const { resetStorageClient, readSessionMessages } = await import("./storage") + resetStorageClient() const messages = await readSessionMessages("ses_test") - // then - should return empty array since no client and no JSON fallback + //#then should return empty array since no client and no JSON fallback expect(messages).toEqual([]) }) }) diff --git a/src/tools/session-manager/storage.ts b/src/tools/session-manager/storage.ts index 530782af7..de0226f1c 100644 --- a/src/tools/session-manager/storage.ts +++ b/src/tools/session-manager/storage.ts @@ -141,9 +141,9 @@ export function getMessageDir(sessionID: string): string | null { export async function sessionExists(sessionID: string): Promise { if (isSqliteBackend() && sdkClient) { try { - const response = await sdkClient.session.messages({ path: { id: sessionID } }) - const messages = response.data as unknown[] | undefined - return Array.isArray(messages) && messages.length > 0 + const response = await sdkClient.session.list() + const sessions = (response.data || []) as Array<{ id?: string }> + return sessions.some((s) => s.id === sessionID) } catch { return false } diff --git a/src/tools/task/todo-sync.ts b/src/tools/task/todo-sync.ts index 0f5e63fdf..68717fe4f 100644 --- a/src/tools/task/todo-sync.ts +++ b/src/tools/task/todo-sync.ts @@ -168,10 +168,15 @@ export async function syncAllTasksToTodos( const finalTodos: TodoInfo[] = []; + const removedTaskSubjects = new Set( + tasks.filter((t) => t.status === "deleted").map((t) => t.subject), + ); + for (const existing of currentTodos) { const isInNewTodos = newTodos.some((newTodo) => todosMatch(existing, newTodo)); - const isRemoved = existing.id && tasksToRemove.has(existing.id); - if (!isInNewTodos && !isRemoved) { + const isRemovedById = existing.id ? tasksToRemove.has(existing.id) : false; + const isRemovedByContent = !existing.id && removedTaskSubjects.has(existing.content); + if (!isInNewTodos && !isRemovedById && !isRemovedByContent) { finalTodos.push(existing); } }