fix(sync): preserve pending questions across session switch and directory eviction (closes #918) (#1103)
* fix(sync): preserve pending questions across session switch and directory eviction Closes #918, completes the gap left by #909. The 'agent question disappears after switching session / coming back later' bug had two root causes that #909 only partially addressed: 1. Directory-eviction TTL (20 min) silently dropped child stores that held pending questions/permissions. The discard wasn't gated on in-flight blocking-request state, so any 'question.asked' event that arrived during the eviction-then-rehydrate window was routed to a non-existent store and silently lost. 2. PR #909 re-fetches listPendingQuestions/Permissions only on SSE reconnect. Switching sessions within the same socket — including navigating back to a directory whose child store was rebuilt after eviction — left the UI relying on store state that may have missed events that fired while a different session was active. Three edits, in src/sync: - eviction.ts / types.ts / child-store.ts: add hasPendingBlockingRequests to EvictPlan + DisposeCheck and never evict a directory whose store carries a non-empty state.question or state.permission record. - sync-context.tsx: extract resyncBlockingRequestsForDirectory from the reconnect path and call it on currentSessionId changes (debounced 250ms), reusing PR #909's signature-based merge so concurrent SSE updates aren't clobbered. - __tests__/eviction.test.ts, __tests__/session-switch-resync.test.ts: new unit coverage for the eviction guard and resync semantics (deduped fetch per switch, in-flight SSE preservation, stale entry cleanup, unknown-session filtering). * fix(sync): refresh store before blocking request resync --------- Co-authored-by: Alexander Busse <alex@ableph.net> Co-authored-by: Bohdan Triapitsyn <artmore@protonmail.com>
This commit is contained in:
Alexander Busse
committed by
GitHub
GitHub
co-authored by
Alexander Busse
Bohdan Triapitsyn
Alexander Busse
Bohdan Triapitsyn
parent
a49e2ce1ae
commit
2b0e1ef42e
@@ -0,0 +1,135 @@
|
||||
import { describe, expect, test } from "bun:test"
|
||||
import type { PermissionRequest, QuestionRequest } from "@opencode-ai/sdk/v2/client"
|
||||
import {
|
||||
canDisposeDirectory,
|
||||
hasPendingBlockingRequests,
|
||||
pickDirectoriesToEvict,
|
||||
} from "../eviction"
|
||||
import { INITIAL_STATE, type DirState, type State } from "../types"
|
||||
|
||||
const DAY_MS = 24 * 60 * 60 * 1000
|
||||
|
||||
function buildState(overrides: Partial<State> = {}): State {
|
||||
return {
|
||||
...INITIAL_STATE,
|
||||
question: {},
|
||||
permission: {},
|
||||
...overrides,
|
||||
}
|
||||
}
|
||||
|
||||
function buildQuestion(overrides: Partial<QuestionRequest> = {}): QuestionRequest {
|
||||
return {
|
||||
id: "que_1",
|
||||
sessionID: "ses_1",
|
||||
questions: [{ question: "Continue?", header: "Q", options: [{ label: "Yes", description: "" }] }],
|
||||
...overrides,
|
||||
} as QuestionRequest
|
||||
}
|
||||
|
||||
function buildPermission(overrides: Partial<PermissionRequest> = {}): PermissionRequest {
|
||||
return {
|
||||
id: "perm_1",
|
||||
sessionID: "ses_1",
|
||||
permission: "bash",
|
||||
patterns: [],
|
||||
metadata: {},
|
||||
always: [],
|
||||
...overrides,
|
||||
} as PermissionRequest
|
||||
}
|
||||
|
||||
describe("hasPendingBlockingRequests", () => {
|
||||
test("returns false on undefined or empty state", () => {
|
||||
expect(hasPendingBlockingRequests(undefined)).toBe(false)
|
||||
expect(hasPendingBlockingRequests(buildState())).toBe(false)
|
||||
})
|
||||
|
||||
test("returns true when at least one session has a pending question", () => {
|
||||
const state = buildState({ question: { ses_a: [buildQuestion()] } })
|
||||
expect(hasPendingBlockingRequests(state)).toBe(true)
|
||||
})
|
||||
|
||||
test("returns true when at least one session has a pending permission", () => {
|
||||
const state = buildState({ permission: { ses_a: [buildPermission()] } })
|
||||
expect(hasPendingBlockingRequests(state)).toBe(true)
|
||||
})
|
||||
|
||||
test("treats empty arrays under a session key as no pending work", () => {
|
||||
const state = buildState({ question: { ses_a: [] }, permission: { ses_b: [] } })
|
||||
expect(hasPendingBlockingRequests(state)).toBe(false)
|
||||
})
|
||||
})
|
||||
|
||||
describe("pickDirectoriesToEvict", () => {
|
||||
test("does not evict an idle directory that has a pending question", () => {
|
||||
const stores = ["/idle-with-question", "/idle-empty"]
|
||||
const state = new Map<string, DirState>([
|
||||
["/idle-with-question", { lastAccessAt: 0 }],
|
||||
["/idle-empty", { lastAccessAt: 0 }],
|
||||
])
|
||||
const list = pickDirectoriesToEvict({
|
||||
stores,
|
||||
state,
|
||||
pins: new Set(),
|
||||
max: 30,
|
||||
ttl: 1000,
|
||||
now: DAY_MS,
|
||||
hasPendingBlockingRequests: (dir) => dir === "/idle-with-question",
|
||||
})
|
||||
expect(list).toEqual(["/idle-empty"])
|
||||
})
|
||||
|
||||
test("never includes a directory with pending blocking requests even under overflow pressure", () => {
|
||||
const stores = ["/active", "/overflow-with-permission", "/old-empty"]
|
||||
const state = new Map<string, DirState>([
|
||||
["/active", { lastAccessAt: DAY_MS }],
|
||||
["/overflow-with-permission", { lastAccessAt: DAY_MS - 100_000 }],
|
||||
["/old-empty", { lastAccessAt: 0 }],
|
||||
])
|
||||
const list = pickDirectoriesToEvict({
|
||||
stores,
|
||||
state,
|
||||
pins: new Set(),
|
||||
max: 1,
|
||||
ttl: 60_000,
|
||||
now: DAY_MS,
|
||||
hasPendingBlockingRequests: (dir) => dir === "/overflow-with-permission",
|
||||
})
|
||||
expect(list).not.toContain("/overflow-with-permission")
|
||||
expect(list).toContain("/old-empty")
|
||||
})
|
||||
|
||||
test("falls back to legacy behavior when no predicate is provided", () => {
|
||||
const stores = ["/idle"]
|
||||
const state = new Map<string, DirState>([["/idle", { lastAccessAt: 0 }]])
|
||||
const list = pickDirectoriesToEvict({
|
||||
stores,
|
||||
state,
|
||||
pins: new Set(),
|
||||
max: 30,
|
||||
ttl: 1000,
|
||||
now: DAY_MS,
|
||||
})
|
||||
expect(list).toEqual(["/idle"])
|
||||
})
|
||||
})
|
||||
|
||||
describe("canDisposeDirectory", () => {
|
||||
const baseInput = {
|
||||
directory: "/repo",
|
||||
hasStore: true,
|
||||
pinned: false,
|
||||
booting: false,
|
||||
loadingSessions: false,
|
||||
hasPendingBlockingRequests: false,
|
||||
}
|
||||
|
||||
test("refuses to dispose a directory holding pending blocking requests", () => {
|
||||
expect(canDisposeDirectory({ ...baseInput, hasPendingBlockingRequests: true })).toBe(false)
|
||||
})
|
||||
|
||||
test("permits disposal when no blocking requests are pending", () => {
|
||||
expect(canDisposeDirectory(baseInput)).toBe(true)
|
||||
})
|
||||
})
|
||||
@@ -0,0 +1,159 @@
|
||||
import { describe, expect, test, beforeEach, mock } from "bun:test"
|
||||
import { create, type StoreApi } from "zustand"
|
||||
import type { PermissionRequest, QuestionRequest } from "@opencode-ai/sdk/v2/client"
|
||||
|
||||
const listPendingQuestionsCalls: Array<{ directories?: Array<string | null | undefined> }> = []
|
||||
const listPendingPermissionsCalls: Array<{ directories?: Array<string | null | undefined> }> = []
|
||||
let pendingQuestionsResponse: QuestionRequest[] = []
|
||||
let pendingPermissionsResponse: PermissionRequest[] = []
|
||||
|
||||
mock.module("@/lib/opencode/client", () => ({
|
||||
opencodeClient: {
|
||||
listPendingQuestions: mock(async (opts?: { directories?: Array<string | null | undefined> }) => {
|
||||
listPendingQuestionsCalls.push(opts ?? {})
|
||||
return pendingQuestionsResponse
|
||||
}),
|
||||
listPendingPermissions: mock(async (opts?: { directories?: Array<string | null | undefined> }) => {
|
||||
listPendingPermissionsCalls.push(opts ?? {})
|
||||
return pendingPermissionsResponse
|
||||
}),
|
||||
getDirectory: () => "/repo",
|
||||
getScopedSdkClient: () => ({}),
|
||||
setDirectory: () => undefined,
|
||||
},
|
||||
}))
|
||||
|
||||
mock.module("@/stores/permissionStore", () => ({
|
||||
usePermissionStore: {
|
||||
getState: () => ({ isSessionAutoAccepting: () => false }),
|
||||
},
|
||||
}))
|
||||
|
||||
mock.module("@/stores/useConfigStore", () => ({
|
||||
useConfigStore: {
|
||||
getState: () => ({ isConnected: true, hasEverConnected: true }),
|
||||
setState: () => undefined,
|
||||
},
|
||||
}))
|
||||
|
||||
mock.module("@/stores/useTodosPersistStore", () => ({
|
||||
useTodosPersistStore: { getState: () => ({}) },
|
||||
}))
|
||||
|
||||
mock.module("@/components/ui", () => ({
|
||||
toast: { info: () => undefined, error: () => undefined, success: () => undefined },
|
||||
}))
|
||||
|
||||
import { INITIAL_STATE, type State } from "../types"
|
||||
import type { DirectoryStore } from "../child-store"
|
||||
import { resyncBlockingRequestsForDirectory } from "../sync-context"
|
||||
|
||||
function buildQuestion(overrides: Partial<QuestionRequest> = {}): QuestionRequest {
|
||||
return {
|
||||
id: "que_1",
|
||||
sessionID: "ses_a",
|
||||
questions: [{ question: "Continue?", header: "Q", options: [{ label: "Yes", description: "" }] }],
|
||||
...overrides,
|
||||
} as QuestionRequest
|
||||
}
|
||||
|
||||
function buildPermission(overrides: Partial<PermissionRequest> = {}): PermissionRequest {
|
||||
return {
|
||||
id: "perm_1",
|
||||
sessionID: "ses_a",
|
||||
permission: "bash",
|
||||
patterns: [],
|
||||
metadata: {},
|
||||
always: [],
|
||||
...overrides,
|
||||
} as PermissionRequest
|
||||
}
|
||||
|
||||
function createDirectoryStore(initial: Partial<State>): StoreApi<DirectoryStore> {
|
||||
return create<DirectoryStore>()((set) => ({
|
||||
...INITIAL_STATE,
|
||||
...initial,
|
||||
session: initial.session ?? [{ id: "ses_a", title: "ses_a", time: { created: 1, updated: 1 }, version: "1" } as State["session"][number]],
|
||||
patch: (partial) => set(partial),
|
||||
replace: (next) => set(next),
|
||||
}))
|
||||
}
|
||||
|
||||
describe("resyncBlockingRequestsForDirectory", () => {
|
||||
beforeEach(() => {
|
||||
listPendingQuestionsCalls.length = 0
|
||||
listPendingPermissionsCalls.length = 0
|
||||
pendingQuestionsResponse = []
|
||||
pendingPermissionsResponse = []
|
||||
})
|
||||
|
||||
test("calls listPendingQuestions and listPendingPermissions exactly once for the directory", async () => {
|
||||
const store = createDirectoryStore({})
|
||||
pendingQuestionsResponse = [buildQuestion()]
|
||||
pendingPermissionsResponse = [buildPermission()]
|
||||
|
||||
await resyncBlockingRequestsForDirectory("/repo", store)
|
||||
|
||||
expect(listPendingQuestionsCalls).toHaveLength(1)
|
||||
expect(listPendingQuestionsCalls[0]).toEqual({ directories: ["/repo"] })
|
||||
expect(listPendingPermissionsCalls).toHaveLength(1)
|
||||
expect(listPendingPermissionsCalls[0]).toEqual({ directories: ["/repo"] })
|
||||
})
|
||||
|
||||
test("merges newly fetched questions/permissions into the directory store", async () => {
|
||||
const store = createDirectoryStore({})
|
||||
pendingQuestionsResponse = [buildQuestion()]
|
||||
pendingPermissionsResponse = [buildPermission()]
|
||||
|
||||
await resyncBlockingRequestsForDirectory("/repo", store)
|
||||
|
||||
expect(store.getState().question["ses_a"]).toHaveLength(1)
|
||||
expect(store.getState().question["ses_a"]?.[0]?.id).toBe("que_1")
|
||||
expect(store.getState().permission["ses_a"]).toHaveLength(1)
|
||||
expect(store.getState().permission["ses_a"]?.[0]?.id).toBe("perm_1")
|
||||
})
|
||||
|
||||
test("preserves an in-flight SSE-delivered question whose signature changed during the fetch", async () => {
|
||||
const store = createDirectoryStore({
|
||||
question: { ses_a: [{ ...buildQuestion(), id: "que_initial" }] },
|
||||
})
|
||||
pendingQuestionsResponse = []
|
||||
|
||||
const promise = resyncBlockingRequestsForDirectory("/repo", store)
|
||||
store.setState({
|
||||
question: { ses_a: [{ ...buildQuestion(), id: "que_sse_arrived" }] },
|
||||
})
|
||||
await promise
|
||||
|
||||
expect(store.getState().question["ses_a"]).toHaveLength(1)
|
||||
expect(store.getState().question["ses_a"]?.[0]?.id).toBe("que_sse_arrived")
|
||||
})
|
||||
|
||||
test("clears stale entries when API returns no pending requests and signature unchanged", async () => {
|
||||
const store = createDirectoryStore({
|
||||
question: { ses_a: [{ ...buildQuestion(), id: "que_stale" }] },
|
||||
})
|
||||
pendingQuestionsResponse = []
|
||||
pendingPermissionsResponse = []
|
||||
|
||||
await resyncBlockingRequestsForDirectory("/repo", store)
|
||||
|
||||
expect(store.getState().question["ses_a"]).toEqual(undefined)
|
||||
})
|
||||
|
||||
test("ignores questions for sessions the directory does not know about", async () => {
|
||||
const store = createDirectoryStore({})
|
||||
pendingQuestionsResponse = [{ ...buildQuestion(), sessionID: "ses_unknown" }]
|
||||
|
||||
await resyncBlockingRequestsForDirectory("/repo", store)
|
||||
|
||||
expect(store.getState().question["ses_unknown"]).toEqual(undefined)
|
||||
})
|
||||
|
||||
test("returns early without fetching when no candidate sessions are known", async () => {
|
||||
const store = createDirectoryStore({ session: [] })
|
||||
await resyncBlockingRequestsForDirectory("/repo", store)
|
||||
expect(listPendingQuestionsCalls).toHaveLength(0)
|
||||
expect(listPendingPermissionsCalls).toHaveLength(0)
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user