From 4b2410d0a24659f43deb0af64efd7040cc06c036 Mon Sep 17 00:00:00 2001
From: YeonGyu-Kim <code.yeon.gyu@gmail.com>
Date: Sat, 14 Feb 2026 18:56:21 +0900
Subject: [PATCH] fix: address remaining Cubic review comments (P2 issues)

- Add content-based fallback matching for todos without ids

- Add TODO comment for exported but unused SDK functions

- Add resetStorageClient() for test isolation

- Fixes todo duplication risk on beta (SQLite backend)
---
 src/features/hook-message-injector/injector.ts |  5 +++++
 src/tools/session-manager/storage.test.ts      |  4 ++++
 src/tools/session-manager/storage.ts           |  4 ++++
 src/tools/task/todo-sync.ts                    | 17 +++++++++++++----
 4 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/src/features/hook-message-injector/injector.ts b/src/features/hook-message-injector/injector.ts
index e8fac0d4..4d455240 100644
--- a/src/features/hook-message-injector/injector.ts
+++ b/src/features/hook-message-injector/injector.ts
@@ -49,6 +49,11 @@ function convertSDKMessageToStoredMessage(msg: SDKMessage): StoredMessage | null
   }
 }
 
+// TODO: These SDK-based functions are exported for future use when hooks migrate to async.
+// Currently, callers still use the sync JSON-based functions which return null on beta.
+// Migration requires making callers async, which is a larger refactoring.
+// See: https://github.com/code-yeongyu/oh-my-opencode/pull/1837
+
 /**
  * Finds the nearest message with required fields using SDK (for beta/SQLite backend).
  * Uses client.session.messages() to fetch message data from SQLite.
diff --git a/src/tools/session-manager/storage.test.ts b/src/tools/session-manager/storage.test.ts
index 771457c4..7239a7e0 100644
--- a/src/tools/session-manager/storage.test.ts
+++ b/src/tools/session-manager/storage.test.ts
@@ -475,6 +475,10 @@ describe("session-manager storage - SDK path (beta mode)", () => {
       resetSqliteBackendCache: () => {},
     }))
 
+    // Reset client to ensure "client not set" case is exercised
+    const { resetStorageClient } = await import("./storage")
+    resetStorageClient()
+
     // Re-import without setting client
     const { readSessionMessages } = await import("./storage")
 
diff --git a/src/tools/session-manager/storage.ts b/src/tools/session-manager/storage.ts
index d10a18d6..fab794d8 100644
--- a/src/tools/session-manager/storage.ts
+++ b/src/tools/session-manager/storage.ts
@@ -17,6 +17,10 @@ export function setStorageClient(client: PluginInput["client"]): void {
   sdkClient = client
 }
 
+export function resetStorageClient(): void {
+  sdkClient = null
+}
+
 export async function getMainSessions(options: GetMainSessionsOptions): Promise<SessionMetadata[]> {
   // Beta mode: use SDK
   if (isSqliteBackend() && sdkClient) {
diff --git a/src/tools/task/todo-sync.ts b/src/tools/task/todo-sync.ts
index 05075e2d..8a06ce52 100644
--- a/src/tools/task/todo-sync.ts
+++ b/src/tools/task/todo-sync.ts
@@ -47,6 +47,13 @@ function extractPriority(
   return undefined;
 }
 
+function todosMatch(todo1: TodoInfo, todo2: TodoInfo): boolean {
+  if (todo1.id && todo2.id) {
+    return todo1.id === todo2.id;
+  }
+  return todo1.content === todo2.content;
+}
+
 export function syncTaskToTodo(task: Task): TodoInfo | null {
   const todoStatus = mapTaskStatusToTodoStatus(task.status);
 
@@ -100,8 +107,9 @@ export async function syncTaskTodoUpdate(
       path: { id: sessionID },
     });
     const currentTodos = extractTodos(response);
-    const nextTodos = currentTodos.filter((todo) => !todo.id || todo.id !== task.id);
-    const todo = syncTaskToTodo(task);
+    const taskTodo = syncTaskToTodo(task);
+    const nextTodos = currentTodos.filter((todo) => !taskTodo || !todosMatch(todo, taskTodo));
+    const todo = taskTodo;
 
     if (todo) {
       nextTodos.push(todo);
@@ -150,10 +158,11 @@ export async function syncAllTasksToTodos(
     }
 
     const finalTodos: TodoInfo[] = [];
-    const newTodoIds = new Set(newTodos.map((t) => t.id).filter((id) => id !== undefined));
 
     for (const existing of currentTodos) {
-      if ((!existing.id || !newTodoIds.has(existing.id)) && !tasksToRemove.has(existing.id || "")) {
+      const isInNewTodos = newTodos.some((newTodo) => todosMatch(existing, newTodo));
+      const isRemoved = existing.id && tasksToRemove.has(existing.id);
+      if (!isInNewTodos && !isRemoved) {
         finalTodos.push(existing);
       }
     }