diff --git a/src/lib/session-status-reconcile.test.ts b/src/lib/session-status-reconcile.test.ts new file mode 100644 index 0000000..6219cf4 --- /dev/null +++ b/src/lib/session-status-reconcile.test.ts @@ -0,0 +1,57 @@ +import { test } from "node:test" +import assert from "node:assert/strict" +import { isSessionActuallyIdle } from "./session-status-reconcile.ts" +import type { Message } from "./sdk.ts" + +const userMsg = (id: string, overrides: Partial = {}): Message => ({ + id, + sessionID: "s1", + role: "user", + time: { created: 1 }, + ...overrides, +}) + +const assistantMsg = (id: string, overrides: Partial = {}): Message => ({ + id, + sessionID: "s1", + role: "assistant", + time: { created: 1 }, + ...overrides, +}) + +test("no messages -> still busy (nothing to reconcile from)", () => { + assert.equal(isSessionActuallyIdle(undefined), false) + assert.equal(isSessionActuallyIdle(null), false) + assert.equal(isSessionActuallyIdle([]), false) +}) + +test("last message is a completed assistant reply -> idle (the missed-event case)", () => { + const messages = [userMsg("u1"), assistantMsg("a1", { time: { created: 1, completed: 5 } })] + assert.equal(isSessionActuallyIdle(messages), true) +}) + +test("last message is an assistant reply that errored out -> idle", () => { + const messages = [userMsg("u1"), assistantMsg("a1", { error: { message: "boom" } })] + assert.equal(isSessionActuallyIdle(messages), true) +}) + +test("last message is a user prompt awaiting a reply -> still busy", () => { + const messages = [assistantMsg("a1", { time: { created: 1, completed: 5 } }), userMsg("u2")] + assert.equal(isSessionActuallyIdle(messages), false) +}) + +test("last message is an assistant reply still streaming (no completed, no error) -> still busy", () => { + const messages = [userMsg("u1"), assistantMsg("a1")] + assert.equal(isSessionActuallyIdle(messages), false) +}) + +test("a follow-up user prompt after a completed assistant reply -> still busy again", () => { + // Server queued a second turn: the previous reply completed, but a new user + // message was appended after it, so the run may be in progress again. + const messages = [ + userMsg("u1"), + assistantMsg("a1", { time: { created: 1, completed: 5 } }), + userMsg("u2"), + ] + assert.equal(isSessionActuallyIdle(messages), false) +}) diff --git a/src/lib/session-status-reconcile.ts b/src/lib/session-status-reconcile.ts new file mode 100644 index 0000000..84c6c74 --- /dev/null +++ b/src/lib/session-status-reconcile.ts @@ -0,0 +1,31 @@ +import type { Message } from "./sdk" + +/** + * Decide whether a session the client believes is "busy" has actually + * finished, based on the tail of its message history. + * + * Why this exists (issue #123): `sessionStatus`/`sending` are SSE-driven — + * the server's busy -> idle `session.status` event is the only thing that + * normally clears them. If the network drops while a session is busy and + * that busy -> idle event fires DURING the outage, it is lost: SSE reconnect + * resumes the stream from "now", it does not replay missed events. Without a + * resync, the UI shows a stuck 'processing' spinner forever even though the + * server finished long ago. + * + * Heuristic: idle iff the most recent message is an assistant message that + * has terminated — either it completed normally (`time.completed` set) or it + * ended in error (`error` set; the server still finalizes the message on + * error, it just never gets a successful `time.completed`). Anything else — + * the last message is a user prompt still awaiting a reply, or an assistant + * message that hasn't finished streaming — means the run may still be in + * progress server-side. Callers MUST treat that as "still busy" and leave the + * local state alone: this heuristic only ever clears a stale busy flag, it + * never forces a session busy that the server hasn't reported as such, so a + * genuinely still-busy session is never clobbered. + */ +export function isSessionActuallyIdle(messages: Message[] | null | undefined): boolean { + if (!messages || messages.length === 0) return false + const last = messages[messages.length - 1] + if (last.role !== "assistant") return false + return Boolean(last.time?.completed) || Boolean(last.error) +} diff --git a/src/stores/events.ts b/src/stores/events.ts index 9256eeb..c53a848 100644 --- a/src/stores/events.ts +++ b/src/stores/events.ts @@ -8,6 +8,7 @@ import { addBreadcrumb } from "../lib/sentry" import { AnalyticsEvent, track } from "../lib/analytics" import { recordSuccessfulSession } from "../lib/store-review" import { isAuthError } from "../lib/api-error" +import { isSessionActuallyIdle } from "../lib/session-status-reconcile" import type { Client, Part, Session, Message } from "../lib/sdk" // Session status from the server @@ -88,6 +89,62 @@ export async function refreshPending(client: Client, sessionID: string) { } } +// Re-sync any session currently marked "busy" against the server after an +// SSE reconnect. sessionStatus/sending are SSE-driven and there is normally +// no other path to idle — if the server's busy -> idle `session.status` +// event fired while the network was down, SSE reconnect resumes the stream +// from "now" (it does not replay missed events), so without this the busy +// flag would never clear and the UI would show a stuck 'processing' spinner +// forever (issue #123). +// +// Only ever CLEARS a busy flag the server confirms is stale via +// isSessionActuallyIdle — it never marks a session busy, so it can't +// clobber a genuinely still-busy session. Also re-checks sessionStatus right +// before writing, so a real session.status event that lands while the fetch +// is in flight (e.g. the session went busy again) wins over this resync. +async function resyncBusySessions() { + const busySessionIDs = Object.entries(useEvents.getState().sessionStatus) + .filter(([, status]) => status.type === "busy") + .map(([sessionID]) => sessionID) + if (busySessionIDs.length === 0) return + + await Promise.all( + busySessionIDs.map(async (sessionID) => { + try { + const sessionsState = useSessions.getState() + const session = + sessionsState.sessions.find((s) => s.id === sessionID) ?? + (sessionsState.currentSession?.id === sessionID ? sessionsState.currentSession : undefined) + const connState = useConnections.getState() + const client = session?.directory + ? connState.clientForDirectory(session.directory) ?? connState.client + : connState.client + if (!client) return + + const response = await client.session.messages(sessionID) + const messages = (response || []).map((m) => m.info) + if (!isSessionActuallyIdle(messages)) return // server says still busy - leave it alone + + // A fresh session.status event may have landed on the SSE stream + // while this fetch was in flight — that's authoritative, don't + // stomp on it. + if (useEvents.getState().sessionStatus[sessionID]?.type !== "busy") return + + useEvents.setState((state) => ({ + sessionStatus: { ...state.sessionStatus, [sessionID]: { type: "idle" } }, + statusText: { ...state.statusText, [sessionID]: "" }, + })) + useSessions.setState((state) => ({ sending: { ...state.sending, [sessionID]: false } })) + if (useSessions.getState().currentSession?.id === sessionID) { + useSessions.getState().refreshMessages() + } + } catch (err) { + console.warn("[Events] Failed to resync session status for", sessionID, err) + } + }), + ) +} + export const useEvents = create((set, get) => ({ connected: false, authError: false, @@ -118,6 +175,12 @@ export const useEvents = create((set, get) => ({ // Run in background ;(async () => { let reconnectScheduled = false + // True if this connect() call is resuming after a prior disconnect — + // gates the one-time busy-session resync below so a cold app start + // (sessionStatus is always empty then) never triggers it, and a run of + // failed retries can't re-arm the check on every attempt. + const isReconnect = get().reconnectAttempts > 0 + let resyncedAfterReconnect = false const stableTimer = setTimeout(() => { if (!currentController.signal.aborted) { set({ reconnectAttempts: 0, lastDisconnectAt: null }) @@ -163,6 +226,14 @@ export const useEvents = create((set, get) => ({ for await (const event of client.global.events(currentController.signal)) { if (currentController.signal.aborted) break + // The stream is genuinely live again (we're actually receiving + // data, not just optimistically marked "connected") — resync once + // per reconnect, not on every event. + if (isReconnect && !resyncedAfterReconnect) { + resyncedAfterReconnect = true + void resyncBusySessions() + } + const payload = (event as any).payload || event const type = payload.type as string const props = payload.properties || {}