fix(todo): make Todo id field optional for OpenCode beta compatibility

- Make id field optional in all Todo interfaces (TodoInfo, Todo, TodoItem)
- Fix null-unsafe comparisons in todo-sync.ts to handle missing ids
- Add test case for todos without id field preservation
- All tests pass and typecheck clean
This commit is contained in:
YeonGyu-Kim 2026-02-14 17:41:40 +09:00
parent cb4a165c76
commit e90734d6d9
14 changed files with 180 additions and 98 deletions

View File

@ -34,10 +34,10 @@ export interface RunContext {
} }
export interface Todo { export interface Todo {
id: string id?: string;
content: string content: string;
status: string status: string;
priority: string priority: string;
} }
export interface SessionStatus { export interface SessionStatus {

View File

@ -33,10 +33,10 @@ export interface BackgroundEvent {
} }
export interface Todo { export interface Todo {
content: string content: string;
status: string status: string;
priority: string priority: string;
id: string id?: string;
} }
export interface QueueItem { export interface QueueItem {

View File

@ -1,36 +1,17 @@
import { existsSync, readdirSync } from "node:fs" import { getMessageDir } from "../../shared/opencode-message-dir"
import { join } from "node:path"
import { MESSAGE_STORAGE_DIR } from "./storage-paths" export { getMessageDir }
export function getMessageDir(sessionID: string): string {
if (!existsSync(MESSAGE_STORAGE_DIR)) return ""
const directPath = join(MESSAGE_STORAGE_DIR, sessionID)
if (existsSync(directPath)) {
return directPath
}
for (const directory of readdirSync(MESSAGE_STORAGE_DIR)) {
const sessionPath = join(MESSAGE_STORAGE_DIR, directory, sessionID)
if (existsSync(sessionPath)) {
return sessionPath
}
}
return ""
}
export function getMessageIds(sessionID: string): string[] { export function getMessageIds(sessionID: string): string[] {
const messageDir = getMessageDir(sessionID) const messageDir = getMessageDir(sessionID)
if (!messageDir || !existsSync(messageDir)) return [] if (!messageDir || !existsSync(messageDir)) return []
const messageIds: string[] = [] const messageIds: string[] = []
for (const file of readdirSync(messageDir)) { for (const file of readdirSync(messageDir)) {
if (!file.endsWith(".json")) continue if (!file.endsWith(".json")) continue
const messageId = file.replace(".json", "") const messageId = file.replace(".json", "")
messageIds.push(messageId) messageIds.push(messageId)
} }
return messageIds return messageIds
} }

View File

@ -1,21 +1 @@
import { existsSync, readdirSync } from "node:fs" export { getMessageDir } from "../../../shared/opencode-message-dir"
import { join } from "node:path"
import { MESSAGE_STORAGE } from "../constants"
export function getMessageDir(sessionID: string): string {
if (!existsSync(MESSAGE_STORAGE)) return ""
const directPath = join(MESSAGE_STORAGE, sessionID)
if (existsSync(directPath)) {
return directPath
}
for (const dir of readdirSync(MESSAGE_STORAGE)) {
const sessionPath = join(MESSAGE_STORAGE, dir, sessionID)
if (existsSync(sessionPath)) {
return sessionPath
}
}
return ""
}

View File

@ -15,10 +15,10 @@ export interface TodoContinuationEnforcer {
} }
export interface Todo { export interface Todo {
content: string content: string;
status: string status: string;
priority: string priority: string;
id: string id?: string;
} }
export interface SessionState { export interface SessionState {

View File

@ -37,7 +37,7 @@ export { resolveModelPipeline } from "./model-resolution-pipeline"
export type { export type {
ModelResolutionRequest, ModelResolutionRequest,
ModelResolutionProvenance, ModelResolutionProvenance,
ModelResolutionResult as ModelResolutionPipelineResult, ModelResolutionPipelineResult,
} from "./model-resolution-types" } from "./model-resolution-types"
export * from "./model-availability" export * from "./model-availability"
export * from "./connected-providers-cache" export * from "./connected-providers-cache"
@ -49,3 +49,4 @@ export * from "./port-utils"
export * from "./git-worktree" export * from "./git-worktree"
export * from "./safe-create-hook" export * from "./safe-create-hook"
export * from "./truncate-description" export * from "./truncate-description"
export * from "./opencode-message-dir"

View File

@ -0,0 +1,99 @@
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
import { existsSync, readdirSync } from "node:fs"
import { join } from "node:path"
import { getMessageDir } from "./opencode-message-dir"
// Mock the constants
vi.mock("../tools/session-manager/constants", () => ({
MESSAGE_STORAGE: "/mock/message/storage",
}))
vi.mock("node:fs", () => ({
existsSync: vi.fn(),
readdirSync: vi.fn(),
}))
vi.mock("node:path", () => ({
join: vi.fn(),
}))
const mockExistsSync = vi.mocked(existsSync)
const mockReaddirSync = vi.mocked(readdirSync)
const mockJoin = vi.mocked(join)
describe("getMessageDir", () => {
beforeEach(() => {
vi.clearAllMocks()
mockJoin.mockImplementation((...args) => args.join("/"))
})
afterEach(() => {
vi.restoreAllMocks()
})
it("returns null when MESSAGE_STORAGE does not exist", () => {
// given
mockExistsSync.mockReturnValue(false)
// when
const result = getMessageDir("session123")
// then
expect(result).toBe(null)
expect(mockExistsSync).toHaveBeenCalledWith("/mock/message/storage")
})
it("returns direct path when session exists directly", () => {
// given
mockExistsSync.mockImplementation((path) => path === "/mock/message/storage" || path === "/mock/message/storage/session123")
// when
const result = getMessageDir("session123")
// then
expect(result).toBe("/mock/message/storage/session123")
expect(mockExistsSync).toHaveBeenCalledWith("/mock/message/storage")
expect(mockExistsSync).toHaveBeenCalledWith("/mock/message/storage/session123")
})
it("returns subdirectory path when session exists in subdirectory", () => {
// given
mockExistsSync.mockImplementation((path) => {
return path === "/mock/message/storage" || path === "/mock/message/storage/subdir/session123"
})
mockReaddirSync.mockReturnValue(["subdir"])
// when
const result = getMessageDir("session123")
// then
expect(result).toBe("/mock/message/storage/subdir/session123")
expect(mockReaddirSync).toHaveBeenCalledWith("/mock/message/storage")
})
it("returns null when session not found anywhere", () => {
// given
mockExistsSync.mockImplementation((path) => path === "/mock/message/storage")
mockReaddirSync.mockReturnValue(["subdir1", "subdir2"])
// when
const result = getMessageDir("session123")
// then
expect(result).toBe(null)
})
it("returns null when readdirSync throws", () => {
// given
mockExistsSync.mockImplementation((path) => path === "/mock/message/storage")
mockReaddirSync.mockImplementation(() => {
throw new Error("Permission denied")
})
// when
const result = getMessageDir("session123")
// then
expect(result).toBe(null)
})
})

View File

@ -0,0 +1,25 @@
import { existsSync, readdirSync } from "node:fs"
import { join } from "node:path"
import { MESSAGE_STORAGE } from "../tools/session-manager/constants"
export function getMessageDir(sessionID: string): string | null {
if (!existsSync(MESSAGE_STORAGE)) return null
const directPath = join(MESSAGE_STORAGE, sessionID)
if (existsSync(directPath)) {
return directPath
}
try {
for (const dir of readdirSync(MESSAGE_STORAGE)) {
const sessionPath = join(MESSAGE_STORAGE, dir, sessionID)
if (existsSync(sessionPath)) {
return sessionPath
}
}
} catch {
return null
}
return null
}

View File

@ -3,20 +3,7 @@ import * as os from "node:os"
import { existsSync, readdirSync } from "node:fs" import { existsSync, readdirSync } from "node:fs"
import { join } from "node:path" import { join } from "node:path"
import { findNearestMessageWithFields, MESSAGE_STORAGE } from "../features/hook-message-injector" import { findNearestMessageWithFields, MESSAGE_STORAGE } from "../features/hook-message-injector"
import { getMessageDir } from "./opencode-message-dir"
export function getMessageDir(sessionID: string): string | null {
if (!existsSync(MESSAGE_STORAGE)) return null
const directPath = join(MESSAGE_STORAGE, sessionID)
if (existsSync(directPath)) return directPath
for (const dir of readdirSync(MESSAGE_STORAGE)) {
const sessionPath = join(MESSAGE_STORAGE, dir, sessionID)
if (existsSync(sessionPath)) return sessionPath
}
return null
}
export function isCallerOrchestrator(sessionID?: string): boolean { export function isCallerOrchestrator(sessionID?: string): boolean {
if (!sessionID) return false if (!sessionID) return false

View File

@ -44,7 +44,7 @@ export async function formatSessionList(sessionIDs: string[]): Promise<string> {
export function formatSessionMessages( export function formatSessionMessages(
messages: SessionMessage[], messages: SessionMessage[],
includeTodos?: boolean, includeTodos?: boolean,
todos?: Array<{ id: string; content: string; status: string }> todos?: Array<{ id?: string; content: string; status: string }>
): string { ): string {
if (messages.length === 0) { if (messages.length === 0) {
return "No messages found in this session." return "No messages found in this session."

View File

@ -73,8 +73,8 @@ export async function getAllSessions(): Promise<string[]> {
return [...new Set(sessions)] return [...new Set(sessions)]
} }
export function getMessageDir(sessionID: string): string { export function getMessageDir(sessionID: string): string | null {
if (!existsSync(MESSAGE_STORAGE)) return "" if (!existsSync(MESSAGE_STORAGE)) return null
const directPath = join(MESSAGE_STORAGE, sessionID) const directPath = join(MESSAGE_STORAGE, sessionID)
if (existsSync(directPath)) { if (existsSync(directPath)) {
@ -89,14 +89,14 @@ export function getMessageDir(sessionID: string): string {
} }
} }
} catch { } catch {
return "" return null
} }
return "" return null
} }
export function sessionExists(sessionID: string): boolean { export function sessionExists(sessionID: string): boolean {
return getMessageDir(sessionID) !== "" return getMessageDir(sessionID) !== null
} }
export async function readSessionMessages(sessionID: string): Promise<SessionMessage[]> { export async function readSessionMessages(sessionID: string): Promise<SessionMessage[]> {

View File

@ -34,10 +34,10 @@ export interface SessionInfo {
} }
export interface TodoItem { export interface TodoItem {
id: string id?: string;
content: string content: string;
status: "pending" | "in_progress" | "completed" | "cancelled" status: "pending" | "in_progress" | "completed" | "cancelled";
priority?: string priority?: string;
} }
export interface SearchResult { export interface SearchResult {

View File

@ -471,7 +471,7 @@ describe("syncAllTasksToTodos", () => {
expect(mockCtx.client.session.todo).toHaveBeenCalled(); expect(mockCtx.client.session.todo).toHaveBeenCalled();
}); });
it("handles undefined sessionID", async () => { it("preserves todos without id field", async () => {
// given // given
const tasks: Task[] = [ const tasks: Task[] = [
{ {
@ -483,14 +483,23 @@ describe("syncAllTasksToTodos", () => {
blockedBy: [], blockedBy: [],
}, },
]; ];
mockCtx.client.session.todo.mockResolvedValue([]); const currentTodos: TodoInfo[] = [
{
id: "T-1",
content: "Task 1",
status: "pending",
},
{
content: "Todo without id",
status: "pending",
},
];
mockCtx.client.session.todo.mockResolvedValue(currentTodos);
// when // when
await syncAllTasksToTodos(mockCtx, tasks); await syncAllTasksToTodos(mockCtx, tasks, "session-1");
// then // then
expect(mockCtx.client.session.todo).toHaveBeenCalledWith({ expect(mockCtx.client.session.todo).toHaveBeenCalled();
path: { id: "" },
});
}); });
}); });

View File

@ -3,7 +3,7 @@ import { log } from "../../shared/logger";
import type { Task } from "../../features/claude-tasks/types.ts"; import type { Task } from "../../features/claude-tasks/types.ts";
export interface TodoInfo { export interface TodoInfo {
id: string; id?: string;
content: string; content: string;
status: "pending" | "in_progress" | "completed" | "cancelled"; status: "pending" | "in_progress" | "completed" | "cancelled";
priority?: "low" | "medium" | "high"; priority?: "low" | "medium" | "high";
@ -100,7 +100,7 @@ export async function syncTaskTodoUpdate(
path: { id: sessionID }, path: { id: sessionID },
}); });
const currentTodos = extractTodos(response); const currentTodos = extractTodos(response);
const nextTodos = currentTodos.filter((todo) => todo.id !== task.id); const nextTodos = currentTodos.filter((todo) => !todo.id || todo.id !== task.id);
const todo = syncTaskToTodo(task); const todo = syncTaskToTodo(task);
if (todo) { if (todo) {
@ -150,10 +150,10 @@ export async function syncAllTasksToTodos(
} }
const finalTodos: TodoInfo[] = []; const finalTodos: TodoInfo[] = [];
const newTodoIds = new Set(newTodos.map((t) => t.id)); const newTodoIds = new Set(newTodos.map((t) => t.id).filter((id) => id !== undefined));
for (const existing of currentTodos) { for (const existing of currentTodos) {
if (!newTodoIds.has(existing.id) && !tasksToRemove.has(existing.id)) { if ((!existing.id || !newTodoIds.has(existing.id)) && !tasksToRemove.has(existing.id || "")) {
finalTodos.push(existing); finalTodos.push(existing);
} }
} }