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
6 changes: 5 additions & 1 deletion hub/src/web/routes/guards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,11 @@ export function requireSession(
return c.json({ error }, status)
}
if (options?.requireActive && !access.session.active) {
return c.json({ error: 'Session is inactive' }, 409)
// `code` lets the web client discriminate the inactive-session 409 from
// other 4xx without string-matching the human message (which is i18n'd
// by the consumer and may change). See web onError handler in
// router.tsx which surfaces a Reopen affordance on this code.
return c.json({ error: 'Session is inactive', code: 'session_inactive' }, 409)
}
return { sessionId: access.sessionId, session: access.session }
}
Expand Down
24 changes: 24 additions & 0 deletions hub/src/web/routes/messages.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,3 +217,27 @@ describe('POST /api/sessions/:id/messages — scheduledAt + attachments rejected
expect(sentMessages).toHaveLength(1)
})
})

// ---------------------------------------------------------------------------
// #918: inactive session 409 carries a machine-readable code
// ---------------------------------------------------------------------------

describe('POST /api/sessions/:id/messages — inactive session response shape', () => {
it('returns 409 with code "session_inactive" when sending to an inactive session', async () => {
const { app, sentMessages } = createApp({ active: false })

const response = await app.request('/api/sessions/session-1/messages', {
method: 'POST',
headers: { 'content-type': 'application/json' },
body: JSON.stringify({ text: 'hello', localId: 'local-inactive' })
})

expect(response.status).toBe(409)
const body = await response.json() as { error: string; code: string }
expect(body.error).toBe('Session is inactive')
// Web client discriminates this branch via `code` without string-matching
// the human message; see useSendMessage onError consumer in router.tsx.
expect(body.code).toBe('session_inactive')
expect(sentMessages).toHaveLength(0)
})
})
24 changes: 22 additions & 2 deletions web/src/components/AssistantChat/HappyComposer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ export interface TextInputState {
* or null for an immediate send. When non-null, the composer also
* restores the schedule via `onSchedule` so the operator can edit and
* retry without silently downgrading a scheduled send to immediate.
* - `action` is an optional recovery affordance rendered as a button next
* to the message. Used by the inactive-session branch (#918) to expose
* a one-click Reopen. Other failure modes (5xx, network, generic 4xx)
* leave this null and only render the message.
*
* Owned by the route component (`router.tsx`); the composer is a pure
* consumer that:
Expand All @@ -63,6 +67,11 @@ export type ComposerSendError = {
text: string
message: string
scheduledAt: number | null
action?: {
label: string
onClick: () => void
pending?: boolean
} | null
}

const defaultSuggestionHandler = async (): Promise<Suggestion[]> => []
Expand Down Expand Up @@ -983,9 +992,20 @@ export function HappyComposer(props: {
<div
role="alert"
data-testid="composer-send-error"
className="mb-2 rounded-md bg-[var(--app-subtle-bg)] px-3 py-2 text-sm text-red-600"
className="mb-2 flex items-center justify-between gap-3 rounded-md bg-[var(--app-subtle-bg)] px-3 py-2 text-sm text-red-600"
>
{sendError.message}
<span className="flex-1">{sendError.message}</span>
{sendError.action ? (
<button
type="button"
data-testid="composer-send-error-action"
onClick={sendError.action.onClick}
disabled={sendError.action.pending}
className="shrink-0 rounded-md border border-red-300 px-2 py-1 text-xs font-medium text-red-700 hover:bg-red-50 disabled:cursor-not-allowed disabled:opacity-60"
>
{sendError.action.label}
</button>
) : null}
</div>
) : null}

Expand Down
110 changes: 109 additions & 1 deletion web/src/hooks/mutations/useSendMessage.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { renderHook, act, waitFor } from '@testing-library/react'
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
import type { ReactNode } from 'react'
import { useSendMessage } from './useSendMessage'
import type { ApiClient } from '@/api/client'
import { ApiError, type ApiClient } from '@/api/client'

vi.mock('@/lib/message-window-store', () => ({
appendOptimisticMessage: vi.fn(),
Expand Down Expand Up @@ -523,6 +523,114 @@ describe('useSendMessage', () => {
await expect(acceptedPromise!).resolves.toBe(true)
})

// #918: the inactive-session 409 path
describe('inactive-session 409 (issue #918)', () => {
it('fires onError with the ApiError so the consumer can render a session_inactive affordance', async () => {
// Hub returns 409 with code: 'session_inactive' (guards.ts).
// The api client throws ApiError(status=409, code='session_inactive').
const onError = vi.fn()
const api = createMockApi(async () => {
throw new ApiError(
'HTTP 409 Conflict: {"error":"Session is inactive","code":"session_inactive"}',
409,
'session_inactive',
'{"error":"Session is inactive","code":"session_inactive"}'
)
})

const { result } = renderHook(
() => useSendMessage(api, 'session-A', { onError }),
{ wrapper: createWrapper() },
)

act(() => {
result.current.sendMessage('hello inactive')
})

await waitFor(() => {
expect(onError).toHaveBeenCalledTimes(1)
})
const info = onError.mock.calls[0][0] as { text: string; error: unknown; sessionId: string }
expect(info.text).toBe('hello inactive')
expect(info.sessionId).toBe('session-A')
expect(info.error).toBeInstanceOf(ApiError)
const apiErr = info.error as ApiError
expect(apiErr.status).toBe(409)
expect(apiErr.code).toBe('session_inactive')
})

it('fires onError when resolveSessionId rejects (pre-mutation inactive-session failure)', async () => {
// Pre-mutation: the route's resolveSessionId throws when
// inactiveSessionCanResume returns false OR api.resumeSession
// fails. Prior to #918 this dropped the typed text into the
// void with only a console.error; the operator saw nothing.
// The hook must surface this through onError too.
const onError = vi.fn()
const api = createMockApi()
const resumeError = new ApiError('Session is inactive', 409, 'session_inactive')

const { result } = renderHook(
() => useSendMessage(api, 'session-A', {
onError,
resolveSessionId: async () => { throw resumeError },
onSessionResolved: vi.fn(),
}),
{ wrapper: createWrapper() },
)

act(() => {
result.current.sendMessage('hello pre-mutation')
})

await waitFor(() => {
expect(onError).toHaveBeenCalledTimes(1)
})
const info = onError.mock.calls[0][0] as { text: string; error: unknown; sessionId: string }
expect(info.text).toBe('hello pre-mutation')
// Keyed by the ORIGINAL sessionId: pre-mutation never navigated.
expect(info.sessionId).toBe('session-A')
expect(info.error).toBe(resumeError)
})

it('5xx still uses the legacy text-restore path (#918 must not regress transient-failure UX)', async () => {
// Acceptance criterion: a real transient 500/network failure
// must keep the original behavior (remove optimistic row,
// onError fires with the plain message), not adopt the
// session_inactive affordance.
const onError = vi.fn()
const api = createMockApi(async () => {
throw new ApiError(
'HTTP 500 Internal Server Error',
500,
undefined,
undefined
)
})

const { removeOptimisticMessage } = await import('@/lib/message-window-store')
const removeMock = vi.mocked(removeOptimisticMessage)

const { result } = renderHook(
() => useSendMessage(api, 'session-A', { onError }),
{ wrapper: createWrapper() },
)

act(() => {
result.current.sendMessage('hello transient')
})

await waitFor(() => {
expect(onError).toHaveBeenCalledTimes(1)
})
expect(removeMock).toHaveBeenCalledWith('session-A', 'local-id-1')
const info = onError.mock.calls[0][0] as { error: unknown }
// No session_inactive code -> consumer renders fallback
// message, no Reopen action attached.
expect((info.error as ApiError).code).toBeUndefined()
expect((info.error as ApiError).status).toBe(500)
})
})

it('preserves scheduledAt when retrying a failed scheduled message', async () => {
const sendMock = vi.fn(async () => {})
const api = createMockApi(sendMock)
Expand Down
16 changes: 16 additions & 0 deletions web/src/hooks/mutations/useSendMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,22 @@ export function useSendMessage(
} catch (error) {
haptic.notification('error')
console.error('Failed to resolve session before send:', error)
// #918: surface the failure via onError so the route can render
// an inline affordance instead of silently swallowing the
// typed text. This covers the "no resume target" branch
// (inactiveSessionCanResume === false) and also any failure
// from api.resumeSession itself. The mutation never started
// (no optimistic row to clean up); onError is the only
// visibility hook the consumer has for this pre-mutation
// path. Key by the ORIGINAL sessionId because navigation
// hasn't happened yet -- the operator is still on the
// archived session's route.
options?.onError?.({
sessionId,
text,
error,
scheduledAt: scheduledAt ?? null
})
return false
} finally {
resolveGuardRef.current = false
Expand Down
2 changes: 2 additions & 0 deletions web/src/lib/locales/en.ts
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,8 @@ export default {
'chat.terminal': 'Terminal',
'chat.switchRemote': 'Switch to remote mode',
'chat.sendError.fallback': "Couldn't send your message. Edit and try again.",
'chat.sendError.sessionInactive': 'This session is archived. Reopen it to send your message.',
'chat.sendError.sessionInactive.action': 'Reopen',

// Codex review
'codexReview.title': 'Codex review',
Expand Down
2 changes: 2 additions & 0 deletions web/src/lib/locales/zh-CN.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,8 @@ export default {
'chat.terminal': '终端',
'chat.switchRemote': '切换到远程模式',
'chat.sendError.fallback': '消息未能发送。请修改后重试。',
'chat.sendError.sessionInactive': '此会话已归档。请先重新打开再发送消息。',
'chat.sendError.sessionInactive.action': '重新打开',

// Codex review
'codexReview.title': 'Codex review',
Expand Down
Loading
Loading