Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/tui-permission-eviction.md
Original file line number Diff line number Diff line change
@@ -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.
56 changes: 53 additions & 3 deletions packages/tui/src/context/sync.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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]
}),
Expand All @@ -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<T extends PermissionRequest | QuestionRequest>(
list: T[],
current: Record<string, T[]>,
before: Set<string>,
): Record<string, T[]> {
const fresh: Record<string, T[]> = {}
for (const request of list) (fresh[request.sessionID] ??= []).push(request)
const next: Record<string, T[]> = {}
for (const sessionID of new Set([...Object.keys(current), ...Object.keys(fresh)])) {
const merged = new Map<string, T>()
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 })
Comment thread
rakshith1928 marked this conversation as resolved.
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
Expand Down Expand Up @@ -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")
})
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions packages/tui/test/fixture/tui-sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ export function createFetch(override?: FetchHandler, events?: ReturnType<typeof
return json({})
// kilocode_change start - Kilo bootstrap endpoints
if (["/network", "/background-process", "/config/warnings"].includes(url.pathname)) return json([])
if (["/permission", "/question"].includes(url.pathname)) return json([])
if (url.pathname === "/indexing/status")
return json({ state: "Disabled", message: "Indexing disabled.", processedFiles: 0, totalFiles: 0, percent: 0 })
// kilocode_change end
Expand Down
242 changes: 242 additions & 0 deletions packages/tui/test/kilocode/sync-pending-asks.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,242 @@
/** @jsxImportSource @opentui/solid */
import { expect, test } from "bun:test"
import type { GlobalEvent, PermissionRequest, QuestionRequest, Session } from "@kilocode/sdk/v2"
import { tmpdir } from "../fixture/fixture"
import { json, mount, wait } from "../cli/cmd/tui/sync-fixture"

const directory = "/tmp/opencode/packages/tui"
const parentID = "ses_parent"
const childID = "ses_child"

const parent: Session = {
id: parentID,
slug: "parent",
title: "parent",
projectID: "proj_test",
directory,
version: "7.5.15",
time: { created: 1, updated: 1 },
}

const child: Session = {
id: childID,
slug: "child",
title: "child",
projectID: "proj_test",
directory,
version: "7.5.15",
parentID,
time: { created: 2, updated: 2 },
}

function permission(id: string, sessionID = childID): PermissionRequest {
return {
id,
sessionID,
permission: "edit",
patterns: ["src/**"],
metadata: {},
always: [],
}
}

function question(id: string, sessionID = childID): QuestionRequest {
return {
id,
sessionID,
questions: [{ question: "Proceed?", header: "Proceed", options: [{ label: "Yes", description: "" }] }],
}
}

function wrap(payload: GlobalEvent["payload"]): GlobalEvent {
return { directory, project: "proj_test", payload }
}

function serveSessions(sessions: Session[], asks: () => { 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<PermissionRequest[]>((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<PermissionRequest[]>((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()
}
})
Loading