Merge pull request #2256 from bashrusakh/fix/issue-2244-todo-event-resilience

fix(sync): route directory-less todo updates
This commit is contained in:
Serhii Dziupin
2026-08-06 18:09:24 +03:00
committed by GitHub
4 changed files with 101 additions and 7 deletions
+4
View File
@@ -363,6 +363,10 @@ Keep this in sync with `handleDirectoryEvent` in `sync-context.tsx`:
| `question.asked/replied/rejected` | `question` | | `question.asked/replied/rejected` | `question` |
| `lsp.updated` | `lsp` | | `lsp.updated` | `lsp` |
### Directory-less session events
The global stream can omit a directory for a session-addressed event. Resolve it through the session routing index first. If the index is briefly stale during a session transition, route only when the event session matches the active session and that directory store exists; otherwise leave it un-routed rather than updating another directory.
## Adding a new event type ## Adding a new event type
1. Add the case to the event reducer (`event-reducer.ts`) 1. Add the case to the event reducer (`event-reducer.ts`)
@@ -1,9 +1,10 @@
import { describe, expect, test, beforeEach, mock } from "bun:test" import { describe, expect, test, beforeEach, mock } from "bun:test"
import { create, type StoreApi } from "zustand" import { create, type StoreApi } from "zustand"
import type { PermissionRequest, QuestionRequest } from "@opencode-ai/sdk/v2/client" import type { Event, PermissionRequest, QuestionRequest } from "@opencode-ai/sdk/v2/client"
const listPendingQuestionsCalls: Array<{ directories?: Array<string | null | undefined> }> = [] const listPendingQuestionsCalls: Array<{ directories?: Array<string | null | undefined> }> = []
const listPendingPermissionsCalls: Array<{ directories?: Array<string | null | undefined> }> = [] const listPendingPermissionsCalls: Array<{ directories?: Array<string | null | undefined> }> = []
const todoPersistWrites: Array<{ sessionID: string; todos: unknown }> = []
let pendingQuestionsResponse: QuestionRequest[] = [] let pendingQuestionsResponse: QuestionRequest[] = []
let pendingPermissionsResponse: PermissionRequest[] = [] let pendingPermissionsResponse: PermissionRequest[] = []
let pendingQuestionsShouldThrow = false let pendingQuestionsShouldThrow = false
@@ -41,7 +42,22 @@ mock.module("@/stores/useConfigStore", () => ({
})) }))
mock.module("@/stores/useTodosPersistStore", () => ({ mock.module("@/stores/useTodosPersistStore", () => ({
useTodosPersistStore: { getState: () => ({}) }, useTodosPersistStore: {
getState: () => ({
setSessionTodos: (sessionID: string, todos: unknown) => {
todoPersistWrites.push({ sessionID, todos })
},
}),
},
}))
mock.module("sonner", () => ({
toast: {
dismiss: () => undefined,
error: () => undefined,
info: () => undefined,
success: () => undefined,
},
})) }))
mock.module("@/components/ui", () => ({ mock.module("@/components/ui", () => ({
@@ -49,8 +65,13 @@ mock.module("@/components/ui", () => ({
})) }))
import { INITIAL_STATE, type State } from "../types" import { INITIAL_STATE, type State } from "../types"
import type { DirectoryStore } from "../child-store" import { ChildStoreManager, type DirectoryStore } from "../child-store"
import { resyncBlockingRequestsForDirectory } from "../sync-context" const {
createEventRoutingIndex,
handleEvent,
resyncBlockingRequestsForDirectory,
setActiveSession,
} = await import("../sync-context")
function buildQuestion(overrides: Partial<QuestionRequest> = {}): QuestionRequest { function buildQuestion(overrides: Partial<QuestionRequest> = {}): QuestionRequest {
return { return {
@@ -91,6 +112,8 @@ describe("resyncBlockingRequestsForDirectory", () => {
pendingPermissionsResponse = [] pendingPermissionsResponse = []
pendingQuestionsShouldThrow = false pendingQuestionsShouldThrow = false
pendingPermissionsShouldThrow = false pendingPermissionsShouldThrow = false
todoPersistWrites.length = 0
setActiveSession("", "")
}) })
test("calls listPendingQuestions and listPendingPermissions exactly once for the directory", async () => { test("calls listPendingQuestions and listPendingPermissions exactly once for the directory", async () => {
@@ -205,4 +228,56 @@ describe("resyncBlockingRequestsForDirectory", () => {
expect(store.getState().question["ses_a"]?.[0]?.id).toBe("que_1") expect(store.getState().question["ses_a"]?.[0]?.id).toBe("que_1")
expect(listPendingPermissionsCalls).toHaveLength(1) expect(listPendingPermissionsCalls).toHaveLength(1)
}) })
test("routes a directory-less todo snapshot to its active session during a multi-store routing-index gap", () => {
const childStores = new ChildStoreManager()
const store = childStores.ensureChild("/target", { bootstrap: false })
childStores.ensureChild("/other", { bootstrap: false })
const todos = [
{ content: "Finish plan", status: "completed", priority: "high" },
{ content: "Implement changes", status: "in_progress", priority: "high" },
]
const event = {
type: "todo.updated",
properties: { sessionID: "ses_a", todos },
} as Event
const routingIndex = createEventRoutingIndex()
expect(childStores.children.size).toBe(2)
expect(routingIndex.sessionDirectoryById.size).toBe(0)
for (const candidate of childStores.children.values()) {
const state = candidate.getState()
expect(state.session).toEqual([])
expect(state.message.ses_a).toBe(undefined)
expect(state.session_status.ses_a).toBe(undefined)
}
let storeWrites = 0
const unsubscribe = store.subscribe(() => {
storeWrites += 1
})
setActiveSession("/target", "ses_a")
handleEvent("global", event, childStores, routingIndex)
expect(store.getState().todo.ses_a).toEqual(todos)
expect(todoPersistWrites).toEqual([{ sessionID: "ses_a", todos }])
expect(storeWrites).toBe(1)
const stateAfterFirstSnapshot = store.getState()
const duplicateTodos = todos.map((todo) => ({ ...todo }))
const duplicateEvent = {
type: "todo.updated",
properties: { sessionID: "ses_a", todos: duplicateTodos },
} as Event
expect(duplicateTodos).not.toBe(todos)
expect(duplicateTodos).toEqual(todos)
handleEvent("global", duplicateEvent, childStores, routingIndex)
expect(store.getState()).toBe(stateAfterFirstSnapshot)
expect(todoPersistWrites).toEqual([{ sessionID: "ses_a", todos }])
expect(storeWrites).toBe(1)
unsubscribe()
childStores.disposeAll()
})
}) })
+3
View File
@@ -309,6 +309,9 @@ export function applyDirectoryEvent(
case "todo.updated": { case "todo.updated": {
const props = event.properties as { sessionID: string; todos: Todo[] } const props = event.properties as { sessionID: string; todos: Todo[] }
if (areJsonEquivalent(draft.todo[props.sessionID], props.todos)) {
return false
}
draft.todo[props.sessionID] = props.todos draft.todo[props.sessionID] = props.todos
callbacks?.onSetSessionTodo?.(props.sessionID, props.todos) callbacks?.onSetSessionTodo?.(props.sessionID, props.todos)
return true return true
+15 -3
View File
@@ -697,7 +697,7 @@ const dispatchVSCodeRuntimeNotificationEvent = (directory: string, payload: Even
})) }))
} }
const createEventRoutingIndex = (): EventRoutingIndex => ({ export const createEventRoutingIndex = (): EventRoutingIndex => ({
sessionDirectoryById: new Map(), sessionDirectoryById: new Map(),
messageSessionById: new Map(), messageSessionById: new Map(),
sessionMessageIdsById: new Map(), sessionMessageIdsById: new Map(),
@@ -997,8 +997,12 @@ const childStoreHasMessagePartState = (
return Object.prototype.hasOwnProperty.call(getDirectoryEventState(store, batch).part, messageID) return Object.prototype.hasOwnProperty.call(getDirectoryEventState(store, batch).part, messageID)
} }
const getActiveDirectoryFallback = (childStores: ChildStoreManager): string | null => { const getActiveDirectoryFallback = (
childStores: ChildStoreManager,
sessionID?: string | null,
): string | null => {
if (!_activeDirectory || !_activeSession) return null if (!_activeDirectory || !_activeSession) return null
if (sessionID && sessionID !== _activeSession) return null
return childStores.getChild(_activeDirectory) ? _activeDirectory : null return childStores.getChild(_activeDirectory) ? _activeDirectory : null
} }
@@ -1029,6 +1033,14 @@ const resolveDirectoryFromRoutingIndex = (
if (found) { if (found) {
return found return found
} }
// The global stream does not always include a directory. During a session
// transition, its routing index can lag the active session briefly; route
// a session-addressed event only when that session is the one being viewed.
const activeDirectory = getActiveDirectoryFallback(childStores, sessionID)
if (activeDirectory) {
return activeDirectory
}
} }
const messageID = getMessageIdFromPayload(payload) const messageID = getMessageIdFromPayload(payload)
@@ -1384,7 +1396,7 @@ async function resyncDirectoryAfterReconnect(
ingestDirectoryStateIntoRoutingIndex(routingIndex, directory, store.getState()) ingestDirectoryStateIntoRoutingIndex(routingIndex, directory, store.getState())
} }
function handleEvent( export function handleEvent(
rawDirectory: string, rawDirectory: string,
payload: Event, payload: Event,
childStores: ChildStoreManager, childStores: ChildStoreManager,