From 7612a6790fb9e2caa631c4a004281f27becf7e20 Mon Sep 17 00:00:00 2001 From: Brian Genisio Date: Fri, 7 Aug 2026 16:53:08 -0400 Subject: [PATCH 1/2] fix(chat): allow new conversations while a stream is active New chat was waiting on Octavus session create, which could stall behind an in-flight trigger. Switch the UI optimistically and create sessions on a dedicated HTTP pool so parallel conversations stay usable. Co-authored-by: Cursor --- lib/octavus-create.js | 52 +++++++++++++ lib/sessions-file.js | 21 ++++++ package-lock.json | 4 +- package.json | 3 +- public/app.js | 140 +++++++++++++++++++++++++++++++---- server.js | 107 +++++++++++++++----------- tests/dom/harness.js | 28 ++++++- tests/dom/render.test.js | 25 +++++++ tests/octavus-create.test.js | 69 +++++++++++++++++ tests/server.test.js | 44 +++++++++++ tests/sessions-file.test.js | 36 +++++++++ 11 files changed, 464 insertions(+), 65 deletions(-) create mode 100644 lib/octavus-create.js create mode 100644 lib/sessions-file.js create mode 100644 tests/octavus-create.test.js create mode 100644 tests/sessions-file.test.js diff --git a/lib/octavus-create.js b/lib/octavus-create.js new file mode 100644 index 0000000..5e0cde5 --- /dev/null +++ b/lib/octavus-create.js @@ -0,0 +1,52 @@ +/** + * Create Octavus agent sessions on a dedicated HTTP connection pool. + * + * Long-lived /api/trigger streams use the process-wide fetch dispatcher. When + * that pool is saturated (or pipelined behind a streaming response), a normal + * `octavus.agentSessions.create()` can stall until the stream ends — which + * blocks "New chat" in the UI. Routing creates through their own Agent keeps + * session creation independent of in-flight triggers. + */ +import { Agent, fetch as undiciFetch } from 'undici'; + +const createAgent = new Agent({ + connections: 8, + pipelining: 0, +}); + +/** + * @param {object} opts + * @param {string} opts.baseUrl - Octavus API base URL. + * @param {string} [opts.apiKey] - Bearer token. + * @param {string} opts.agentId - Deployed agent id. + * @param {Record} [opts.input] - Session input interpolations. + * @returns {Promise} The new session id. + */ +export async function createAgentSession({ baseUrl, apiKey, agentId, input = {} }) { + if (!baseUrl) throw new Error('OCTAVUS_API_URL is not configured'); + if (!agentId) throw new Error('Octavus agent id is not configured'); + + const url = `${String(baseUrl).replace(/\/$/, '')}/api/agent-sessions`; + const headers = { 'Content-Type': 'application/json' }; + if (apiKey) headers.Authorization = `Bearer ${apiKey}`; + + const res = await undiciFetch(url, { + method: 'POST', + headers, + body: JSON.stringify({ agentId, input }), + dispatcher: createAgent, + }); + + if (!res.ok) { + const text = await res.text().catch(() => ''); + throw new Error( + `Failed to create agent session (${res.status})${text ? `: ${text}` : ''}`, + ); + } + + const data = await res.json(); + if (typeof data?.sessionId !== 'string' || !data.sessionId) { + throw new Error('Agent session create returned no sessionId'); + } + return data.sessionId; +} diff --git a/lib/sessions-file.js b/lib/sessions-file.js new file mode 100644 index 0000000..615cee6 --- /dev/null +++ b/lib/sessions-file.js @@ -0,0 +1,21 @@ +/** + * Serialize read-modify-write access to chat-sessions.json. + * + * Stream saves and session creates can overlap; without a queue, two handlers + * can read the same snapshot and the later write drops the earlier change. + */ + +/** + * @param {() => Promise} operation + * @param {{ chain?: Promise }} [state] - Mutable holder for the queue tip. + * @returns {Promise} + */ +export function enqueueSessionsWrite(operation, state = enqueueSessionsWrite) { + const prev = state.chain ?? Promise.resolve(); + const run = prev.then(operation, operation); + // Keep the chain alive after failures so later writes still run. + state.chain = run.catch(() => {}); + return run; +} + +enqueueSessionsWrite.chain = Promise.resolve(); diff --git a/package-lock.json b/package-lock.json index c22560a..7be5b44 100644 --- a/package-lock.json +++ b/package-lock.json @@ -14,7 +14,8 @@ "express": "^4.19.2", "highlight.js": "^11.11.1", "marked": "^18.0.3", - "marked-highlight": "^2.2.4" + "marked-highlight": "^2.2.4", + "undici": "^8.10.0" }, "devDependencies": { "@octavus/cli": "^3.2.0", @@ -3572,7 +3573,6 @@ "version": "8.10.0", "resolved": "https://registry.npmjs.org/undici/-/undici-8.10.0.tgz", "integrity": "sha512-HvltHd7avK13QIw/oLe4qoOLyoVSoafqJ2jYOrtMRBkbYT31eiBQ8O0ehRKZiEZCMEyLFQNIADpgCWC5fALvYQ==", - "dev": true, "license": "MIT", "engines": { "node": ">=22.19.0" diff --git a/package.json b/package.json index 4a0fdd8..5f771c8 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,8 @@ "express": "^4.19.2", "highlight.js": "^11.11.1", "marked": "^18.0.3", - "marked-highlight": "^2.2.4" + "marked-highlight": "^2.2.4", + "undici": "^8.10.0" }, "devDependencies": { "@octavus/cli": "^3.2.0", diff --git a/public/app.js b/public/app.js index 709334e..801b039 100644 --- a/public/app.js +++ b/public/app.js @@ -1521,7 +1521,9 @@ async function sendMessage() { updateSendBtn(); try { - await active.chat.send( + const rt = await ensureActiveReady(); + if (!rt?.chat) return; + await rt.chat.send( 'user-message', { USER_MESSAGE: text, @@ -1555,7 +1557,7 @@ uploadImageBtn.addEventListener('click', () => openFilePicker(ACCEPT_IMAGE_TYPES uploadFileBtn.addEventListener('click', () => openFilePicker(ACCEPT_FILE_TYPES)); async function handleFiles(files) { - if (!files.length || !active?.chat) return; + if (!files.length || (!active?.chat && !active?.pendingCreate)) return; const newItems = files.map((f) => ({ file: f, @@ -1569,7 +1571,9 @@ async function handleFiles(files) { updateSendBtn(); try { - const refs = await active.chat.uploadFiles(files); + const rt = await ensureActiveReady(); + if (!rt?.chat) throw new Error('Session not ready'); + const refs = await rt.chat.uploadFiles(files); refs.forEach((ref, i) => { newItems[i].ref = ref; newItems[i].status = 'ready'; @@ -1696,8 +1700,10 @@ function clearAttachment() { promptInput.addEventListener('input', updateSendBtn); function isComposerSendAllowed() { + // Pending new chats have no OctavusChat yet, but the user should still be + // able to queue a send — sendMessage() awaits create before calling send. return canSendMessage({ - hasActiveChat: Boolean(active?.chat), + hasActiveChat: Boolean(active?.chat) || Boolean(active?.pendingCreate), isUploading, activeStatus: active?.chat?.status, streamingCount: streamingCount(), @@ -1707,6 +1713,17 @@ function isComposerSendAllowed() { }); } +/** Resolve a pending optimistic session to a real Octavus-backed runtime. */ +async function ensureActiveReady() { + if (!active?.pendingCreate || !active.createPromise) return active; + try { + return await active.createPromise; + } catch (err) { + console.error('[ChatCPT] Pending session create failed:', err); + return null; + } +} + promptInput.addEventListener('keydown', (e) => { const isEnter = e.key === 'Enter' || e.key === 'NumpadEnter'; if (!isEnter || e.isComposing) return; @@ -1854,8 +1871,14 @@ function renderSidebar() { async function deleteSession(sid) { const wasActive = active?.sessionId === sid; + const pending = sessions.get(sid); + // Optimistic local ids never hit the server; mark aborted so a late create + // response deletes the remote session instead of promoting it. + if (pending?.pendingCreate) pending.createAborted = true; teardownRuntime(sid); - await fetch(`/api/sessions/${sid}`, { method: 'DELETE' }); + if (!String(sid).startsWith('pending-')) { + await fetch(`/api/sessions/${sid}`, { method: 'DELETE' }); + } allSessionsMeta = allSessionsMeta.filter((s) => s.session_id !== sid); if (wasActive) { @@ -1927,29 +1950,114 @@ async function replaceCurrentChat() { } async function startNewChat() { - const res = await fetch('/api/sessions', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ model: selectedModel, temperature: selectedTemperature, thinking: selectedThinking }), - }); - if (!res.ok) return; - const data = await res.json(); + // Switch the UI immediately. Octavus session create can stall while another + // conversation is streaming; waiting on it made New chat appear blocked. + const localId = `pending-${crypto.randomUUID()}`; + const pendingRt = { + sessionId: localId, + chat: null, + unsubscribe: null, + abortController: null, + restoredMessages: [], + lastStatus: null, + lastSaveTime: 0, + saveThrottleTimer: null, + streamingStartTime: null, + pendingCreate: true, + createAborted: false, + createPromise: null, + }; + sessions.set(localId, pendingRt); clearAttachment(); applyInitialPrompt(); + active = pendingRt; - active = getOrCreateRuntime(data.sessionId, []); - + const meta = { + session_id: localId, + title: t('New conversation'), + updated_at: new Date().toISOString(), + }; if (chatConfig.hideHistory) { - allSessionsMeta = [{ session_id: active.sessionId, title: t('New conversation'), updated_at: new Date().toISOString() }]; + allSessionsMeta = [meta]; } else { - allSessionsMeta.unshift({ session_id: active.sessionId, title: t('New conversation'), updated_at: new Date().toISOString() }); + allSessionsMeta.unshift(meta); } renderActive(); renderSidebar(); updateSendBtn(); syncStreamingLoop(); + + pendingRt.createPromise = (async () => { + const res = await fetch('/api/sessions', { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ + model: selectedModel, + temperature: selectedTemperature, + thinking: selectedThinking, + }), + }); + if (!res.ok) throw new Error(`Failed to create session (HTTP ${res.status})`); + const data = await res.json(); + + // User deleted this optimistic thread while create was in flight. + if (pendingRt.createAborted || !sessions.has(localId)) { + sessions.delete(localId); + if (data.sessionId) { + fetch(`/api/sessions/${data.sessionId}`, { method: 'DELETE' }).catch(() => {}); + } + return null; + } + + const wasActive = active === pendingRt; + sessions.delete(localId); + const real = getOrCreateRuntime(data.sessionId, []); + + const idx = allSessionsMeta.findIndex((s) => s.session_id === localId); + if (idx >= 0) { + allSessionsMeta[idx] = { + ...allSessionsMeta[idx], + session_id: data.sessionId, + }; + } else if (!allSessionsMeta.some((s) => s.session_id === data.sessionId)) { + allSessionsMeta.unshift({ + session_id: data.sessionId, + title: t('New conversation'), + updated_at: new Date().toISOString(), + }); + } + + if (wasActive) { + active = real; + renderActive(); + updateSendBtn(); + syncStreamingLoop(); + } + renderSidebar(); + return real; + })().catch((err) => { + console.error('[ChatCPT] New chat create failed:', err); + sessions.delete(localId); + allSessionsMeta = allSessionsMeta.filter((s) => s.session_id !== localId); + if (active === pendingRt) { + active = null; + if (allSessionsMeta.length > 0) { + // Best-effort fall back; ignore switch errors. + switchSession(allSessionsMeta[0].session_id); + } else { + renderActive(); + renderSidebar(); + updateSendBtn(); + } + } else { + renderSidebar(); + } + throw err; + }); + + return pendingRt.createPromise; } // ── Regenerate / Edit ───────────────────────────────────────── diff --git a/server.js b/server.js index 9ac56e7..ee5d93e 100644 --- a/server.js +++ b/server.js @@ -18,6 +18,8 @@ import { buildCapabilitiesPayload, loadCapabilities, } from './lib/model-capabilities.js'; +import { createAgentSession } from './lib/octavus-create.js'; +import { enqueueSessionsWrite } from './lib/sessions-file.js'; const __dirname = path.dirname(fileURLToPath(import.meta.url)); const SESSIONS_FILE = path.join(__dirname, 'chat-sessions.json'); @@ -134,16 +136,33 @@ app.get('/api/models', async (_req, res) => { const readSessionsFile = () => readJsonFile(SESSIONS_FILE, { sessions: [] }); const writeSessionsFile = (data) => writeJsonFile(SESSIONS_FILE, data); +/** Run a read-modify-write against chat-sessions.json without overlapping writers. */ +function updateSessionsFile(mutator) { + return enqueueSessionsWrite(async () => { + const data = await readSessionsFile(); + const next = await mutator(data); + if (next !== false) await writeSessionsFile(next ?? data); + return next ?? data; + }); +} + async function createNewSession(options = {}) { const config = await readConfig(); const input = buildSessionInput(options, config); console.log('[session] Creating with input:', JSON.stringify(input)); - const sessionId = await octavus.agentSessions.create(AGENT_ID, input); + // Dedicated HTTP pool — must not share the streaming trigger dispatcher. + const sessionId = await createAgentSession({ + baseUrl: process.env.OCTAVUS_API_URL, + apiKey: process.env.OCTAVUS_API_KEY, + agentId: AGENT_ID, + input, + }); const record = buildSessionRecord(sessionId); - const data = await readSessionsFile(); - // With history hidden there is only ever one conversation; drop the rest. - data.sessions = config.hideHistory ? [record] : [...data.sessions, record]; - await writeSessionsFile(data); + await updateSessionsFile((data) => { + // With history hidden there is only ever one conversation; drop the rest. + data.sessions = config.hideHistory ? [record] : [...data.sessions, record]; + return data; + }); return record; } @@ -186,8 +205,10 @@ app.get('/api/session', async (req, res) => { ); // With history hidden, keep only the resumed session; discard any others. if (config.hideHistory && data.sessions.length > 1) { - data.sessions = [latest]; - await writeSessionsFile(data); + await updateSessionsFile((current) => { + current.sessions = [latest]; + return current; + }); } return res.json({ sessionId: latest.session_id, messages: latest.messages }); } @@ -224,9 +245,10 @@ app.post('/api/sessions', async (req, res) => { // ── DELETE /api/sessions/:sessionId ────────────────────────── app.delete('/api/sessions/:sessionId', async (req, res) => { const { sessionId } = req.params; - const data = await readSessionsFile(); - data.sessions = data.sessions.filter((s) => s.session_id !== sessionId); - await writeSessionsFile(data); + await updateSessionsFile((data) => { + data.sessions = data.sessions.filter((s) => s.session_id !== sessionId); + return data; + }); res.json({ ok: true }); }); @@ -241,16 +263,15 @@ app.post('/api/session/fork', async (req, res) => { try { const record = await createNewSession({ model, temperature, thinking }); - const data = await readSessionsFile(); - - const idx = data.sessions.findIndex((s) => s.session_id === record.session_id); - if (idx >= 0) { - data.sessions[idx].messages = messages; - data.sessions[idx].updated_at = new Date().toISOString(); - } - - data.sessions = data.sessions.filter((s) => s.session_id !== oldSessionId); - await writeSessionsFile(data); + await updateSessionsFile((data) => { + const idx = data.sessions.findIndex((s) => s.session_id === record.session_id); + if (idx >= 0) { + data.sessions[idx].messages = messages; + data.sessions[idx].updated_at = new Date().toISOString(); + } + data.sessions = data.sessions.filter((s) => s.session_id !== oldSessionId); + return data; + }); res.json({ sessionId: record.session_id }); } catch (err) { console.error('[session/fork] Error:', err); @@ -267,30 +288,30 @@ app.post('/api/session/save', async (req, res) => { } try { - const data = await readSessionsFile(); const config = await readConfig(); - const idx = data.sessions.findIndex((s) => s.session_id === sessionId); - const now = new Date().toISOString(); - - if (idx >= 0) { - data.sessions[idx].messages = messages; - data.sessions[idx].updated_at = now; - } else { - data.sessions.push({ - session_id: sessionId, - created_at: now, - updated_at: now, - messages, - selected_submission: null, - }); - } - - // With history hidden, only the current conversation is ever persisted. - if (config.hideHistory) { - data.sessions = data.sessions.filter((s) => s.session_id === sessionId); - } - - await writeSessionsFile(data); + await updateSessionsFile((data) => { + const idx = data.sessions.findIndex((s) => s.session_id === sessionId); + const now = new Date().toISOString(); + + if (idx >= 0) { + data.sessions[idx].messages = messages; + data.sessions[idx].updated_at = now; + } else { + data.sessions.push({ + session_id: sessionId, + created_at: now, + updated_at: now, + messages, + selected_submission: null, + }); + } + + // With history hidden, only the current conversation is ever persisted. + if (config.hideHistory) { + data.sessions = data.sessions.filter((s) => s.session_id === sessionId); + } + return data; + }); res.json({ ok: true }); } catch (err) { console.error('[session/save] Error:', err); diff --git a/tests/dom/harness.js b/tests/dom/harness.js index 7fe5c40..ef51917 100644 --- a/tests/dom/harness.js +++ b/tests/dom/harness.js @@ -72,8 +72,16 @@ const DEFAULT_CONFIG = { * @param {object} [options.config] Overrides merged into the default chat config. * @param {string[]} [options.models] Model ids returned by /api/models. More than * one causes app.js to construct a real Dropdown. + * @param {boolean} [options.holdNewSession] When true, POST /api/sessions waits + * until releaseNewSession() is called — used to + * assert optimistic New chat UI. */ -export async function bootApp({ messages = [], config = {}, models = ['anthropic/claude-sonnet-4-6'] } = {}) { +export async function bootApp({ + messages = [], + config = {}, + models = ['anthropic/claude-sonnet-4-6'], + holdNewSession = false, +} = {}) { vi.resetModules(); fakeChats.length = 0; @@ -83,6 +91,9 @@ export async function bootApp({ messages = [], config = {}, models = ['anthropic document.body.innerHTML = body; const requests = []; + /** @type {null | (() => void)} */ + let releaseNewSession = null; + globalThis.fetch = vi.fn(async (url, init = {}) => { const u = String(url); requests.push({ url: u, method: init.method ?? 'GET' }); @@ -92,7 +103,14 @@ export async function bootApp({ messages = [], config = {}, models = ['anthropic if (u === '/api/session') return json({ sessionId: 'session-1', messages }); // startNewChat() POSTs to the same path the session list is read from, so // the method check has to come first. - if (u === '/api/sessions' && init.method === 'POST') return json({ sessionId: 'session-2' }); + if (u === '/api/sessions' && init.method === 'POST') { + if (holdNewSession) { + await new Promise((resolve) => { + releaseNewSession = resolve; + }); + } + return json({ sessionId: 'session-2' }); + } if (u === '/api/sessions') return json({ sessions: [{ session_id: 'session-1', title: 'Test conversation', updated_at: '2026-08-05T00:00:00Z' }] }); if (u === '/api/config') return json({ ...DEFAULT_CONFIG, ...config }); if (u === '/api/models') return json({ models, capabilities: {} }); @@ -110,7 +128,11 @@ export async function bootApp({ messages = [], config = {}, models = ['anthropic await import('../../public/app.js'); await settle(); - return { requests, chat: fakeChats[0] }; + return { + requests, + chat: fakeChats[0], + releaseNewSession: () => releaseNewSession?.(), + }; } /** Node 26 does not expose localStorage without a backing file; app.js needs one. */ diff --git a/tests/dom/render.test.js b/tests/dom/render.test.js index 8b17b95..3345734 100644 --- a/tests/dom/render.test.js +++ b/tests/dom/render.test.js @@ -442,3 +442,28 @@ describe('composer availability during streaming (A3)', () => { expect(document.activeElement).toBe(input); }); }); + +describe('new chat while create is slow', () => { + it('shows a new sidebar thread before POST /api/sessions resolves', async () => { + const { releaseNewSession } = await bootApp({ holdNewSession: true }); + const chatsBefore = fakeChats.length; + + const before = qa('.session-item').length; + q('#newChatBtn').click(); + await settle(); + + // Optimistic thread appears even though Octavus create is still held. + expect(qa('.session-item').length).toBe(before + 1); + expect(qa('.session-item')[0].classList.contains('session-item--active')).toBe(true); + expect(q('#emptyState')?.hidden).toBe(false); + // Real OctavusChat is not attached until create finishes. + expect(fakeChats.length).toBe(chatsBefore); + + releaseNewSession(); + await settle(8); + + // Create finished → runtime wired with a real chat instance. + expect(fakeChats.length).toBe(chatsBefore + 1); + expect(qa('.session-item').length).toBe(before + 1); + }); +}); diff --git a/tests/octavus-create.test.js b/tests/octavus-create.test.js new file mode 100644 index 0000000..476dff8 --- /dev/null +++ b/tests/octavus-create.test.js @@ -0,0 +1,69 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; +import { Agent } from 'undici'; + +const fetchMock = vi.fn(); + +vi.mock('undici', async () => { + const actual = await vi.importActual('undici'); + return { + ...actual, + fetch: (...args) => fetchMock(...args), + }; +}); + +const { createAgentSession } = await import('../lib/octavus-create.js'); + +describe('createAgentSession', () => { + beforeEach(() => { + fetchMock.mockReset(); + }); + + afterEach(() => { + fetchMock.mockReset(); + }); + + it('POSTs to /api/agent-sessions on a dedicated dispatcher', async () => { + fetchMock.mockResolvedValue({ + ok: true, + json: async () => ({ sessionId: 'sess-123' }), + }); + + const id = await createAgentSession({ + baseUrl: 'https://octavus.example/', + apiKey: 'secret', + agentId: 'agent-1', + input: { MODEL: 'x' }, + }); + + expect(id).toBe('sess-123'); + expect(fetchMock).toHaveBeenCalledOnce(); + const [url, init] = fetchMock.mock.calls[0]; + expect(url).toBe('https://octavus.example/api/agent-sessions'); + expect(init.method).toBe('POST'); + expect(init.headers.Authorization).toBe('Bearer secret'); + expect(JSON.parse(init.body)).toEqual({ agentId: 'agent-1', input: { MODEL: 'x' } }); + expect(init.dispatcher).toBeInstanceOf(Agent); + }); + + it('throws when the API returns a non-OK status', async () => { + fetchMock.mockResolvedValue({ + ok: false, + status: 503, + text: async () => 'busy', + }); + + await expect( + createAgentSession({ + baseUrl: 'https://octavus.example', + apiKey: 'secret', + agentId: 'agent-1', + }), + ).rejects.toThrow(/503/); + }); + + it('throws when baseUrl is missing', async () => { + await expect( + createAgentSession({ agentId: 'agent-1' }), + ).rejects.toThrow(/OCTAVUS_API_URL/); + }); +}); diff --git a/tests/server.test.js b/tests/server.test.js index 551dfab..a0c967a 100644 --- a/tests/server.test.js +++ b/tests/server.test.js @@ -24,9 +24,14 @@ vi.mock('@octavus/server-sdk', () => { }; }); +vi.mock('../lib/octavus-create.js', () => ({ + createAgentSession: vi.fn().mockResolvedValue('new-session-id'), +})); + vi.mock('dotenv/config', () => ({})); const fs = (await import('fs/promises')).default; +const { createAgentSession } = await import('../lib/octavus-create.js'); // Set env vars before importing server process.env.NODE_ENV = 'test'; @@ -36,6 +41,15 @@ process.env.OCTAVUS_AGENT_ID = 'test-agent-id'; const { app } = await import('../server.js'); +function mockSessionsFile(data = { sessions: [] }) { + fs.readFile.mockImplementation(async (path) => { + if (String(path).includes('chat-sessions')) return JSON.stringify(data); + if (String(path).includes('chat-config')) return '{}'; + throw new Error('ENOENT'); + }); + fs.writeFile.mockResolvedValue(undefined); +} + // ── GET /api/config ─────────────────────────────────────────── describe('GET /api/config', () => { @@ -209,6 +223,36 @@ describe('GET /api/session', () => { }); }); +// ── POST /api/sessions ──────────────────────────────────────── + +describe('POST /api/sessions', () => { + beforeEach(() => { + vi.resetAllMocks(); + createAgentSession.mockResolvedValue('new-session-id'); + }); + + it('creates via the dedicated Octavus create helper and persists the record', async () => { + mockSessionsFile({ sessions: [] }); + + const res = await request(app) + .post('/api/sessions') + .send({ model: 'anthropic/claude-sonnet-4-6', temperature: 0.7, thinking: 'off' }); + + expect(res.status).toBe(200); + expect(res.body.sessionId).toBe('new-session-id'); + expect(createAgentSession).toHaveBeenCalledOnce(); + expect(createAgentSession.mock.calls[0][0]).toMatchObject({ + baseUrl: 'https://test.api', + apiKey: 'test-key', + agentId: 'test-agent-id', + }); + + const written = JSON.parse(fs.writeFile.mock.calls[0][1]); + expect(written.sessions).toHaveLength(1); + expect(written.sessions[0].session_id).toBe('new-session-id'); + }); +}); + // ── DELETE /api/sessions/:sessionId ─────────────────────────── describe('DELETE /api/sessions/:sessionId', () => { diff --git a/tests/sessions-file.test.js b/tests/sessions-file.test.js new file mode 100644 index 0000000..29a755a --- /dev/null +++ b/tests/sessions-file.test.js @@ -0,0 +1,36 @@ +import { describe, it, expect } from 'vitest'; +import { enqueueSessionsWrite } from '../lib/sessions-file.js'; + +describe('enqueueSessionsWrite', () => { + it('runs operations in order even when started concurrently', async () => { + const state = { chain: Promise.resolve() }; + const order = []; + + const slow = enqueueSessionsWrite(async () => { + await new Promise((r) => setTimeout(r, 20)); + order.push('a'); + return 'a'; + }, state); + + const fast = enqueueSessionsWrite(async () => { + order.push('b'); + return 'b'; + }, state); + + await expect(Promise.all([slow, fast])).resolves.toEqual(['a', 'b']); + expect(order).toEqual(['a', 'b']); + }); + + it('continues the queue after a rejected operation', async () => { + const state = { chain: Promise.resolve() }; + + const failed = enqueueSessionsWrite(async () => { + throw new Error('boom'); + }, state); + + const next = enqueueSessionsWrite(async () => 'ok', state); + + await expect(failed).rejects.toThrow('boom'); + await expect(next).resolves.toBe('ok'); + }); +}); From 53e96dda1e7d4c25b02911e13dc94d13fddfbf62 Mon Sep 17 00:00:00 2001 From: Brian Genisio Date: Fri, 7 Aug 2026 17:04:12 -0400 Subject: [PATCH 2/2] fix(chat): harden optimistic new-chat failure paths Restore composer content when session create fails before send, settle createPromise to null instead of rejecting, and cover the review nits with timeouts plus create-failure tests. Co-authored-by: Cursor --- lib/octavus-create.js | 2 ++ public/app.js | 22 ++++++++++++++++++---- server.js | 8 +++++++- tests/dom/harness.js | 10 ++++++++++ tests/dom/render.test.js | 21 ++++++++++++++++++++- tests/octavus-create.test.js | 15 +++++++++++++++ tests/server.test.js | 10 ++++++++++ 7 files changed, 82 insertions(+), 6 deletions(-) diff --git a/lib/octavus-create.js b/lib/octavus-create.js index 5e0cde5..09268fc 100644 --- a/lib/octavus-create.js +++ b/lib/octavus-create.js @@ -12,6 +12,8 @@ import { Agent, fetch as undiciFetch } from 'undici'; const createAgent = new Agent({ connections: 8, pipelining: 0, + headersTimeout: 15_000, + bodyTimeout: 15_000, }); /** diff --git a/public/app.js b/public/app.js index 801b039..c1b8f69 100644 --- a/public/app.js +++ b/public/app.js @@ -1512,7 +1512,8 @@ async function sendMessage() { if (!isComposerSendAllowed()) return; const text = promptInput.value.trim(); - const readyRefs = fileItems.filter((i) => i.status === 'ready').map((i) => i.ref); + const savedItems = fileItems.slice(); + const readyRefs = savedItems.filter((i) => i.status === 'ready').map((i) => i.ref); if (!text && readyRefs.length === 0) return; promptInput.value = ''; @@ -1520,9 +1521,19 @@ async function sendMessage() { clearAttachment(); updateSendBtn(); + const restoreComposer = () => { + promptInput.value = text; + fileItems = savedItems; + renderAttachmentPreview(); + updateSendBtn(); + }; + try { const rt = await ensureActiveReady(); - if (!rt?.chat) return; + if (!rt?.chat) { + restoreComposer(); + return; + } await rt.chat.send( 'user-message', { @@ -1828,6 +1839,7 @@ function renderSidebar() { + (isStreaming ? ' session-item--streaming' : ''); item.setAttribute('role', 'button'); item.setAttribute('tabindex', '0'); + item.dataset.sessionId = s.session_id; item.setAttribute('aria-label', isStreaming ? t('{title} (responding)', { title: s.title }) : s.title); const title = document.createElement('span'); @@ -2045,7 +2057,9 @@ async function startNewChat() { active = null; if (allSessionsMeta.length > 0) { // Best-effort fall back; ignore switch errors. - switchSession(allSessionsMeta[0].session_id); + switchSession(allSessionsMeta[0].session_id).catch((switchErr) => { + console.error('[ChatCPT] Fallback switch after failed create:', switchErr); + }); } else { renderActive(); renderSidebar(); @@ -2054,7 +2068,7 @@ async function startNewChat() { } else { renderSidebar(); } - throw err; + return null; }); return pendingRt.createPromise; diff --git a/server.js b/server.js index ee5d93e..1a9e206 100644 --- a/server.js +++ b/server.js @@ -136,7 +136,13 @@ app.get('/api/models', async (_req, res) => { const readSessionsFile = () => readJsonFile(SESSIONS_FILE, { sessions: [] }); const writeSessionsFile = (data) => writeJsonFile(SESSIONS_FILE, data); -/** Run a read-modify-write against chat-sessions.json without overlapping writers. */ +/** + * Run a read-modify-write against chat-sessions.json without overlapping writers. + * The mutator receives the current data object. Return contract: + * • a new object → write that object + * • undefined → write the (possibly mutated) existing data + * • false → skip persistence + */ function updateSessionsFile(mutator) { return enqueueSessionsWrite(async () => { const data = await readSessionsFile(); diff --git a/tests/dom/harness.js b/tests/dom/harness.js index ef51917..a17e1a9 100644 --- a/tests/dom/harness.js +++ b/tests/dom/harness.js @@ -75,12 +75,14 @@ const DEFAULT_CONFIG = { * @param {boolean} [options.holdNewSession] When true, POST /api/sessions waits * until releaseNewSession() is called — used to * assert optimistic New chat UI. + * @param {boolean} [options.failNewSession] When true, POST /api/sessions returns 500. */ export async function bootApp({ messages = [], config = {}, models = ['anthropic/claude-sonnet-4-6'], holdNewSession = false, + failNewSession = false, } = {}) { vi.resetModules(); fakeChats.length = 0; @@ -109,6 +111,14 @@ export async function bootApp({ releaseNewSession = resolve; }); } + if (failNewSession) { + return { + ok: false, + status: 500, + json: async () => ({ error: 'Failed to create session' }), + text: async () => 'Failed to create session', + }; + } return json({ sessionId: 'session-2' }); } if (u === '/api/sessions') return json({ sessions: [{ session_id: 'session-1', title: 'Test conversation', updated_at: '2026-08-05T00:00:00Z' }] }); diff --git a/tests/dom/render.test.js b/tests/dom/render.test.js index 3345734..d4e0891 100644 --- a/tests/dom/render.test.js +++ b/tests/dom/render.test.js @@ -445,7 +445,7 @@ describe('composer availability during streaming (A3)', () => { describe('new chat while create is slow', () => { it('shows a new sidebar thread before POST /api/sessions resolves', async () => { - const { releaseNewSession } = await bootApp({ holdNewSession: true }); + const { requests, releaseNewSession } = await bootApp({ holdNewSession: true }); const chatsBefore = fakeChats.length; const before = qa('.session-item').length; @@ -458,6 +458,7 @@ describe('new chat while create is slow', () => { expect(q('#emptyState')?.hidden).toBe(false); // Real OctavusChat is not attached until create finishes. expect(fakeChats.length).toBe(chatsBefore); + expect(qa('.session-item')[0].dataset.sessionId).toMatch(/^pending-/); releaseNewSession(); await settle(8); @@ -465,5 +466,23 @@ describe('new chat while create is slow', () => { // Create finished → runtime wired with a real chat instance. expect(fakeChats.length).toBe(chatsBefore + 1); expect(qa('.session-item').length).toBe(before + 1); + + const createdId = 'session-2'; + expect(requests.some((r) => r.url === '/api/sessions' && r.method === 'POST')).toBe(true); + const sidebarIds = qa('.session-item').map((el) => el.dataset.sessionId); + expect(sidebarIds).toContain(createdId); + expect(sidebarIds.some((id) => id?.startsWith('pending-'))).toBe(false); + }); + + it('rolls back pending session metadata when POST /api/sessions fails', async () => { + await bootApp({ failNewSession: true }); + const before = qa('.session-item').map((el) => el.dataset.sessionId); + + q('#newChatBtn').click(); + await settle(8); + + const after = qa('.session-item').map((el) => el.dataset.sessionId); + expect(after.some((id) => id?.startsWith('pending-'))).toBe(false); + expect(after).toEqual(before); }); }); diff --git a/tests/octavus-create.test.js b/tests/octavus-create.test.js index 476dff8..e103f10 100644 --- a/tests/octavus-create.test.js +++ b/tests/octavus-create.test.js @@ -66,4 +66,19 @@ describe('createAgentSession', () => { createAgentSession({ agentId: 'agent-1' }), ).rejects.toThrow(/OCTAVUS_API_URL/); }); + + it('throws when a 200 response has no string sessionId', async () => { + fetchMock.mockResolvedValue({ + ok: true, + json: async () => ({ sessionId: null }), + }); + + await expect( + createAgentSession({ + baseUrl: 'https://octavus.example', + apiKey: 'secret', + agentId: 'agent-1', + }), + ).rejects.toThrow(/no sessionId/); + }); }); diff --git a/tests/server.test.js b/tests/server.test.js index a0c967a..e3596e2 100644 --- a/tests/server.test.js +++ b/tests/server.test.js @@ -251,6 +251,16 @@ describe('POST /api/sessions', () => { expect(written.sessions).toHaveLength(1); expect(written.sessions[0].session_id).toBe('new-session-id'); }); + + it('returns an error and does not persist when createAgentSession rejects', async () => { + mockSessionsFile({ sessions: [] }); + createAgentSession.mockRejectedValueOnce(new Error('octavus down')); + + const res = await request(app).post('/api/sessions').send({}); + + expect(res.status).toBeGreaterThanOrEqual(400); + expect(fs.writeFile).not.toHaveBeenCalled(); + }); }); // ── DELETE /api/sessions/:sessionId ───────────────────────────