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
This commit is contained in:
parent
aad0c3644b
commit
1a744424ab
@ -22,7 +22,7 @@ export function getMessageDir(sessionID: string): string | null {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
log(`Error reading message directory: ${error}`)
|
log("[opencode-message-dir] Failed to scan message directories", { sessionID, error: String(error) })
|
||||||
return null
|
return null
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -1,22 +1,22 @@
|
|||||||
import { findNearestMessageWithFields, findNearestMessageWithFieldsFromSDK } from "../features/hook-message-injector"
|
import { findNearestMessageWithFields, findNearestMessageWithFieldsFromSDK } from "../features/hook-message-injector"
|
||||||
import { getMessageDir } from "./opencode-message-dir"
|
import { getMessageDir } from "./opencode-message-dir"
|
||||||
import { isSqliteBackend } from "./opencode-storage-detection"
|
import { isSqliteBackend } from "./opencode-storage-detection"
|
||||||
|
import { log } from "./logger"
|
||||||
import type { PluginInput } from "@opencode-ai/plugin"
|
import type { PluginInput } from "@opencode-ai/plugin"
|
||||||
|
|
||||||
export async function isCallerOrchestrator(sessionID?: string, client?: PluginInput["client"]): Promise<boolean> {
|
export async function isCallerOrchestrator(sessionID?: string, client?: PluginInput["client"]): Promise<boolean> {
|
||||||
if (!sessionID) return false
|
if (!sessionID) return false
|
||||||
|
|
||||||
// Beta mode: use SDK if client provided
|
|
||||||
if (isSqliteBackend() && client) {
|
if (isSqliteBackend() && client) {
|
||||||
try {
|
try {
|
||||||
const nearest = await findNearestMessageWithFieldsFromSDK(client, sessionID)
|
const nearest = await findNearestMessageWithFieldsFromSDK(client, sessionID)
|
||||||
return nearest?.agent?.toLowerCase() === "atlas"
|
return nearest?.agent?.toLowerCase() === "atlas"
|
||||||
} catch {
|
} catch (error) {
|
||||||
|
log("[session-utils] SDK orchestrator check failed", { sessionID, error: String(error) })
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Stable mode: use JSON files
|
|
||||||
const messageDir = getMessageDir(sessionID)
|
const messageDir = getMessageDir(sessionID)
|
||||||
if (!messageDir) return false
|
if (!messageDir) return false
|
||||||
const nearest = findNearestMessageWithFields(messageDir)
|
const nearest = findNearestMessageWithFields(messageDir)
|
||||||
|
|||||||
@ -469,23 +469,18 @@ describe("session-manager storage - SDK path (beta mode)", () => {
|
|||||||
})
|
})
|
||||||
|
|
||||||
test("SDK path returns empty array when client is not set", async () => {
|
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", () => ({
|
mock.module("../../shared/opencode-storage-detection", () => ({
|
||||||
isSqliteBackend: () => true,
|
isSqliteBackend: () => true,
|
||||||
resetSqliteBackendCache: () => {},
|
resetSqliteBackendCache: () => {},
|
||||||
}))
|
}))
|
||||||
|
|
||||||
// Reset SDK client to ensure "client not set" case is exercised
|
//#when client is explicitly cleared and messages are requested
|
||||||
const { setStorageClient } = await import("./storage")
|
const { resetStorageClient, readSessionMessages } = await import("./storage")
|
||||||
setStorageClient(null as any)
|
resetStorageClient()
|
||||||
|
|
||||||
// Re-import without setting client
|
|
||||||
const { readSessionMessages } = await import("./storage")
|
|
||||||
|
|
||||||
// when - calling readSessionMessages without client set
|
|
||||||
const messages = await readSessionMessages("ses_test")
|
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([])
|
expect(messages).toEqual([])
|
||||||
})
|
})
|
||||||
})
|
})
|
||||||
|
|||||||
@ -141,9 +141,9 @@ export function getMessageDir(sessionID: string): string | null {
|
|||||||
export async function sessionExists(sessionID: string): Promise<boolean> {
|
export async function sessionExists(sessionID: string): Promise<boolean> {
|
||||||
if (isSqliteBackend() && sdkClient) {
|
if (isSqliteBackend() && sdkClient) {
|
||||||
try {
|
try {
|
||||||
const response = await sdkClient.session.messages({ path: { id: sessionID } })
|
const response = await sdkClient.session.list()
|
||||||
const messages = response.data as unknown[] | undefined
|
const sessions = (response.data || []) as Array<{ id?: string }>
|
||||||
return Array.isArray(messages) && messages.length > 0
|
return sessions.some((s) => s.id === sessionID)
|
||||||
} catch {
|
} catch {
|
||||||
return false
|
return false
|
||||||
}
|
}
|
||||||
|
|||||||
@ -168,10 +168,15 @@ export async function syncAllTasksToTodos(
|
|||||||
|
|
||||||
const finalTodos: TodoInfo[] = [];
|
const finalTodos: TodoInfo[] = [];
|
||||||
|
|
||||||
|
const removedTaskSubjects = new Set(
|
||||||
|
tasks.filter((t) => t.status === "deleted").map((t) => t.subject),
|
||||||
|
);
|
||||||
|
|
||||||
for (const existing of currentTodos) {
|
for (const existing of currentTodos) {
|
||||||
const isInNewTodos = newTodos.some((newTodo) => todosMatch(existing, newTodo));
|
const isInNewTodos = newTodos.some((newTodo) => todosMatch(existing, newTodo));
|
||||||
const isRemoved = existing.id && tasksToRemove.has(existing.id);
|
const isRemovedById = existing.id ? tasksToRemove.has(existing.id) : false;
|
||||||
if (!isInNewTodos && !isRemoved) {
|
const isRemovedByContent = !existing.id && removedTaskSubjects.has(existing.content);
|
||||||
|
if (!isInNewTodos && !isRemovedById && !isRemovedByContent) {
|
||||||
finalTodos.push(existing);
|
finalTodos.push(existing);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user