diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index 4bce2273..a5036056 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -2596,12 +2596,22 @@ Object.assign(CodemanApp.prototype, { * in the editor: a confirmed name is applied locally even after its editor is * gone, and the "already that name" check runs only once the earlier writes * have landed, so confirming the name still on screen is a real write. - * Resolves { status: 'confirmed' | 'failed' | 'deleted' }; never rejects. + * Resolves { status: 'confirmed' | 'failed' | 'deleted' }; never rejects, + * and reports a failed write itself, since its editor may be gone by then. + * `_inlineRenamePending` holds the newest queued name per session, so an + * editor reopened over a write in flight starts from that name rather than + * the one the server has not replaced yet. */ _queueInlineSessionName(sessionId, desiredName) { this._inlineRenameWrites ??= new Map(); + this._inlineRenamePending ??= new Map(); const writes = this._inlineRenameWrites; - const task = (writes.get(sessionId) || Promise.resolve()).then(async () => { + const pending = this._inlineRenamePending; + pending.set(sessionId, desiredName); + // Chained from a settled promise, so one rejected write cannot stop the + // writes queued behind it. + const prev = (writes.get(sessionId) || Promise.resolve()).catch(() => {}); + const task = prev.then(async () => { const session = this.sessions.get(sessionId); if (!session) return { status: 'deleted' }; if (session.name === desiredName) return { status: 'confirmed' }; @@ -2612,15 +2622,26 @@ Object.assign(CodemanApp.prototype, { // A failure is a value, so a later write in the chain still runs. } if (!this.sessions.has(sessionId)) return { status: 'deleted' }; - if (confirmed === null) return { status: 'failed' }; - this._applyLocalSessionName(sessionId, confirmed); - this.renderSessionTabs(); + if (confirmed === null) { + this.showToast('Failed to rename', 'error'); + return { status: 'failed' }; + } + try { + this._applyLocalSessionName(sessionId, confirmed); + this.renderSessionTabs(); + } catch (err) { + // The server holds the name; a local repaint failing is not a failed write. + console.error('[rename] applying the confirmed name failed', err); + } return { status: 'confirmed' }; }); writes.set(sessionId, task); - task.then(() => { - if (writes.get(sessionId) === task) writes.delete(sessionId); - }); + const cleanup = () => { + if (writes.get(sessionId) !== task) return; + writes.delete(sessionId); + pending.delete(sessionId); + }; + task.then(cleanup, cleanup); return task; }, @@ -2912,7 +2933,10 @@ Object.assign(CodemanApp.prototype, { tabName.classList.add('tab-name-renaming'); const currentName = this.getSessionName(session); - const parsed = parseSessionPrefix(session.name); + // A rename still in flight is the user's last word, not the name the + // server has yet to replace: start from it, and compare against it below. + const shownName = this._inlineRenamePending?.get(sessionId) ?? session.name; + const parsed = parseSessionPrefix(shownName); const originalContent = tabName.textContent; const originalChildren = [...tabName.childNodes].map((node) => node.cloneNode(true)); const restoreOriginalChildren = () => { @@ -2933,13 +2957,17 @@ Object.assign(CodemanApp.prototype, { const input = document.createElement('input'); input.type = 'text'; - input.value = parsed ? parsed.suffix : (session.name || ''); + input.value = parsed ? parsed.suffix : (shownName || ''); input.placeholder = parsed ? 'Add description...' : currentName; input.className = 'tab-rename-input'; // 80px is tuned for the narrow header tab; a full-width sidebar row can and - // should give the whole line to the input. - const renameWidth = tabName.closest('.tab-rail') ? 'auto' : this.isSessionSidebarActive?.() ? '100%' : '80px'; - input.style.cssText = `width: ${renameWidth}; min-width: 0; font-size: 0.75rem; padding: 2px 4px; background: var(--bg-input); border: 1px solid var(--accent); border-radius: 3px; color: var(--text); outline: none;`; + // should give the whole line to the input. The header editor may shrink to + // nothing, while a rail or sidebar row always keeps room to type. + const inRail = !!tabName.closest('.tab-rail'); + const inSidebar = !inRail && !!this.isSessionSidebarActive?.(); + const renameWidth = inRail ? 'auto' : inSidebar ? '100%' : '80px'; + const renameMinWidth = inRail || inSidebar ? '4rem' : '0'; + input.style.cssText = `width: ${renameWidth}; min-width: ${renameMinWidth}; font-size: 0.75rem; padding: 2px 4px; background: var(--bg-input); border: 1px solid var(--accent); border-radius: 3px; color: var(--text); outline: none;`; tabName.appendChild(input); input.focus(); @@ -2989,7 +3017,7 @@ Object.assign(CodemanApp.prototype, { const suffix = input.value.trim(); const fullName = parsed ? parsed.prefix + (suffix ? ': ' + suffix : '') : suffix; - if (fullName === session.name) restoreOriginalChildren(); + if (fullName === shownName) restoreOriginalChildren(); else tabName.textContent = fullName || originalContent; // Skip the API call if the session vanished between focus and blur. The @@ -2998,10 +3026,8 @@ Object.assign(CodemanApp.prototype, { if (this.sessions.has(sessionId)) { const result = await this._queueInlineSessionName(sessionId, fullName); if (invalidated || this._activeRename !== renameHandle || !this.sessions.has(sessionId)) return; - if (result.status === 'failed') { - restoreOriginalChildren(); - this.showToast('Failed to rename', 'error'); - } + // The queue reports a failure itself; the editor only puts its label back. + if (result.status === 'failed') restoreOriginalChildren(); } // Re-render tabs to restore full tab structure completeCurrentRename(); diff --git a/src/web/public/styles.css b/src/web/public/styles.css index f3ef5797..8ba70b26 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -1867,15 +1867,14 @@ html[data-tab-orientation='vertical'] .tab-rail .session-tab .tab-name-prefix { text-overflow: ellipsis; } -/* The input always keeps room to type. `!important` beats the editor's inline - `min-width: 0`, which the header strip's fixed-width editor relies on. */ +/* The input takes the rest of the row. Its 4rem floor is the editor's own + inline min-width (startInlineRename picks it per layout). */ :is( html[data-tab-orientation='vertical'] .tab-rail, html[data-session-list='sidebar'] .session-sidebar ) .tab-name-renaming .tab-rename-input { flex: 1 1 0; width: auto; - min-width: 4rem !important; } /* A compact rail row has no room for both the editor and its adornments, so diff --git a/test/inline-rename.test.ts b/test/inline-rename.test.ts index 82e7db40..e851f310 100644 --- a/test/inline-rename.test.ts +++ b/test/inline-rename.test.ts @@ -802,16 +802,133 @@ describe('Inline rename write ordering', () => { expect(await state('reopen')).toEqual({ bodies: ['First'], mapName: 'First', renameActive: false }); }); - it('re-sends the shown name when it is confirmed unchanged over a rename still in flight', async () => { + it('reopens the editor on the name still in flight, so confirming it unchanged keeps the rename', async () => { await mount('stale', 'Old'); await commit('stale', 'First'); - // The reopened editor still shows "Old" (the PUT has not answered), and - // Enter confirms that: the user's last word is "Old", not "First". + // The PUT for "First" has not answered, so app.sessions still says "Old". + // The reopened editor must show "First", the user's last word, and an + // untouched confirm must not queue "Old" behind it. + const reopenedValue = await page.evaluate(() => { + (window as unknown as { app: { startInlineRename: (id: string) => void } }).app.startInlineRename('stale'); + return (document.querySelector('.tab-name[data-session-id="stale"] input.tab-rename-input') as HTMLInputElement) + .value; + }); + expect(reopenedValue).toBe('First'); await commit('stale', null); await answer(0, 'First'); - await answer(1, 'Old'); await restoreFetch(); - expect(await state('stale')).toEqual({ bodies: ['First', 'Old'], mapName: 'Old', renameActive: false }); + expect(await state('stale')).toEqual({ bodies: ['First'], mapName: 'First', renameActive: false }); + expect( + await page.evaluate( + () => + (window as unknown as { app: { _inlineRenamePending?: Map } }).app._inlineRenamePending?.has( + 'stale' + ) ?? false + ) + ).toBe(false); + }); + + it('reports a failed write even after its editor is gone', async () => { + await mount('fail-late', 'Old'); + await page.evaluate(() => { + const w = window as unknown as { + app: { showToast: (message: string, type?: string) => void }; + __toasts: string[]; + __origToast?: (message: string, type?: string) => void; + }; + w.__toasts = []; + w.__origToast = w.app.showToast; + w.app.showToast = (message: string) => { + w.__toasts.push(message); + }; + }); + await commit('fail-late', 'First'); + // Reopen and dismiss: the editor that made the write is gone. + await page.evaluate(() => { + const app = ( + window as unknown as { app: { startInlineRename: (id: string) => void; _activeRename: { cancel: () => void } } } + ).app; + app.startInlineRename('fail-late'); + app._activeRename.cancel(); + }); + await page.evaluate(async () => { + const w = window as unknown as { __pending: Pending[] }; + w.__pending[0]?.resolve( + new Response(JSON.stringify({ success: false, error: 'boom' }), { + status: 500, + headers: { 'Content-Type': 'application/json' }, + }) + ); + await new Promise((resolve) => setTimeout(resolve, 30)); + }); + await restoreFetch(); + const toasts = await page.evaluate(() => { + const w = window as unknown as { + app: { showToast: unknown }; + __toasts: string[]; + __origToast?: unknown; + }; + w.app.showToast = w.__origToast; + return w.__toasts; + }); + expect(toasts).toEqual(['Failed to rename']); + expect(await state('fail-late')).toEqual({ bodies: ['First'], mapName: 'Old', renameActive: false }); + }); + + it('keeps sending a session renames after the work following a PUT throws', async () => { + await mount('throws', 'Old'); + await page.evaluate(() => { + const w = window as unknown as { + app: { updateSubagentParentNames?: (id: string) => void }; + __origParentNames?: (id: string) => void; + __throwOnce: boolean; + }; + w.__origParentNames = w.app.updateSubagentParentNames; + w.__throwOnce = true; + w.app.updateSubagentParentNames = (id: string) => { + if (w.__throwOnce) { + w.__throwOnce = false; + throw new Error('forced'); + } + w.__origParentNames?.call(w.app, id); + }; + window.addEventListener('unhandledrejection', (event) => event.preventDefault(), { once: true }); + }); + await commit('throws', 'First'); + await answer(0, 'First'); + await commit('throws', 'Second'); + await answer(1, 'Second'); + await restoreFetch(); + const leftover = await page.evaluate(() => { + const w = window as unknown as { + app: { updateSubagentParentNames?: unknown; _inlineRenameWrites?: Map }; + __origParentNames?: unknown; + }; + w.app.updateSubagentParentNames = w.__origParentNames; + return w.app._inlineRenameWrites?.has('throws') ?? false; + }); + expect(await state('throws')).toEqual({ bodies: ['First', 'Second'], mapName: 'Second', renameActive: false }); + expect(leftover).toBe(false); + }); + + it('lets the header strip editor shrink (inline min-width 0)', async () => { + await mount('header-width', 'Old'); + const minWidth = await page.evaluate(() => { + const app = ( + window as unknown as { + app: { startInlineRename: (id: string) => void; _activeRename: { cancel: () => void } | null }; + } + ).app; + app.startInlineRename('header-width'); + const input = document.querySelector( + '.tab-name[data-session-id="header-width"] input.tab-rename-input' + ) as HTMLInputElement; + const value = input.style.minWidth; + app._activeRename?.cancel(); + return value; + }); + await restoreFetch(); + expect(minWidth).toBe('0px'); }); it('keeps a confirmed session rename when a group rename takes over the editor', async () => { @@ -901,6 +1018,8 @@ describe('Vertical rail rename editor with a long prefix', () => { { variant: 'simple rows', settings: { tabOrientation: 'vertical', tabRailDetail: 'simple' }, compact: false }, { variant: 'detailed rows', settings: { tabOrientation: 'vertical', tabRailDetail: 'rich' }, compact: false }, { variant: 'compact rail', settings: { tabOrientation: 'vertical', tabRailWidth: 208 }, compact: true }, + { variant: 'sidebar', settings: { sessionListLayout: 'sidebar' }, compact: false }, + { variant: 'detailed sidebar', settings: { sessionListLayout: 'sidebar-rich' }, compact: false }, ])('keeps the prefix and a usable input inside the row ($variant)', async ({ settings, compact }) => { const context = await browser.newContext({ viewport: { width: 1280, height: 720 }, deviceScaleFactor: 1 }); try { @@ -910,7 +1029,8 @@ describe('Vertical rail rename editor with a long prefix', () => { ); const page = await context.newPage(); await page.goto(`http://localhost:${port}`, { waitUntil: 'domcontentloaded' }); - const row = page.locator(`#tabRail .session-tab[data-id="${sessionId}"]`); + // One #sessionTabs list, moved into the rail or the sidebar by the layout. + const row = page.locator(`#sessionTabs .session-tab[data-id="${sessionId}"]`); await row.waitFor({ state: 'visible', timeout: 15000 }); expect(await page.evaluate(() => document.documentElement.classList.contains('tab-rail-compact'))).toBe(compact); @@ -937,6 +1057,7 @@ describe('Vertical rail rename editor with a long prefix', () => { inputInsideRow: within(box(input), box(info)), prefixInsideRow: within(box(prefix), box(info)), prefixEllipsis: getComputedStyle(prefix).textOverflow, + inlineMinWidth: input.style.minWidth, }; }); expect(geometry.focused).toBe(true); @@ -945,6 +1066,8 @@ describe('Vertical rail rename editor with a long prefix', () => { expect(geometry.inputInsideRow).toBe(true); expect(geometry.prefixInsideRow).toBe(true); expect(geometry.prefixEllipsis).toBe('ellipsis'); + // The floor is the editor's own inline style, not a stylesheet override. + expect(geometry.inlineMinWidth).toBe('4rem'); await input.press('Escape'); expect(await row.locator('input.tab-rename-input').count()).toBe(0);