diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index 81b5beff..fdaf336c 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -1355,9 +1355,54 @@ Object.assign(CodemanApp.prototype, { this.activeFocusTrap.activate(); }, + /** + * Write a name the server has just confirmed into the local session map. + * + * Both rename surfaces re-render the tab strip from `this.sessions` right + * after their PUT, so without this they depended on the `session:updated` SSE + * frame to carry their own write back. On a page whose SSE stream has gone + * quiet without erroring (a proxy that idle-closed it, a laptop resumed from + * sleep) that frame never lands: the PUT stores the new name, the re-render + * repaints the stale one, and the rename looks like it did nothing until a + * full page reload. The response body is authoritative, so apply it directly. + * The SSE frame, when it does arrive, replaces the object with the same name. + */ + _applyLocalSessionName(sessionId, name) { + if (typeof name !== 'string') return; + const session = this.sessions.get(sessionId); + if (!session) return; + session.name = name; + this.sessions.set(sessionId, session); + // Mirrors _onSessionUpdated: subagent windows cache their parent's name. + this.updateSubagentParentNames?.(sessionId); + }, + + /** + * PUT a session name and return the name the server stored, or null if the + * request failed. `_apiPut` swallows network errors into a null Response and + * an API-level failure arrives as a non-ok status or `{success:false}`, so a + * rename that silently did nothing has to be detected here, not thrown. + */ + async _putSessionName(sessionId, name) { + const res = await this._apiPut(`/api/sessions/${sessionId}/name`, { name }); + if (!res || !res.ok) return null; + let payload = null; + try { + payload = await res.json(); + } catch { + return null; + } + if (payload && payload.success === false) return null; + const confirmed = payload?.data?.name; + return typeof confirmed === 'string' ? confirmed : name; + }, + async saveSessionName() { if (!this.editingSessionId) return; - const session = this.sessions.get(this.editingSessionId); + // Captured: the modal can be closed (or switched to another session) while + // the PUT is in flight, and the name belongs to the session that was open. + const sessionId = this.editingSessionId; + const session = this.sessions.get(sessionId); const parsed = session ? parseSessionPrefix(session.name) : null; const inputVal = document.getElementById('modalSessionName').value.trim(); let name; @@ -1366,11 +1411,13 @@ Object.assign(CodemanApp.prototype, { } else { name = inputVal; } - try { - await this._apiPut(`/api/sessions/${this.editingSessionId}/name`, { name }); - } catch (err) { - this.showToast('Failed to save session name: ' + err.message, 'error'); + const confirmed = await this._putSessionName(sessionId, name); + if (confirmed === null) { + this.showToast('Failed to save session name', 'error'); + return; } + this._applyLocalSessionName(sessionId, confirmed); + this.renderSessionTabs(); }, async autoSaveAutoCompact() { @@ -1680,15 +1727,14 @@ Object.assign(CodemanApp.prototype, { // Skip the API call if the session vanished between focus and blur. const stillExists = this.sessions.has(sessionId); if (stillExists && fullName !== session.name) { - try { - await fetch(`/api/sessions/${sessionId}/name`, { - method: 'PUT', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ name: fullName }) - }); - } catch (err) { + const confirmed = await this._putSessionName(sessionId, fullName); + if (confirmed === null) { tabName.textContent = originalContent; this.showToast('Failed to rename', 'error'); + } else { + // The re-render below repaints from this.sessions, so the new name has + // to be in the map before it runs (see _applyLocalSessionName()). + this._applyLocalSessionName(sessionId, confirmed); } } // Re-render tabs to restore full tab structure diff --git a/test/inline-rename.test.ts b/test/inline-rename.test.ts index 62155575..76eb6d97 100644 --- a/test/inline-rename.test.ts +++ b/test/inline-rename.test.ts @@ -358,6 +358,83 @@ describe('Inline rename input', () => { expect(result.editingAfter).toBe(null); }); + it('Commit writes the confirmed name into app.sessions WITHOUT any session:updated frame', async () => { + await resetState(); + expect(await startRename('no-sse', 'w9-case')).toBe(true); + + // finishRename() re-renders the tab strip from app.sessions, so the rename + // used to depend on the session:updated SSE frame to carry its own write + // back. On a page whose stream has gone quiet without erroring, the PUT + // stored the new name, the re-render repainted the stale one, and the tab + // only showed it after a full reload. No SSE is dispatched here at all. + const result = await page.evaluate(async () => { + const app = ( + window as unknown as { + app: { sessions: Map }; + } + ).app; + const origFetch = window.fetch; + window.fetch = (async () => + new Response('{"success":true,"data":{"name":"w9-case: fresh"}}', { + status: 200, + headers: { 'Content-Type': 'application/json' }, + })) as typeof window.fetch; + + const inputEl = document.querySelector('input.tab-rename-input') as HTMLInputElement; + inputEl.value = 'fresh'; + inputEl.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); + await new Promise((r) => setTimeout(r, 60)); + + window.fetch = origFetch; + return { mapName: app.sessions.get('no-sse')?.name ?? null }; + }); + + expect(result.mapName).toBe('w9-case: fresh'); + }); + + it('A rejected rename restores the old label and leaves app.sessions untouched', async () => { + await resetState(); + expect(await startRename('rename-500', 'w9-case')).toBe(true); + + // _apiPut turns a network error into a null Response and an API-level + // failure arrives as a non-ok status, neither of which throws, so a + // rejected rename has to be detected from the response, or it reports + // success and silently discards the user's edit. + const result = await page.evaluate(async () => { + const app = ( + window as unknown as { + app: { sessions: Map; showToast: (m: string, k: string) => void }; + } + ).app; + const toasts: string[] = []; + const origToast = app.showToast; + app.showToast = (msg: string) => void toasts.push(msg); + const origFetch = window.fetch; + window.fetch = (async () => + new Response('{"success":false,"error":"boom","errorCode":"INTERNAL"}', { + status: 500, + headers: { 'Content-Type': 'application/json' }, + })) as typeof window.fetch; + + const inputEl = document.querySelector('input.tab-rename-input') as HTMLInputElement; + inputEl.value = 'never-stored'; + inputEl.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); + await new Promise((r) => setTimeout(r, 60)); + + window.fetch = origFetch; + app.showToast = origToast; + return { + mapName: app.sessions.get('rename-500')?.name ?? null, + label: document.querySelector('.tab-name[data-session-id="rename-500"]')?.textContent ?? null, + toasts, + }; + }); + + expect(result.mapName).toBe('w9-case'); + expect(result.label).toBe('w9-case'); + expect(result.toasts).toContain('Failed to rename'); + }); + it('Re-entry: starting rename while one is active aborts the previous one', async () => { await resetState(); expect(await startRename('first-id', 'First')).toBe(true);