From 06aba94ef199bf05ccfa26e98bb9a7eebf6584e3 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Sat, 3 Oct 2026 13:18:22 -0400 Subject: [PATCH 1/3] fix(tabs): grouped rail interaction fixes for inline rename Two problems with renaming a tab in the vertical rail, both easier to hit now that the grouped rail has its own inline editor beside the session one. Writes. A committed rename PUT its name and only applied the answer if the same editor was still open when it came back. Reopening the editor before the PUT answered (F2 or right-click again, or starting a group rename, which cancels the session editor) threw the confirmed name away, so the tab kept showing the old name until an SSE frame happened to repaint it. Two quick renames also raced as two concurrent PUTs. Inline renames now go through a per-session queue: one PUT at a time in the order they were made, the confirmed name applied to app.sessions whatever happened to the editor, and the "already that name" check made when the write runs rather than when Enter is pressed, so confirming the name still on screen over a write in flight is a real write. Layout. The editor (a flex row) could not shrink below the input's intrinsic width, so a long w- prefix pushed the label past its row: the prefix slid out of view in the detailed rows and the input was clipped mid-word in the compact rail. The label now has min-width 0, the prefix gives way first (down to 2rem, with an ellipsis), the input keeps 4rem, and in the compact rail the row's adornments step aside while the name is edited. The detailed rows' three-line clamp also outranked the shared unclamp rule, which is what the existing "unclamped editor" browser test caught; it is restated there. Header strip, sidebar and flat-rail markup are unchanged. Tests (test/inline-rename.test.ts, browser suite): the unclamp check runs for simple and detailed rows; a write-ordering describe covers ordering, a reopened editor cancelled over a confirmed write, a re-sent unchanged name and a group rename taking over; a long-prefix describe drives real rows from a live session in simple, detailed and compact rails. --- src/web/public/session-ui.js | 50 ++++- src/web/public/styles.css | 41 +++- test/inline-rename.test.ts | 401 +++++++++++++++++++++++++++++++---- 3 files changed, 441 insertions(+), 51 deletions(-) diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index 27a74ec8..4bce2273 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -2589,6 +2589,41 @@ Object.assign(CodemanApp.prototype, { return typeof confirmed === 'string' ? confirmed : name; }, + /** + * Write an inline rename, one PUT per session at a time, in the order the + * user made them. The editor can be reopened (or cancelled, or replaced by a + * group rename) while a PUT is in flight, so the write lives here rather than + * 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. + */ + _queueInlineSessionName(sessionId, desiredName) { + this._inlineRenameWrites ??= new Map(); + const writes = this._inlineRenameWrites; + const task = (writes.get(sessionId) || Promise.resolve()).then(async () => { + const session = this.sessions.get(sessionId); + if (!session) return { status: 'deleted' }; + if (session.name === desiredName) return { status: 'confirmed' }; + let confirmed = null; + try { + confirmed = await this._putSessionName(sessionId, desiredName); + } catch { + // 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(); + return { status: 'confirmed' }; + }); + writes.set(sessionId, task); + task.then(() => { + if (writes.get(sessionId) === task) writes.delete(sessionId); + }); + return task; + }, + async saveSessionName() { if (!this.editingSessionId) return; // Captured: the modal can be closed (or switched to another session) while @@ -2957,18 +2992,15 @@ Object.assign(CodemanApp.prototype, { if (fullName === session.name) restoreOriginalChildren(); else tabName.textContent = fullName || originalContent; - // Skip the API call if the session vanished between focus and blur. - const stillExists = this.sessions.has(sessionId); - if (stillExists && fullName !== session.name) { - const confirmed = await this._putSessionName(sessionId, fullName); + // Skip the API call if the session vanished between focus and blur. The + // queue applies the confirmed name to this.sessions before the re-render + // below repaints from it (see _applyLocalSessionName()). + if (this.sessions.has(sessionId)) { + const result = await this._queueInlineSessionName(sessionId, fullName); if (invalidated || this._activeRename !== renameHandle || !this.sessions.has(sessionId)) return; - if (confirmed === null) { + if (result.status === 'failed') { restoreOriginalChildren(); 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/src/web/public/styles.css b/src/web/public/styles.css index 6dc388b3..f3ef5797 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -1848,22 +1848,52 @@ html[data-tab-orientation='vertical'] .tab-rail .session-tab .tab-name-prefix { -webkit-line-clamp: unset; line-clamp: unset; overflow: visible; + /* An input's intrinsic width counts toward the label's min-content, so with + the default min-width:auto a long `w-` prefix plus the input + pushed the label past its row: the prefix slid out of view and, in the + compact rail, the input was clipped mid-word. */ + min-width: 0; } +/* The prefix gives way first, down to a stub with an ellipsis. */ :is( html[data-tab-orientation='vertical'] .tab-rail, html[data-session-list='sidebar'] .session-sidebar ) .tab-name-renaming .tab-rename-prefix { - flex: 0 0 auto; + flex: 0 1 auto; + min-width: 2rem; + max-width: 50%; + overflow: hidden; + 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. */ :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: 0; + min-width: 4rem !important; +} + +/* A compact rail row has no room for both the editor and its adornments, so + they step aside while the name is being edited (the re-render that ends the + edit brings them back). */ +html[data-tab-orientation='vertical'].tab-rail-compact + .tab-rail + .session-tab:has(.tab-name-renaming) + :is( + .tab-mode, + .tab-exited-badge, + .tab-detached-badge, + .tab-badge, + .tab-subagent-badge, + .tab-ultracode-badge, + .tab-actions + ) { + display: none; } /* Tab folder path — hidden by default, shown via .tabs-show-folder on container */ @@ -18895,6 +18925,13 @@ html[data-tab-orientation='vertical'][data-tab-rail-detail='rich']:not(.tab-rail line-clamp: 3; } +/* ...except while it is being edited: this rule outranks the shared renaming + rule's unclamp, so restate it (the editor is a flex row, never clamped). */ +html[data-tab-orientation='vertical'][data-tab-rail-detail='rich']:not(.tab-rail-compact) .tab-rail .session-tab .tab-name.tab-name-renaming { + -webkit-line-clamp: unset; + line-clamp: unset; +} + html[data-tab-orientation='vertical'][data-tab-rail-detail='rich']:not(.tab-rail-compact) .tab-rail .session-tab .tab-folder { font-size: 0.66rem; margin-top: 0.1rem; diff --git a/test/inline-rename.test.ts b/test/inline-rename.test.ts index edee0564..82e7db40 100644 --- a/test/inline-rename.test.ts +++ b/test/inline-rename.test.ts @@ -13,7 +13,8 @@ * Strategy: stub a synthetic .tab-name node and a fake session entry, then * drive the rename function directly via page.evaluate(). No real PTY/tmux. * - * Port: 3164 (per MEMORY.md, ports 3150+ for tests) + * Ports: 3164, plus 3165 and 3192 for the two server-backed describes below + * (per MEMORY.md, ports 3150+ for tests) */ import { describe, it, expect, beforeAll, afterAll } from 'vitest'; @@ -21,6 +22,8 @@ import { chromium, type Browser, type Page } from 'playwright'; import { WebServer } from '../src/web/server.js'; const PORT = 3164; +const ORDERING_PORT = 3165; +const LONG_PREFIX_PORT = 3192; const BASE_URL = `http://localhost:${PORT}`; describe('Inline rename input', () => { @@ -581,54 +584,372 @@ describe('Inline rename input', () => { expect(await input.evaluate((node) => node.getBoundingClientRect().width)).toBeGreaterThan(0); }); - it('Vertical rail paints typing in an unclamped editor and restores the clamp on cancel', async () => { - await resetState(); - const id = 'vertical-live-input'; + // The rail has two row variants, and the detailed one clamps the name to + // three lines instead of two: the editor must come out unclamped in both, + // and cancelling must put back the clamp of the variant it was opened in. + it.each([ + { detail: 'simple', restoredClamp: '2' }, + { detail: 'rich', restoredClamp: '3' }, + ])( + 'Vertical rail ($detail rows) paints typing in an unclamped editor and restores the clamp on cancel', + async ({ detail, restoredClamp }) => { + await resetState(); + const id = `vertical-live-input-${detail}`; - await page.evaluate((sessionId) => { + await page.evaluate( + ({ sessionId, detail }) => { + const app = ( + window as unknown as { + app: { + sessions: Map; + startInlineRename: (id: string) => void; + }; + } + ).app; + document.documentElement.dataset.tabOrientation = 'vertical'; + document.documentElement.dataset.tabRailDetail = detail; + const rail = document.getElementById('tabRail') as HTMLElement; + const tab = document.createElement('div'); + tab.setAttribute('data-test-tab', '1'); + tab.className = 'session-tab'; + tab.innerHTML = + `` + + 'w9-case: old'; + rail.appendChild(tab); + app.sessions.set(sessionId, { id: sessionId, name: 'w9-case: old' }); + app.startInlineRename(sessionId); + }, + { sessionId: id, detail } + ); + + const label = page.locator(`.tab-name[data-session-id="${id}"]`); + const input = label.locator('input.tab-rename-input'); + await input.press(process.platform === 'darwin' ? 'Meta+A' : 'Control+A'); + await page.keyboard.type('edited title'); + + expect(await input.inputValue()).toBe('edited title'); + expect(await input.evaluate((node) => document.activeElement === node)).toBe(true); + expect(await label.evaluate((node) => node.classList.contains('tab-name-renaming'))).toBe(true); + expect(await label.evaluate((node) => getComputedStyle(node).webkitLineClamp)).toBe('none'); + expect(await input.evaluate((node) => node.getBoundingClientRect().width)).toBeGreaterThan(0); + + const settled = await page.evaluate((sessionId) => { + const app = (window as unknown as { app: { _activeRename: { cancel: () => void } | null } }).app; + app._activeRename?.cancel(); + const label = document.querySelector(`.tab-name[data-session-id="${sessionId}"]`) as HTMLElement; + const result = { + classActive: label.classList.contains('tab-name-renaming'), + inputPresent: !!label.querySelector('input.tab-rename-input'), + webkitLineClamp: getComputedStyle(label).webkitLineClamp, + }; + document.documentElement.dataset.tabOrientation = 'horizontal'; + return result; + }, id); + + expect(settled).toEqual({ classActive: false, inputPresent: false, webkitLineClamp: restoredClamp }); + } + ); +}); + +/** + * Two renames of one session can be in flight at once: commit, reopen the + * editor before the PUT answers, then commit or cancel again (or start a group + * rename, which cancels the session editor). The writes go out one at a time + * in the order they were made, and a confirmed write is applied locally even + * if the editor that made it has since been cancelled, so the tab never shows + * a name the server no longer holds. + */ +describe('Inline rename write ordering', () => { + let server: WebServer; + let browser: Browser; + let page: Page; + + type Pending = { body: string; resolve: (response: Response) => void }; + + beforeAll(async () => { + server = new WebServer(ORDERING_PORT, false, true); + await server.start(); + browser = await chromium.launch({ headless: true }); + page = await browser.newPage(); + await page.goto(`http://localhost:${ORDERING_PORT}`, { waitUntil: 'domcontentloaded' }); + await page.waitForFunction( + () => + typeof (window as { app?: unknown }).app !== 'undefined' && + !!(window as { app?: { sessions?: Map } }).app?.sessions + ); + }, 60000); + + afterAll(async () => { + if (browser) await browser.close(); + if (server) await server.stop(); + }, 60000); + + /** Mount a header-strip row for `id`, hold every PUT open, and open its editor. */ + async function mount(id: string, name: string): Promise { + await page.evaluate( + ({ id, name }) => { + const w = window as unknown as { + app: { + _activeRename: { cancel: () => void } | null; + sessions: Map; + sessionOrder: string[]; + }; + __pending: Array<{ body: string; resolve: (response: Response) => void }>; + __origFetch?: typeof window.fetch; + }; + w.app._activeRename?.cancel(); + w.app.sessions.clear(); + document.querySelectorAll('[data-test-tab]').forEach((n) => n.remove()); + w.app.sessions.set(id, { id, name, status: 'idle' }); + w.app.sessionOrder = [id]; + const tab = document.createElement('div'); + tab.setAttribute('data-test-tab', '1'); + tab.className = 'session-tab'; + tab.dataset.id = id; + tab.innerHTML = `${name}`; + (document.getElementById('sessionTabs') as HTMLElement).appendChild(tab); + w.__pending = []; + w.__origFetch ??= window.fetch; + const passThrough = w.__origFetch; + window.fetch = ((input: RequestInfo | URL, init?: RequestInit) => { + if (init?.method !== 'PUT' || !String(input).endsWith('/name')) return passThrough(input, init); + return new Promise((resolve) => { + w.__pending.push({ body: String(init?.body ?? ''), resolve }); + }); + }) as typeof window.fetch; + }, + { id, name } + ); + } + + async function restoreFetch(): Promise { + await page.evaluate(() => { + const w = window as unknown as { __origFetch?: typeof window.fetch }; + if (w.__origFetch) window.fetch = w.__origFetch; + }); + } + + async function commit(id: string, value: string | null): Promise { + await page.evaluate( + async ({ id, value }) => { + const app = (window as unknown as { app: { startInlineRename: (id: string) => void } }).app; + if (!document.querySelector(`.tab-name[data-session-id="${id}"] input.tab-rename-input`)) { + app.startInlineRename(id); + } + const input = document.querySelector( + `.tab-name[data-session-id="${id}"] input.tab-rename-input` + ) as HTMLInputElement; + if (value !== null) input.value = value; + input.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true })); + await new Promise((resolve) => setTimeout(resolve, 0)); + }, + { id, value } + ); + } + + async function answer(index: number, name: string): Promise { + await page.evaluate( + async ({ index, body }) => { + const w = window as unknown as { __pending: Pending[] }; + w.__pending[index]?.resolve( + new Response(body, { status: 200, headers: { 'Content-Type': 'application/json' } }) + ); + await new Promise((resolve) => setTimeout(resolve, 30)); + }, + { index, body: JSON.stringify({ success: true, data: { name } }) } + ); + } + + async function state(id: string) { + return page.evaluate((id) => { + const w = window as unknown as { + app: { sessions: Map; _activeRename: unknown }; + __pending: Pending[]; + }; + return { + bodies: w.__pending.map(({ body }) => JSON.parse(body).name), + mapName: w.app.sessions.get(id)?.name ?? null, + renameActive: !!w.app._activeRename, + }; + }, id); + } + + it('sends successive renames of one session one at a time, in the order they were made', async () => { + await mount('order', 'Old'); + await commit('order', 'First'); + await commit('order', 'Second'); + expect((await state('order')).bodies).toEqual(['First']); + + await answer(0, 'First'); + expect((await state('order')).bodies).toEqual(['First', 'Second']); + await answer(1, 'Second'); + await restoreFetch(); + expect(await state('order')).toEqual({ bodies: ['First', 'Second'], mapName: 'Second', renameActive: false }); + }); + + it('keeps a confirmed rename when the editor reopened over it is cancelled', async () => { + await mount('reopen', 'Old'); + await commit('reopen', 'First'); + await page.evaluate(() => { + const app = ( + window as unknown as { app: { startInlineRename: (id: string) => void; _activeRename: { cancel: () => void } } } + ).app; + app.startInlineRename('reopen'); + app._activeRename.cancel(); + }); + await answer(0, 'First'); + await restoreFetch(); + 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 () => { + 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". + 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 }); + }); + + it('keeps a confirmed session rename when a group rename takes over the editor', async () => { + await mount('to-group', 'Old'); + await commit('to-group', 'Saved'); + const groupStarted = await page.evaluate(() => { const app = ( window as unknown as { app: { - sessions: Map; - startInlineRename: (id: string) => void; + tabLayout: unknown; + startTabGroupRename: (groupId: string) => boolean; }; } ).app; - document.documentElement.dataset.tabOrientation = 'vertical'; - const rail = document.getElementById('tabRail') as HTMLElement; - const tab = document.createElement('div'); - tab.setAttribute('data-test-tab', '1'); - tab.className = 'session-tab'; - tab.innerHTML = - `` + - 'w9-case: old'; - rail.appendChild(tab); - app.sessions.set(sessionId, { id: sessionId, name: 'w9-case: old' }); - app.startInlineRename(sessionId); - }, id); + const section = document.createElement('section'); + section.setAttribute('data-test-tab', '1'); + section.innerHTML = + '
' + + 'Group
'; + (document.getElementById('sessionTabs') as HTMLElement).appendChild(section); + (window as unknown as { __origLayout: unknown }).__origLayout = app.tabLayout; + app.tabLayout = { version: 1, groups: [{ id: 'g1', name: 'Group', refs: [] }], ungrouped: [] }; + return app.startTabGroupRename('g1'); + }); + expect(groupStarted).toBe(true); - const label = page.locator(`.tab-name[data-session-id="${id}"]`); - const input = label.locator('input.tab-rename-input'); - await input.press(process.platform === 'darwin' ? 'Meta+A' : 'Control+A'); - await page.keyboard.type('edited title'); - - expect(await input.inputValue()).toBe('edited title'); - expect(await input.evaluate((node) => document.activeElement === node)).toBe(true); - expect(await label.evaluate((node) => node.classList.contains('tab-name-renaming'))).toBe(true); - expect(await label.evaluate((node) => getComputedStyle(node).webkitLineClamp)).toBe('none'); - expect(await input.evaluate((node) => node.getBoundingClientRect().width)).toBeGreaterThan(0); - - const settled = await page.evaluate((sessionId) => { - const app = (window as unknown as { app: { _activeRename: { cancel: () => void } | null } }).app; - app._activeRename?.cancel(); - const label = document.querySelector(`.tab-name[data-session-id="${sessionId}"]`) as HTMLElement; - return { - classActive: label.classList.contains('tab-name-renaming'), - inputPresent: !!label.querySelector('input.tab-rename-input'), - webkitLineClamp: getComputedStyle(label).webkitLineClamp, + await answer(0, 'Saved'); + const after = await page.evaluate(() => { + const w = window as unknown as { + app: { + sessions: Map; + _inlineRenameActive: boolean; + _activeRename: { cancel: () => void } | null; + tabLayout: unknown; + }; + __origLayout: unknown; }; - }, id); + const groupInput = document.querySelector('.tab-layout-group-rename-input'); + const result = { + mapName: w.app.sessions.get('to-group')?.name ?? null, + groupEditorOpen: !!groupInput?.isConnected, + guardHeld: w.app._inlineRenameActive, + }; + w.app._activeRename?.cancel(); + w.app.tabLayout = w.__origLayout; + return result; + }); + await restoreFetch(); + expect(after).toEqual({ mapName: 'Saved', groupEditorOpen: true, guardHeld: true }); + }); +}); - expect(settled).toEqual({ classActive: false, inputPresent: false, webkitLineClamp: '2' }); +/** + * Real rows, rendered by the app from a live session: a long `w-` + * prefix must not push the editor (or the prefix itself) out of the row in any + * rail variant. The prefix gives way first, with an ellipsis, and the input + * always keeps a usable width. + */ +describe('Vertical rail rename editor with a long prefix', () => { + let server: WebServer; + let browser: Browser; + const port = LONG_PREFIX_PORT; + const NAME = 'w3-this_is_a_very_long_valid_prefix: charlie'; + let sessionId = ''; + + beforeAll(async () => { + server = new WebServer(port, false, true); + await server.start(); + const res = await fetch(`http://localhost:${port}/api/sessions`, { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + body: JSON.stringify({ name: NAME, mode: 'shell' }), + }); + expect(res.ok).toBe(true); + const created = (await res.json()) as { data?: { id?: string; session?: { id?: string } } }; + sessionId = created.data?.session?.id ?? created.data?.id ?? ''; + expect(sessionId).not.toBe(''); + browser = await chromium.launch({ headless: true }); + }, 60000); + + afterAll(async () => { + if (browser) await browser.close(); + if (server) await server.stop(); + }, 60000); + + it.each([ + { 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 }, + ])('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 { + await context.addInitScript( + (value) => localStorage.setItem('codeman-app-settings', JSON.stringify(value)), + settings + ); + const page = await context.newPage(); + await page.goto(`http://localhost:${port}`, { waitUntil: 'domcontentloaded' }); + const row = page.locator(`#tabRail .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); + + await row.click({ button: 'right' }); + const input = row.locator('input.tab-rename-input'); + await input.press('Control+A'); + await page.keyboard.type('typed live text'); + expect(await input.inputValue()).toBe('typed live text'); + + const geometry = await row.evaluate((tab) => { + const box = (el: Element) => el.getBoundingClientRect(); + const within = (inner: DOMRect, outer: DOMRect) => + inner.left >= outer.left - 0.5 && + inner.right <= outer.right + 0.5 && + inner.top >= outer.top - 0.5 && + inner.bottom <= outer.bottom + 0.5; + const input = tab.querySelector('input.tab-rename-input') as HTMLInputElement; + const prefix = tab.querySelector('.tab-rename-prefix') as HTMLElement; + const info = tab.querySelector('.tab-info') as HTMLElement; + return { + focused: document.activeElement === input, + inputWidth: box(input).width, + prefixWidth: box(prefix).width, + inputInsideRow: within(box(input), box(info)), + prefixInsideRow: within(box(prefix), box(info)), + prefixEllipsis: getComputedStyle(prefix).textOverflow, + }; + }); + expect(geometry.focused).toBe(true); + expect(geometry.inputWidth).toBeGreaterThanOrEqual(64); + expect(geometry.prefixWidth).toBeGreaterThanOrEqual(24); + expect(geometry.inputInsideRow).toBe(true); + expect(geometry.prefixInsideRow).toBe(true); + expect(geometry.prefixEllipsis).toBe('ellipsis'); + + await input.press('Escape'); + expect(await row.locator('input.tab-rename-input').count()).toBe(0); + } finally { + await context.close(); + } }); }); From 92f51fa6194240802a1625367b75221925aba146 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Sun, 4 Oct 2026 20:12:36 -0400 Subject: [PATCH 2/3] fix(tabs): inline rename review fixes Reopening the editor over a rename still in flight filled it from the name the server had not replaced yet, so dismissing it (blur commits) queued the old name behind the new one and undid the rename. The queue now records the newest queued name per session (_inlineRenamePending, cleared with the queue entry), and a reopened editor takes its prefix, input and "unchanged" comparison from it. An untouched confirm sends nothing more. A failed write only toasted while its editor was still current. The queue reports the failure itself now, and the editor only puts its label back. One rejected task blocked every later rename of that session until reload. Each task now chains from a settled predecessor, the local apply after a successful PUT is guarded, and the queue entry is cleaned up on either outcome. The rail and sidebar editor's 4rem floor moves from a stylesheet `!important` into the inline min-width startInlineRename already writes per layout (0 in the header strip, 4rem in the rail and sidebar). Tests: the reopened-editor case now expects only "First" to be sent; new cases cover a 500 answered after the editor is gone and a throw in updateSubagentParentNames; the long-prefix check runs in the sidebar and detailed sidebar too and asserts the inline floor; the header strip editor keeps min-width 0. --- src/web/public/session-ui.js | 62 +++++++++++----- src/web/public/styles.css | 5 +- test/inline-rename.test.ts | 135 +++++++++++++++++++++++++++++++++-- 3 files changed, 175 insertions(+), 27 deletions(-) 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); From 3e768e1b5bb321a4795a6641410462fb8a2c1e30 Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Mon, 5 Oct 2026 09:47:08 -0400 Subject: [PATCH 3/3] fix(tabs): show the in-flight name when an unchanged rename is confirmed --- src/web/public/session-ui.js | 6 +++++- test/inline-rename.test.ts | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index a5036056..e8e84dc3 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -2936,6 +2936,7 @@ Object.assign(CodemanApp.prototype, { // 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 renameInFlight = shownName !== session.name; const parsed = parseSessionPrefix(shownName); const originalContent = tabName.textContent; const originalChildren = [...tabName.childNodes].map((node) => node.cloneNode(true)); @@ -3017,7 +3018,10 @@ Object.assign(CodemanApp.prototype, { const suffix = input.value.trim(); const fullName = parsed ? parsed.prefix + (suffix ? ': ' + suffix : '') : suffix; - if (fullName === shownName) restoreOriginalChildren(); + // An unchanged confirm puts the old label back, unless the editor opened + // over a rename in flight: that label was repainted from the server's + // older name, so show the in-flight name rather than make it look lost. + if (fullName === shownName && !renameInFlight) restoreOriginalChildren(); else tabName.textContent = fullName || originalContent; // Skip the API call if the session vanished between focus and blur. The diff --git a/test/inline-rename.test.ts b/test/inline-rename.test.ts index e851f310..aa8c7c17 100644 --- a/test/inline-rename.test.ts +++ b/test/inline-rename.test.ts @@ -828,6 +828,26 @@ describe('Inline rename write ordering', () => { ).toBe(false); }); + it('shows the in-flight name when a reopened editor is confirmed unchanged, before the PUT lands', async () => { + await mount('shown', 'Old'); + await commit('shown', 'First'); + // Reopen while the PUT for "First" is held, then confirm it untouched. The + // label must read "First" now, not the "Old" the cancelled editor + // repainted from app.sessions. + await page.evaluate(() => + (window as unknown as { app: { startInlineRename: (id: string) => void } }).app.startInlineRename('shown') + ); + await commit('shown', null); + const label = await page.evaluate( + () => (document.querySelector('.tab-name[data-session-id="shown"]') as HTMLElement).textContent + ); + expect((await state('shown')).bodies).toEqual(['First']); + expect(label).toBe('First'); + await answer(0, 'First'); + await restoreFetch(); + expect(await state('shown')).toEqual({ bodies: ['First'], mapName: 'First', renameActive: false }); + }); + it('reports a failed write even after its editor is gone', async () => { await mount('fail-late', 'Old'); await page.evaluate(() => {