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
37 changes: 32 additions & 5 deletions packages/ui/src/components/tool-call.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,14 @@ import { messageStoreBus } from "../stores/message-v2/bus"
import { useTheme } from "../lib/theme"
import { useGlobalCache } from "../lib/hooks/use-global-cache"
import { useConfig } from "../stores/preferences"
import { activeInterruption, sendPermissionResponse, sendQuestionReject, sendQuestionReply } from "../stores/instances"
import {
activeInterruption,
hasRepliedQuestion,
QuestionExpiredError,
sendPermissionResponse,
sendQuestionReject,
sendQuestionReply,
} from "../stores/instances"
import { copyToClipboard } from "../lib/clipboard"
import type { PermissionRequestLike } from "../types/permission"
import { getPermissionSessionId } from "../types/permission"
Expand Down Expand Up @@ -275,10 +282,18 @@ function ToolCallDetails(props: {
}

async function handleQuestionSubmit() {
// Synchronous double-submit guard BEFORE any await: Enter + button (or a
// rapid re-submit) must never fire two `question.reply` calls.
if (questionSubmitting()) return
const request = questionDetails()
if (!request || !props.isQuestionActive()) {
return
}
// Second defense: a stale request already replied locally (e.g. an SSE
// `question.replied` raced ahead) is a no-op.
if (hasRepliedQuestion(props.instanceId, request.id)) {
return
}
const answers = (questionDraftAnswers()[request.id] ?? []).map((x) => (Array.isArray(x) ? x : []))
const normalized = request.questions.map((_, index) => {
const row = answers[index] ?? []
Expand All @@ -295,26 +310,38 @@ function ToolCallDetails(props: {
const sessionId = (request as any).sessionID ?? (request as any).sessionId ?? props.sessionId
await sendQuestionReply(props.instanceId, sessionId, request.id, normalized)
} catch (error) {
log.error("Failed to send question reply", error)
setQuestionError(error instanceof Error ? error.message : props.t("toolCall.question.errors.unableToReply"))
if (error instanceof QuestionExpiredError) {
setQuestionError(props.t("toolCall.question.errors.expired"))
} else {
log.error("Failed to send question reply", error)
setQuestionError(error instanceof Error ? error.message : props.t("toolCall.question.errors.unableToReply"))
}
} finally {
setQuestionSubmitting(false)
}
}

async function handleQuestionDismiss() {
if (questionSubmitting()) return
const request = questionDetails()
if (!request || !props.isQuestionActive()) {
return
}
if (hasRepliedQuestion(props.instanceId, request.id)) {
return
}
setQuestionSubmitting(true)
setQuestionError(null)
try {
const sessionId = (request as any).sessionID ?? (request as any).sessionId ?? props.sessionId
await sendQuestionReject(props.instanceId, sessionId, request.id)
} catch (error) {
log.error("Failed to reject question", error)
setQuestionError(error instanceof Error ? error.message : props.t("toolCall.question.errors.unableToDismiss"))
if (error instanceof QuestionExpiredError) {
setQuestionError(props.t("toolCall.question.errors.expired"))
} else {
log.error("Failed to reject question", error)
setQuestionError(error instanceof Error ? error.message : props.t("toolCall.question.errors.unableToDismiss"))
}
} finally {
setQuestionSubmitting(false)
}
Expand Down
8 changes: 5 additions & 3 deletions packages/ui/src/lib/client-identity.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,23 @@ export function getClientIdentity(): { clientId: string; connectionId: string }

function getOrCreateClientId(): string {
if (cachedClientId) return cachedClientId
cachedClientId = getOrCreateStoredValue(CLIENT_ID_STORAGE_KEY, window.localStorage)
cachedClientId = getOrCreateStoredValue(CLIENT_ID_STORAGE_KEY, () => window.localStorage)
return cachedClientId
}

function getOrCreateConnectionId(): string {
if (cachedConnectionId) return cachedConnectionId
cachedConnectionId = getOrCreateStoredValue(CONNECTION_ID_STORAGE_KEY, window.sessionStorage)
cachedConnectionId = getOrCreateStoredValue(CONNECTION_ID_STORAGE_KEY, () => window.sessionStorage)
return cachedConnectionId
}

function getOrCreateStoredValue(key: string, storage: Storage): string {
function getOrCreateStoredValue(key: string, getStorage: () => Storage): string {
if (typeof window === "undefined") {
return generateUUID()
}

const storage = getStorage()

try {
const existing = storage.getItem(key)
if (existing && existing.trim()) {
Expand Down
1 change: 1 addition & 0 deletions packages/ui/src/lib/i18n/messages/en/toolCall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export const toolCallMessages = {
"toolCall.question.validation.answerAll": "Please answer all questions before submitting.",
"toolCall.question.errors.unableToReply": "Unable to reply",
"toolCall.question.errors.unableToDismiss": "Unable to dismiss",
"toolCall.question.errors.expired": "This prompt expired. Send your answer as a message instead.",

"toolCall.task.action.delegating": "Delegating...",
"toolCall.task.sections.prompt": "Prompt",
Expand Down
1 change: 1 addition & 0 deletions packages/ui/src/lib/i18n/messages/es/toolCall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export const toolCallMessages = {
"toolCall.question.validation.answerAll": "Responde todas las preguntas antes de enviar.",
"toolCall.question.errors.unableToReply": "No se pudo responder",
"toolCall.question.errors.unableToDismiss": "No se pudo descartar",
"toolCall.question.errors.expired": "Este mensaje expiró. Envía tu respuesta como un mensaje en su lugar.",

"toolCall.task.action.delegating": "Delegando...",
"toolCall.task.sections.prompt": "Prompt",
Expand Down
1 change: 1 addition & 0 deletions packages/ui/src/lib/i18n/messages/fr/toolCall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export const toolCallMessages = {
"toolCall.question.validation.answerAll": "Veuillez répondre à toutes les questions avant d'envoyer.",
"toolCall.question.errors.unableToReply": "Impossible de répondre",
"toolCall.question.errors.unableToDismiss": "Impossible d'ignorer",
"toolCall.question.errors.expired": "Cette invite a expiré. Envoyez plutôt votre réponse sous forme de message.",

"toolCall.task.action.delegating": "Délégation...",
"toolCall.task.sections.prompt": "Prompt",
Expand Down
1 change: 1 addition & 0 deletions packages/ui/src/lib/i18n/messages/he/toolCall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export const toolCallMessages = {
"toolCall.question.validation.answerAll": "אנא ענה על כל השאלות לפני השליחה.",
"toolCall.question.errors.unableToReply": "לא ניתן לשלוח תשובה",
"toolCall.question.errors.unableToDismiss": "לא ניתן לסגור",
"toolCall.question.errors.expired": "תוקף ההודעה הזו פג. שלחו את התשובה כהודעה רגילה במקום.",

"toolCall.task.action.delegating": "מאציל...",
"toolCall.task.sections.prompt": "פקודה",
Expand Down
1 change: 1 addition & 0 deletions packages/ui/src/lib/i18n/messages/ja/toolCall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export const toolCallMessages = {
"toolCall.question.validation.answerAll": "送信する前にすべての質問に回答してください。",
"toolCall.question.errors.unableToReply": "回答できません",
"toolCall.question.errors.unableToDismiss": "閉じられません",
"toolCall.question.errors.expired": "このプロンプトは期限切れです。代わりにメッセージとして回答を送信してください。",

"toolCall.task.action.delegating": "委任中...",
"toolCall.task.sections.prompt": "プロンプト",
Expand Down
1 change: 1 addition & 0 deletions packages/ui/src/lib/i18n/messages/ru/toolCall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export const toolCallMessages = {
"toolCall.question.validation.answerAll": "Ответьте на все вопросы перед отправкой.",
"toolCall.question.errors.unableToReply": "Не удалось ответить",
"toolCall.question.errors.unableToDismiss": "Не удалось скрыть",
"toolCall.question.errors.expired": "Срок действия этого запроса истёк. Отправьте ответ обычным сообщением.",

"toolCall.task.action.delegating": "Делегирование…",
"toolCall.task.sections.prompt": "Prompt",
Expand Down
1 change: 1 addition & 0 deletions packages/ui/src/lib/i18n/messages/zh-Hans/toolCall.ts
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,7 @@ export const toolCallMessages = {
"toolCall.question.validation.answerAll": "请先回答所有问题再提交。",
"toolCall.question.errors.unableToReply": "无法回复",
"toolCall.question.errors.unableToDismiss": "无法忽略",
"toolCall.question.errors.expired": "此提示已过期。请改为以消息形式发送你的回答。",

"toolCall.task.action.delegating": "正在委派...",
"toolCall.task.sections.prompt": "Prompt",
Expand Down
5 changes: 5 additions & 0 deletions packages/ui/src/lib/server-events.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@ class ServerEvents {
}

private connect() {
// EventSource is only available in browser-like runtimes. In SSR/tests the
// global is absent; skip connecting rather than throwing at module load.
if (typeof EventSource === "undefined") {
return
}
if (this.reconnectTimer !== null) {
clearTimeout(this.reconnectTimer)
this.reconnectTimer = null
Expand Down
121 changes: 110 additions & 11 deletions packages/ui/src/stores/instances.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { getPermissionCreatedAt, getPermissionSessionId, mergePermissionRequest
import type { QuestionRequest } from "@opencode-ai/sdk/v2"
import { getQuestionSessionId } from "../types/question"
import { requestData } from "../lib/opencode-api"
import { buildInstanceBaseUrl, sdkManager } from "../lib/sdk-manager"
import { buildInstanceBaseUrl, sdkManager, type OpencodeClient } from "../lib/sdk-manager"
import { sseManager } from "../lib/sse-manager"
import { serverApi } from "../lib/api-client"
import { serverEvents } from "../lib/server-events"
Expand Down Expand Up @@ -38,6 +38,12 @@ import {
markPermissionReplied,
pruneRepliedPermissions,
} from "./permission-replies"
import {
clearRepliedQuestions,
hasRepliedQuestion,
markQuestionReplied,
pruneRepliedQuestions,
} from "./question-replies"
import {
clearAutoAcceptPermission,
drainAutoAcceptPermissions,
Expand Down Expand Up @@ -244,12 +250,19 @@ async function syncPendingQuestions(instanceId: string): Promise<void> {
if (!instance?.client) return

try {
const syncStartedAt = Date.now()
const remote = await requestData<QuestionRequest[]>(
instance.client.question.list(),
"question.list",
)

const remoteIds = new Set(remote.map((item) => item.id))
const remotePendingIds = new Set(remote.map((item) => item.id))
pruneRepliedQuestions(instanceId, remotePendingIds, syncStartedAt)

// Never re-surface a question that was already answered locally; a stale
// server snapshot during reconnect would otherwise re-add it to the queue.
const pendingRemote = remote.filter((item) => !hasRepliedQuestion(instanceId, item.id))
const remoteIds = new Set(pendingRemote.map((item) => item.id))
const local = getQuestionQueue(instanceId)

// Remove any stale local requests missing from server.
Expand All @@ -261,7 +274,7 @@ async function syncPendingQuestions(instanceId: string): Promise<void> {
}

// Upsert all server-side pending questions.
for (const request of remote) {
for (const request of pendingRemote) {
ensureQuestionEnqueuedAt(request)
addQuestionToQueue(instanceId, request)
upsertQuestionV2(instanceId, request)
Expand Down Expand Up @@ -541,6 +554,7 @@ function removeInstance(id: string) {
clearPermissionQueue(id)
clearRepliedPermissions(id)
clearQuestionQueue(id)
clearRepliedQuestions(id)
clearInstanceMetadata(id)

if (activeInstanceId() === id) {
Expand Down Expand Up @@ -1043,6 +1057,58 @@ function setActiveQuestionIdForInstance(instanceId: string, requestId: string):
setActiveInterruptionForInstance(instanceId, { kind: "question", id: requestId })
}

/**
* Thrown when a question reply/reject targets a request the backend no longer
* tracks (e.g. after a reconnect/restart gap). Callers should surface a
* non-fatal "this prompt expired — answer as a message" UX path rather than
* POSTing to a fallback target and triggering opencode's
* `reply for unknown request` warning.
*/
class QuestionExpiredError extends Error {
readonly requestId: string
constructor(requestId: string) {
super(`Question request expired: ${requestId}`)
this.name = "QuestionExpiredError"
this.requestId = requestId
}
}

/**
* Reconcile-before-POST: resolves the worktree client that still tracks the
* question request. Returns the owning client, or null when the request is
* absent from every candidate client's `question.list()` (i.e. expired).
*
* AC-6 (minimal scope): when the stored slug is missing, the session-based
* fallback is consulted via `question.list()` to confirm ownership instead of
* blindly POSTing to the previous `"root"` fallback. Full multi-worktree
* enumeration is intentionally out of scope here.
*/
async function resolveQuestionReplyClient(
instanceId: string,
sessionId: string,
requestId: string,
): Promise<OpencodeClient | null> {
const stored = questionWorktreeSlugByInstance.get(instanceId)?.get(requestId)
const fallback = sessionId ? getWorktreeSlugForSession(instanceId, sessionId) : "root"
// De-duplicate candidate slugs while preserving the stored slug's priority.
const candidateSlugs = Array.from(new Set([stored, fallback, "root"].filter((slug): slug is string => !!slug)))

for (const slug of candidateSlugs) {
const client = getOrCreateWorktreeClient(instanceId, slug)
try {
const remote = await requestData<QuestionRequest[]>(client.question.list(), "question.list")
if (remote.some((item) => item.id === requestId)) {
return client
}
} catch (error) {
// A failed list lookup is non-authoritative; keep checking other clients.
log.warn("Failed to reconcile question worktree", { instanceId, requestId, slug, error })
}
}

return null
}

async function sendQuestionReply(
instanceId: string,
sessionId: string,
Expand All @@ -1054,11 +1120,24 @@ async function sendQuestionReply(
throw new Error("Instance not ready")
}

// Idempotency guard: if we already replied locally, this is a stale re-submit
// (double-submit race or reconnect re-delivery). No-op instead of POSTing.
if (hasRepliedQuestion(instanceId, requestId)) {
log.info("Ignoring duplicate question reply after local reply", { instanceId, requestId })
removeQuestionFromQueue(instanceId, requestId)
return
}

try {
const stored = questionWorktreeSlugByInstance.get(instanceId)?.get(requestId)
const fallback = sessionId ? getWorktreeSlugForSession(instanceId, sessionId) : "root"
const worktreeSlug = stored ?? fallback
const client = getOrCreateWorktreeClient(instanceId, worktreeSlug)
// Reconcile-before-POST: only POST when the request is still pending on a
// resolvable client. Otherwise surface the expired-prompt UX path.
const client = await resolveQuestionReplyClient(instanceId, sessionId, requestId)
if (!client) {
log.info("Question request no longer pending; treating as expired", { instanceId, requestId })
markQuestionReplied(instanceId, requestId)
removeQuestionFromQueue(instanceId, requestId)
throw new QuestionExpiredError(requestId)
}

await requestData(
client.question.reply({
Expand All @@ -1068,8 +1147,12 @@ async function sendQuestionReply(
"question.reply",
)

markQuestionReplied(instanceId, requestId)
removeQuestionFromQueue(instanceId, requestId)
} catch (error) {
if (error instanceof QuestionExpiredError) {
throw error
}
log.error("Failed to send question reply", error)
throw error
}
Expand All @@ -1081,11 +1164,20 @@ async function sendQuestionReject(instanceId: string, sessionId: string, request
throw new Error("Instance not ready")
}

if (hasRepliedQuestion(instanceId, requestId)) {
log.info("Ignoring duplicate question reject after local reply", { instanceId, requestId })
removeQuestionFromQueue(instanceId, requestId)
return
}

try {
const stored = questionWorktreeSlugByInstance.get(instanceId)?.get(requestId)
const fallback = sessionId ? getWorktreeSlugForSession(instanceId, sessionId) : "root"
const worktreeSlug = stored ?? fallback
const client = getOrCreateWorktreeClient(instanceId, worktreeSlug)
const client = await resolveQuestionReplyClient(instanceId, sessionId, requestId)
if (!client) {
log.info("Question request no longer pending; treating as expired", { instanceId, requestId })
markQuestionReplied(instanceId, requestId)
removeQuestionFromQueue(instanceId, requestId)
throw new QuestionExpiredError(requestId)
}

await requestData(
client.question.reject({
Expand All @@ -1094,8 +1186,12 @@ async function sendQuestionReject(instanceId: string, sessionId: string, request
"question.reject",
)

markQuestionReplied(instanceId, requestId)
removeQuestionFromQueue(instanceId, requestId)
} catch (error) {
if (error instanceof QuestionExpiredError) {
throw error
}
log.error("Failed to send question reject", error)
throw error
}
Expand Down Expand Up @@ -1244,8 +1340,11 @@ export {
addQuestionToQueue,
removeQuestionFromQueue,
clearQuestionQueue,
markQuestionReplied,
hasRepliedQuestion,
sendQuestionReply,
sendQuestionReject,
QuestionExpiredError,
setActiveQuestionIdForInstance,
disconnectedInstance,
acknowledgeDisconnectedInstance,
Expand Down
Loading
Loading