diff --git a/.changeset/tui-permission-eviction.md b/.changeset/tui-permission-eviction.md new file mode 100644 index 000000000000..f68198ee11a0 --- /dev/null +++ b/.changeset/tui-permission-eviction.md @@ -0,0 +1,7 @@ +--- +"@kilocode/cli": patch +--- + +Fix pending permission and question prompts disappearing when switching sessions in the TUI. + +Viewing another session and returning used to drop the approval popup of a running subagent or session, leaving it blocked forever. Pending asks now survive session switches and are refetched from the server when a session becomes visible, so the prompt can always be answered. diff --git a/packages/tui/src/context/sync.tsx b/packages/tui/src/context/sync.tsx index 7d70f1e09494..a5016ec31d49 100644 --- a/packages/tui/src/context/sync.tsx +++ b/packages/tui/src/context/sync.tsx @@ -177,8 +177,8 @@ export const { const processes = draft.background_process[sessionID]?.filter((item) => item.lifetime === "persistent") if (processes?.length) draft.background_process[sessionID] = processes else delete draft.background_process[sessionID] - delete draft.permission[sessionID] - delete draft.question[sessionID] + // pending asks are one-shot events; an unanswered ask hangs its session forever, so + // eviction keeps them and permission.replied/question.replied remove them delete draft.suggestion[sessionID] delete draft.network[sessionID] }), @@ -187,6 +187,54 @@ export const { for (const child of children) evict(child) } + // pending asks are one-shot events; refetch them so an evicted or missed ask cannot strand a session + function mergePending( + list: T[], + current: Record, + before: Set, + ): Record { + const fresh: Record = {} + for (const request of list) (fresh[request.sessionID] ??= []).push(request) + const next: Record = {} + for (const sessionID of new Set([...Object.keys(current), ...Object.keys(fresh)])) { + const merged = new Map() + for (const request of fresh[sessionID] ?? []) { + // skip entries the store already dropped (replied mid-fetch): the stale list resurrects answered asks + if (before.has(request.id) && !(current[sessionID] ?? []).some((r) => r.id === request.id)) continue + merged.set(request.id, request) + } + for (const request of current[sessionID] ?? []) { + if (merged.has(request.id)) continue + if (before.has(request.id)) continue // the server list no longer holds it + merged.set(request.id, request) + } + if (merged.size) next[sessionID] = [...merged.values()].sort((a, b) => a.id.localeCompare(b.id)) + } + return next + } + + async function syncPending() { + const workspace = project.workspace.current() + const before = { + permission: new Set(Object.values(store.permission).flatMap((list) => list.map((r) => r.id))), + question: new Set(Object.values(store.question).flatMap((list) => list.map((r) => r.id))), + } + const [permissions, questions] = await Promise.all([ + // throwOnError so a failed list fetch rejects into the caller's catch instead of + // merging an empty list, which would drop live asks and re-hang the session + sdk.client.permission.list({ workspace }, { throwOnError: true }).then((x) => x.data ?? []), + sdk.client.question.list({ workspace }, { throwOnError: true }).then((x) => x.data ?? []), + ]) + if (permission.mode === "auto") { + for (const request of permissions) + void sdk.client.permission.reply({ requestID: request.id, reply: "once", workspace }) + setStore("permission", reconcile({})) + } else { + setStore("permission", reconcile(mergePending(permissions, store.permission, before.permission))) + } + setStore("question", reconcile(mergePending(questions, store.question, before.question))) + } + function strip(message: Message): Message { if (message.role !== "user" || !message.summary?.diffs) return message return { ...message, summary: { ...message.summary, diffs: [] } } as Message @@ -850,7 +898,7 @@ export const { sdk.client.indexing .status() .then((result) => setStore("indexing", reconcile(result.data ?? store.indexing))), - // kilocode_change end + syncPending().catch(() => {}), // kilocode_change - recover pending asks missed while disconnected ]).then(() => { setStore("status", "complete") }) @@ -991,6 +1039,8 @@ export const { }), ) fullSyncedSessions.add(sessionID) + // a failed pending-ask recovery must not fail the session load; the next visit retries it + await syncPending().catch(() => {}) // kilocode_change - recover pending asks lost to eviction or a missed one-shot event })().finally(() => { syncingSessions.delete(sessionID) hydratingSessions.delete(sessionID) diff --git a/packages/tui/test/fixture/tui-sdk.ts b/packages/tui/test/fixture/tui-sdk.ts index 4d1e8e2f26c8..46a5faad17ac 100644 --- a/packages/tui/test/fixture/tui-sdk.ts +++ b/packages/tui/test/fixture/tui-sdk.ts @@ -89,6 +89,7 @@ export function createFetch(override?: FetchHandler, events?: ReturnType { permission?: PermissionRequest[]; question?: QuestionRequest[] }) { + return (url: URL) => { + if (url.pathname === "/session") return json(sessions) + for (const session of sessions) { + if (url.pathname === `/session/${session.id}`) return json(session) + if (url.pathname === `/session/${session.id}/message`) return json([]) + if (url.pathname === `/session/${session.id}/todo` || url.pathname === `/session/${session.id}/diff`) return json([]) + } + if (url.pathname === "/permission") return json(asks().permission ?? []) + if (url.pathname === "/question") return json(asks().question ?? []) + return undefined + } +} + +test("evicting a parent session keeps pending child permission and question asks", async () => { + await using tmp = await tmpdir() + await Bun.write(`${tmp.path}/kv.json`, "{}") + const { app, emit, sync } = await mount(serveSessions([parent, child], () => ({})), tmp.path) + + try { + emit(wrap({ id: "evt_ask", type: "permission.asked", properties: permission("per_1") })) + emit(wrap({ id: "evt_question", type: "question.asked", properties: question("que_1") })) + await wait(() => (sync.data.permission[childID] ?? []).length === 1) + await wait(() => (sync.data.question[childID] ?? []).length === 1) + + sync.session.evict(parentID) + + expect(sync.data.permission[childID]).toHaveLength(1) + expect(sync.data.question[childID]).toHaveLength(1) + + sync.session.evict(childID) + + expect(sync.data.permission[childID]).toHaveLength(1) + expect(sync.data.question[childID]).toHaveLength(1) + + emit( + wrap({ id: "evt_replied", type: "permission.replied", properties: { sessionID: childID, requestID: "per_1", reply: "once" } }), + ) + emit( + wrap({ + id: "evt_qreplied", + type: "question.replied", + properties: { sessionID: childID, requestID: "que_1", answers: [] }, + }), + ) + await wait(() => (sync.data.permission[childID] ?? []).length === 0) + await wait(() => (sync.data.question[childID] ?? []).length === 0) + } finally { + app.renderer.destroy() + } +}) + +test("session sync refetches pending permission and question asks and drops stale ones", async () => { + await using tmp = await tmpdir() + await Bun.write(`${tmp.path}/kv.json`, "{}") + let pending = { permission: [permission("per_1")], question: [question("que_1")] } + const { app, sync } = await mount(serveSessions([parent, child], () => pending), tmp.path) + + try { + // Eviction no longer wipes the asks; simulate losing them anyway (e.g. the + // ask arrived while the SSE stream was briefly down) and resync. + sync.set("permission", { [childID]: [] }) + sync.set("question", { [childID]: [] }) + await sync.session.sync(childID) + + expect(sync.data.permission[childID]).toHaveLength(1) + expect(sync.data.question[childID]).toHaveLength(1) + + // A later sync with no pending asks drops the stale entries. + pending = { permission: [], question: [] } + await sync.session.sync(parentID) + + expect(sync.data.permission[childID]).toBeUndefined() + expect(sync.data.question[childID]).toBeUndefined() + } finally { + app.renderer.destroy() + } +}) + +test("an ask arriving while the pending refetch is in flight survives it", async () => { + await using tmp = await tmpdir() + await Bun.write(`${tmp.path}/kv.json`, "{}") + let resolveSecond!: (value: PermissionRequest[]) => void + const second = new Promise((resolve) => { + resolveSecond = resolve + }) + let seen = 0 + const { app, emit, sync } = await mount( + (url) => { + if (url.pathname === "/permission") { + seen += 1 + // bootstrap consumes the first list call; the session sync holds the second + return seen === 1 ? json([]) : second.then((data) => json(data)) + } + return serveSessions([parent, child], () => ({}))(url) + }, + tmp.path, + ) + + try { + // The store starts empty (e.g. the ask raced the SSE stream). + sync.set("permission", { [childID]: [] }) + const hydrate = sync.session.sync(childID) + await wait(() => seen === 2) + // The server list resolves with no pending asks, but the live ask event + // lands while the refetch is still settling. + resolveSecond([]) + emit(wrap({ id: "evt_ask", type: "permission.asked", properties: permission("per_1") })) + await hydrate + + expect(sync.data.permission[childID]).toHaveLength(1) + } finally { + app.renderer.destroy() + } +}) + +test("an ask answered while the pending refetch is in flight is not resurrected", async () => { + await using tmp = await tmpdir() + await Bun.write(`${tmp.path}/kv.json`, "{}") + let resolveSecond!: (value: PermissionRequest[]) => void + const second = new Promise((resolve) => { + resolveSecond = resolve + }) + let seen = 0 + const { app, emit, sync } = await mount( + (url) => { + if (url.pathname === "/permission") { + seen += 1 + // bootstrap consumes the first list call; the session sync holds the second + return seen === 1 ? json([]) : second.then((data) => json(data)) + } + return serveSessions([parent, child], () => ({}))(url) + }, + tmp.path, + ) + + try { + // The store holds a pending ask; the refetch starts while it is still live. + sync.set("permission", { [childID]: [permission("per_1")] }) + const hydrate = sync.session.sync(childID) + await wait(() => seen === 2) + // The ask is answered while the (stale) server list is still in flight. + emit( + wrap({ + id: "evt_replied", + type: "permission.replied", + properties: { sessionID: childID, requestID: "per_1", reply: "once" }, + }), + ) + resolveSecond([permission("per_1")]) + await hydrate + + // The stale list must not resurrect the answered ask. + expect(sync.data.permission[childID]).toBeUndefined() + } finally { + app.renderer.destroy() + } +}) + +test("a failed pending list fetch keeps existing asks", async () => { + await using tmp = await tmpdir() + await Bun.write(`${tmp.path}/kv.json`, "{}") + let seen = 0 + const { app, sync } = await mount( + (url) => { + if (url.pathname === "/permission") { + seen += 1 + // bootstrap consumes the first list call; the session sync gets a 500 + return seen === 1 ? json([]) : json({ message: "boom" }, { status: 500 }) + } + return serveSessions([parent, child], () => ({}))(url) + }, + tmp.path, + ) + + try { + // A live ask is in the store; the refetch then fails. + sync.set("permission", { [childID]: [permission("per_1")] }) + const hydrate = sync.session.sync(childID) + await wait(() => seen === 2) + await hydrate + + // The failed fetch must not merge an empty list over the live ask. + expect(sync.data.permission[childID]).toHaveLength(1) + } finally { + app.renderer.destroy() + } +})