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)
This commit is contained in:
YeonGyu-Kim 2026-02-14 18:56:21 +09:00
parent 07da116671
commit 4b2410d0a2
4 changed files with 26 additions and 4 deletions

View File

@ -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.

View File

@ -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")

View File

@ -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) {

View File

@ -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);
}
}