fix: improve question handling in chat
Updates question card behavior Improves localized question text Covers session question actions with tests
This commit is contained in:
@@ -1,9 +1,12 @@
|
||||
import { describe, expect, test, beforeEach, mock } from "bun:test"
|
||||
import type { PermissionRequest } from "@/types/permission"
|
||||
import type { QuestionRequest } from "@/types/question"
|
||||
|
||||
// Mock SDK client that records permission.reply / question.reply calls
|
||||
const replyCalls: Array<{ method: string; params: Record<string, unknown> }> = []
|
||||
const scopedClientDirectories: string[] = []
|
||||
let sessionRevertResult: { data?: unknown; error?: unknown; response?: { status?: number } } = {}
|
||||
let questionReplyError: unknown | null = null
|
||||
|
||||
const mockScopedClient = {
|
||||
permission: {
|
||||
@@ -15,6 +18,9 @@ const mockScopedClient = {
|
||||
question: {
|
||||
reply: mock((params: Record<string, unknown>) => {
|
||||
replyCalls.push({ method: "question.reply", params })
|
||||
if (questionReplyError) {
|
||||
return Promise.resolve({ error: questionReplyError, response: { status: 404 } })
|
||||
}
|
||||
return Promise.resolve({ data: true })
|
||||
}),
|
||||
reject: mock((params: Record<string, unknown>) => {
|
||||
@@ -48,6 +54,9 @@ const mockSdk = {
|
||||
question: {
|
||||
reply: mock((params: Record<string, unknown>) => {
|
||||
replyCalls.push({ method: "question.reply", params })
|
||||
if (questionReplyError) {
|
||||
return Promise.resolve({ error: questionReplyError, response: { status: 404 } })
|
||||
}
|
||||
return Promise.resolve({ data: true })
|
||||
}),
|
||||
reject: mock((params: Record<string, unknown>) => {
|
||||
@@ -60,8 +69,10 @@ const mockSdk = {
|
||||
// Mock opencodeClient singleton
|
||||
mock.module("@/lib/opencode/client", () => ({
|
||||
opencodeClient: {
|
||||
// eslint-disable-next-line @typescript-eslint/no-unused-vars
|
||||
getScopedSdkClient: (_: string) => mockScopedClient,
|
||||
getScopedSdkClient: (directory: string) => {
|
||||
scopedClientDirectories.push(directory)
|
||||
return mockScopedClient
|
||||
},
|
||||
getDirectory: () => "/test/project",
|
||||
replyToPermission: mock((requestId: string, reply: string, options?: { directory?: string | null }) => {
|
||||
replyCalls.push({ method: "permission.reply", params: { requestID: requestId, reply, directory: options?.directory } })
|
||||
@@ -170,6 +181,7 @@ function createChildStores(entries: Array<[string, StoreApi<DirectoryStore>]>) {
|
||||
describe("respondToPermission passes directory", () => {
|
||||
beforeEach(() => {
|
||||
replyCalls.length = 0
|
||||
scopedClientDirectories.length = 0
|
||||
sessionRevertResult = {}
|
||||
})
|
||||
|
||||
@@ -229,6 +241,7 @@ describe("respondToPermission passes directory", () => {
|
||||
describe("revertToMessage passes session directory", () => {
|
||||
beforeEach(() => {
|
||||
replyCalls.length = 0
|
||||
scopedClientDirectories.length = 0
|
||||
sessionRevertResult = {}
|
||||
Object.assign(inputState, {
|
||||
pendingInputText: "previous draft",
|
||||
@@ -296,6 +309,8 @@ describe("revertToMessage passes session directory", () => {
|
||||
describe("dismissPermission passes directory", () => {
|
||||
beforeEach(() => {
|
||||
replyCalls.length = 0
|
||||
scopedClientDirectories.length = 0
|
||||
questionReplyError = null
|
||||
})
|
||||
|
||||
test("passes directory and reply=reject", async () => {
|
||||
@@ -326,6 +341,8 @@ describe("dismissPermission passes directory", () => {
|
||||
describe("respondToQuestion passes directory", () => {
|
||||
beforeEach(() => {
|
||||
replyCalls.length = 0
|
||||
scopedClientDirectories.length = 0
|
||||
questionReplyError = null
|
||||
})
|
||||
|
||||
test("passes directory to question.reply", async () => {
|
||||
@@ -339,12 +356,45 @@ describe("respondToQuestion passes directory", () => {
|
||||
expect(replyCalls.length).toBe(1)
|
||||
expect(replyCalls[0].params.requestID).toBe("q-1")
|
||||
expect(replyCalls[0].params.directory).toBe("/test/project")
|
||||
expect(scopedClientDirectories).toEqual(["/test/project"])
|
||||
})
|
||||
|
||||
test("removes stale question from child store when reply returns not found", async () => {
|
||||
const question: QuestionRequest = {
|
||||
id: "q-stale",
|
||||
sessionID: "session-a",
|
||||
questions: [
|
||||
{
|
||||
question: "Choose an option",
|
||||
header: "Choice",
|
||||
options: [{ label: "Yes", description: "Proceed" }],
|
||||
},
|
||||
],
|
||||
}
|
||||
const store = createStore({}, { question: { "session-a": [question] } })
|
||||
const childStores = createChildStores([["/test/project", store]])
|
||||
questionReplyError = Object.assign(new Error("question.reply failed (404): QuestionNotFoundError"), { status: 404 })
|
||||
|
||||
const { setActionRefs, respondToQuestion } = await import("./session-actions")
|
||||
setActionRefs(mockSdk as unknown as OpencodeClient, childStores, () => "/test/project")
|
||||
|
||||
let thrown: unknown
|
||||
try {
|
||||
await respondToQuestion("session-a", "q-stale", [["Yes"]])
|
||||
} catch (error) {
|
||||
thrown = error
|
||||
}
|
||||
|
||||
expect(thrown).toBeInstanceOf(Error)
|
||||
expect(store.getState().question["session-a"]).toBe(undefined)
|
||||
})
|
||||
})
|
||||
|
||||
describe("rejectQuestion passes directory", () => {
|
||||
beforeEach(() => {
|
||||
replyCalls.length = 0
|
||||
scopedClientDirectories.length = 0
|
||||
questionReplyError = null
|
||||
})
|
||||
|
||||
test("passes directory to question.reject", async () => {
|
||||
|
||||
@@ -60,7 +60,9 @@ function formatSdkError(error: unknown): string {
|
||||
function assertSdkSuccess<T>(result: SdkResult<T>, operation: string): T | undefined {
|
||||
if (!result.error) return result.data
|
||||
const status = result.response?.status
|
||||
throw new Error(`${operation} failed${status ? ` (${status})` : ""}: ${formatSdkError(result.error)}`)
|
||||
const error = new Error(`${operation} failed${status ? ` (${status})` : ""}: ${formatSdkError(result.error)}`) as Error & { status?: number }
|
||||
if (status !== undefined) error.status = status
|
||||
throw error
|
||||
}
|
||||
|
||||
function assertSdkData<T>(result: SdkResult<T>, operation: string): T {
|
||||
@@ -256,6 +258,56 @@ function resolveDirectoryForBlockingRequest(
|
||||
return null
|
||||
}
|
||||
|
||||
export function isQuestionRequestNotFoundError(error: unknown): boolean {
|
||||
if (error && typeof error === "object") {
|
||||
const status = (error as { status?: unknown }).status
|
||||
if (status === 404) return true
|
||||
}
|
||||
|
||||
let message = ""
|
||||
if (error instanceof Error) {
|
||||
message = error.message
|
||||
} else if (typeof error === "string") {
|
||||
message = error
|
||||
}
|
||||
|
||||
return /Question(?:\.)?NotFoundError|Question request not found/i.test(message)
|
||||
}
|
||||
|
||||
function removeQuestionRequestFromChildStores(sessionId: string, requestId: string): boolean {
|
||||
const stores = _childStores
|
||||
if (!stores || !requestId) return false
|
||||
|
||||
let removed = false
|
||||
for (const [, store] of stores.children) {
|
||||
const current = store.getState().question ?? {}
|
||||
let nextQuestion: typeof current | null = null
|
||||
const sessionIds = new Set([sessionId, ...Object.keys(current)].filter(Boolean))
|
||||
|
||||
for (const candidateSessionId of sessionIds) {
|
||||
const requests = current[candidateSessionId]
|
||||
if (!requests?.length) continue
|
||||
|
||||
const nextRequests = requests.filter((request) => request.id !== requestId)
|
||||
if (nextRequests.length === requests.length) continue
|
||||
|
||||
nextQuestion ??= { ...current }
|
||||
if (nextRequests.length > 0) {
|
||||
nextQuestion[candidateSessionId] = nextRequests
|
||||
} else {
|
||||
delete nextQuestion[candidateSessionId]
|
||||
}
|
||||
removed = true
|
||||
}
|
||||
|
||||
if (nextQuestion) {
|
||||
store.setState({ question: nextQuestion })
|
||||
}
|
||||
}
|
||||
|
||||
return removed
|
||||
}
|
||||
|
||||
function getRequestReplyClient(
|
||||
type: "permission" | "question",
|
||||
sessionId: string,
|
||||
@@ -584,7 +636,12 @@ export async function respondToPermission(
|
||||
const directory = resolveDirectoryForBlockingRequest("permission", sessionId, requestId)
|
||||
|| getSessionDirectory(sessionId)
|
||||
|| dir()
|
||||
if (await opencodeClient.replyToPermission(requestId, response, { directory }) !== true) {
|
||||
const result = await getRequestReplyClient("permission", sessionId, requestId).permission.reply({
|
||||
requestID: requestId,
|
||||
reply: response,
|
||||
...(directory ? { directory } : {}),
|
||||
})
|
||||
if (assertSdkData(result, "permission.reply") !== true) {
|
||||
throw new Error("Permission reply failed")
|
||||
}
|
||||
}
|
||||
@@ -597,7 +654,12 @@ export async function dismissPermission(
|
||||
const directory = resolveDirectoryForBlockingRequest("permission", sessionId, requestId)
|
||||
|| getSessionDirectory(sessionId)
|
||||
|| dir()
|
||||
if (await opencodeClient.replyToPermission(requestId, "reject", { directory }) !== true) {
|
||||
const result = await getRequestReplyClient("permission", sessionId, requestId).permission.reply({
|
||||
requestID: requestId,
|
||||
reply: "reject",
|
||||
...(directory ? { directory } : {}),
|
||||
})
|
||||
if (assertSdkData(result, "permission.reply") !== true) {
|
||||
throw new Error("Permission dismissal failed")
|
||||
}
|
||||
}
|
||||
@@ -615,8 +677,25 @@ export async function respondToQuestion(
|
||||
const directory = resolveDirectoryForBlockingRequest("question", sessionId, requestId)
|
||||
|| getSessionDirectory(sessionId)
|
||||
|| dir()
|
||||
if (await opencodeClient.replyToQuestion(requestId, answers, directory) !== true) {
|
||||
throw new Error("Question reply failed")
|
||||
try {
|
||||
const normalizedAnswers = answers.length === 0
|
||||
? []
|
||||
: Array.isArray(answers[0])
|
||||
? answers as string[][]
|
||||
: [answers as string[]]
|
||||
const result = await getRequestReplyClient("question", sessionId, requestId).question.reply({
|
||||
requestID: requestId,
|
||||
answers: normalizedAnswers,
|
||||
...(directory ? { directory } : {}),
|
||||
})
|
||||
if (assertSdkData(result, "question.reply") !== true) {
|
||||
throw new Error("Question reply failed")
|
||||
}
|
||||
} catch (error) {
|
||||
if (isQuestionRequestNotFoundError(error)) {
|
||||
removeQuestionRequestFromChildStores(sessionId, requestId)
|
||||
}
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
@@ -628,12 +707,19 @@ export async function rejectQuestion(
|
||||
const directory = resolveDirectoryForBlockingRequest("question", sessionId, requestId)
|
||||
|| getSessionDirectory(sessionId)
|
||||
|| dir()
|
||||
const result = await getRequestReplyClient("question", sessionId, requestId).question.reject({
|
||||
requestID: requestId,
|
||||
...(directory ? { directory } : {}),
|
||||
})
|
||||
if (assertSdkData(result, "question.reject") !== true) {
|
||||
throw new Error("Question rejection failed")
|
||||
try {
|
||||
const result = await getRequestReplyClient("question", sessionId, requestId).question.reject({
|
||||
requestID: requestId,
|
||||
...(directory ? { directory } : {}),
|
||||
})
|
||||
if (assertSdkData(result, "question.reject") !== true) {
|
||||
throw new Error("Question rejection failed")
|
||||
}
|
||||
} catch (error) {
|
||||
if (isQuestionRequestNotFoundError(error)) {
|
||||
removeQuestionRequestFromChildStores(sessionId, requestId)
|
||||
}
|
||||
throw error
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user