From 7a86cf87f7303f544739df7214d8eaff9cee5d5c Mon Sep 17 00:00:00 2001 From: Aamer Akhter Date: Sun, 21 Jun 2026 15:16:19 -0400 Subject: [PATCH] COD-143 retain session name when resuming from the session manager resumeHistorySession ignored the row's name and always synthesized a fresh w- name from the working dir, so resuming a custom-named session lost its name. Thread the name through resumeHistorySession(sessionId, workingDir, name) and extract the choice into a pure _resolveResumeName helper: prefer a non-empty existing name, else generate the next free w-. Forward s.name at all three call sites (terminal-ui.js history-item + session-manager menu, session-ui.js run-mode history); sessions without a name fall back to the generated name (unchanged behavior). The unified session rows already carry name, so session-manager rows resume with it. TDD: test/resume-name.test.ts drives the real _resolveResumeName via vm-harness. (cherry picked from commit 56c7906a48d8b453ed55810a02ec70f72d34ed32) --- src/web/public/session-ui.js | 2 +- src/web/public/terminal-ui.js | 37 ++++++++------ test/resume-name.test.ts | 91 +++++++++++++++++++++++++++++++++++ 3 files changed, 115 insertions(+), 15 deletions(-) create mode 100644 test/resume-name.test.ts diff --git a/src/web/public/session-ui.js b/src/web/public/session-ui.js index 14b30693..6ccee3cd 100644 --- a/src/web/public/session-ui.js +++ b/src/web/public/session-ui.js @@ -424,7 +424,7 @@ Object.assign(CodemanApp.prototype, { btn.append(dirSpan, metaSpan); btn.addEventListener('click', (e) => { e.stopPropagation(); - this.resumeHistorySession(s.sessionId, s.workingDir); + this.resumeHistorySession(s.sessionId, s.workingDir, s.name); }); container.appendChild(btn); } diff --git a/src/web/public/terminal-ui.js b/src/web/public/terminal-ui.js index 9dd9e940..7cccbaa5 100644 --- a/src/web/public/terminal-ui.js +++ b/src/web/public/terminal-ui.js @@ -1260,7 +1260,7 @@ Object.assign(CodemanApp.prototype, { if (isLive && this.sessions.has(s.sessionId)) { this.selectSession(s.sessionId); } else { - this.resumeHistorySession(s.claudeSessionId || s.sessionId, s.workingDir || ''); + this.resumeHistorySession(s.claudeSessionId || s.sessionId, s.workingDir || '', s.name); } }) ); @@ -1474,7 +1474,7 @@ Object.assign(CodemanApp.prototype, { } else { // Resume by the Claude conversation UUID when present (resumed sessions // carry theirs separately from their Codeman id). - this.resumeHistorySession(s.claudeSessionId || s.sessionId, s.workingDir || ''); + this.resumeHistorySession(s.claudeSessionId || s.sessionId, s.workingDir || '', s.name); } this.closeSessionManager?.(); closeMenu(); @@ -1772,7 +1772,24 @@ Object.assign(CodemanApp.prototype, { this._folderHistoryState = null; }, - async resumeHistorySession(sessionId, workingDir) { + // Choose the name for a resumed session: keep the session's own name when it + // has one, otherwise synthesize a fresh w- name (next free w-number + // across open sessions). COD-143 — resume used to always generate a new name. + _resolveResumeName(existingName, workingDir) { + if (typeof existingName === 'string' && existingName.trim()) return existingName; + const dirName = (workingDir || '').split('/').pop() || 'session'; + let startNumber = 1; + for (const [, session] of this.sessions) { + const match = session.name && session.name.match(/^w(\d+)-/); + if (match) { + const num = parseInt(match[1]); + if (num >= startNumber) startNumber = num + 1; + } + } + return `w${startNumber}-${dirName}`; + }, + + async resumeHistorySession(sessionId, workingDir, existingName) { // Close the run mode menu if open document.getElementById('runModeMenu')?.classList.remove('active'); // Close folder history modal if open @@ -1781,17 +1798,9 @@ Object.assign(CodemanApp.prototype, { this.terminal.clear(); this.terminal.writeln(`\x1b[1;32m Resuming conversation ${sessionId.slice(0, 8)}...\x1b[0m`); - // Generate a session name from the working dir - const dirName = workingDir.split('/').pop() || 'session'; - let startNumber = 1; - for (const [, session] of this.sessions) { - const match = session.name && session.name.match(/^w(\d+)-/); - if (match) { - const num = parseInt(match[1]); - if (num >= startNumber) startNumber = num + 1; - } - } - const name = `w${startNumber}-${dirName}`; + // Keep the session's own name when resuming; only synthesize a w- + // name when the source row had none (COD-143). + const name = this._resolveResumeName(existingName, workingDir); // Create session with resumeSessionId — include envOverrides so resumed // conversations inherit current UI settings (effort, agent teams, etc.). diff --git a/test/resume-name.test.ts b/test/resume-name.test.ts new file mode 100644 index 00000000..bf20eb34 --- /dev/null +++ b/test/resume-name.test.ts @@ -0,0 +1,91 @@ +/** + * @fileoverview COD-143 — resuming a session from the Session Manager must retain its + * original tab name, not synthesize a fresh `w-` name every time. + * + * Root cause: `resumeHistorySession(sessionId, workingDir)` ignored the row's `name` and + * always built `w-` from the working dir. The fix threads the name through and + * extracts the choice into a pure `_resolveResumeName(existingName, workingDir)` helper: + * prefer a non-empty existing name; otherwise generate the next free `w-` by + * scanning open sessions' names. + * + * This pins the helper's contract: + * 1. a non-empty existing name is returned verbatim (custom name retained), + * 2. a missing/empty/whitespace name falls back to `w-`, + * 3. the generated number is the next free w-index across `this.sessions`, + * 4. the generated dir segment is the basename of workingDir (or `session` when empty). + * + * Loaded via `vm` against a stub `CodemanApp` (no jsdom — same harness as + * file-browser-reveal.test.ts / connection-indicator.test.ts). terminal-ui.js does + * `Object.assign(CodemanApp.prototype, {...})` at module-eval, so we capture the real + * `_resolveResumeName` off the prototype and invoke it against a minimal host whose + * `sessions` is a Map. + */ + +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import vm from 'node:vm'; +import { describe, expect, it, vi } from 'vitest'; + +/** Eval the shipping terminal-ui.js into a vm with a stub CodemanApp, return its prototype. */ +function loadTerminalUiPrototype(): Record unknown> { + const source = readFileSync(resolve(import.meta.dirname, '../src/web/public/terminal-ui.js'), 'utf8'); + const context = vm.createContext({ + console, + CodemanApp: class CodemanApp {}, + setInterval: vi.fn(), + clearInterval: vi.fn(), + setTimeout, + clearTimeout, + requestAnimationFrame: vi.fn(), + document: { addEventListener: vi.fn(), getElementById: vi.fn() }, + window: { addEventListener: vi.fn(), removeEventListener: vi.fn() }, + }); + vm.runInContext(`${source}\nglobalThis.__proto = CodemanApp.prototype;`, context); + return (context as { __proto: Record unknown> }).__proto; +} + +const proto = loadTerminalUiPrototype(); + +/** Minimal host carrying the real `_resolveResumeName` + a sessions Map. */ +function makeApp(sessionNames: string[] = []) { + const sessions = new Map(); + sessionNames.forEach((name, i) => sessions.set(`s${i}`, { name })); + return { + sessions, + _resolveResumeName: proto._resolveResumeName as (existingName: unknown, workingDir: unknown) => string, + }; +} + +describe('COD-143 _resolveResumeName', () => { + it('returns a non-empty existing name verbatim (custom name retained)', () => { + const app = makeApp(['w1-foo', 'w2-bar']); + expect(app._resolveResumeName.call(app, 'my-custom-tab', '/home/me/proj')).toBe('my-custom-tab'); + }); + + it('falls back to w- when no name is given', () => { + const app = makeApp([]); + expect(app._resolveResumeName.call(app, undefined, '/home/me/proj')).toBe('w1-proj'); + }); + + it('treats empty / whitespace names as no-name (falls back)', () => { + const app = makeApp([]); + expect(app._resolveResumeName.call(app, '', '/a/b/widgets')).toBe('w1-widgets'); + expect(app._resolveResumeName.call(app, ' ', '/a/b/widgets')).toBe('w1-widgets'); + }); + + it('generated w-number is the next free index across open sessions', () => { + const app = makeApp(['w1-foo', 'w3-bar', 'plain-name']); + // highest w is 3 → next is 4 + expect(app._resolveResumeName.call(app, null, '/x/y/svc')).toBe('w4-svc'); + }); + + it('uses "session" as the dir segment when workingDir is empty', () => { + const app = makeApp([]); + expect(app._resolveResumeName.call(app, '', '')).toBe('w1-session'); + }); + + it('does not let a generated fallback clobber an explicit name even when sessions exist', () => { + const app = makeApp(['w1-foo', 'w2-bar']); + expect(app._resolveResumeName.call(app, 'keepme', '/p/q')).toBe('keepme'); + }); +});