mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-08 08:29:42 +02:00
COD-143 retain session name when resuming from the session manager
resumeHistorySession ignored the row's name and always synthesized a fresh w<N>-<dir> 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<N>-<dir>. 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)
This commit is contained in:
@@ -424,7 +424,7 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
btn.append(dirSpan, metaSpan);
|
btn.append(dirSpan, metaSpan);
|
||||||
btn.addEventListener('click', (e) => {
|
btn.addEventListener('click', (e) => {
|
||||||
e.stopPropagation();
|
e.stopPropagation();
|
||||||
this.resumeHistorySession(s.sessionId, s.workingDir);
|
this.resumeHistorySession(s.sessionId, s.workingDir, s.name);
|
||||||
});
|
});
|
||||||
container.appendChild(btn);
|
container.appendChild(btn);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1260,7 +1260,7 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
if (isLive && this.sessions.has(s.sessionId)) {
|
if (isLive && this.sessions.has(s.sessionId)) {
|
||||||
this.selectSession(s.sessionId);
|
this.selectSession(s.sessionId);
|
||||||
} else {
|
} 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 {
|
} else {
|
||||||
// Resume by the Claude conversation UUID when present (resumed sessions
|
// Resume by the Claude conversation UUID when present (resumed sessions
|
||||||
// carry theirs separately from their Codeman id).
|
// 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?.();
|
this.closeSessionManager?.();
|
||||||
closeMenu();
|
closeMenu();
|
||||||
@@ -1772,7 +1772,24 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
this._folderHistoryState = null;
|
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<N>-<dir> 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
|
// Close the run mode menu if open
|
||||||
document.getElementById('runModeMenu')?.classList.remove('active');
|
document.getElementById('runModeMenu')?.classList.remove('active');
|
||||||
// Close folder history modal if open
|
// Close folder history modal if open
|
||||||
@@ -1781,17 +1798,9 @@ Object.assign(CodemanApp.prototype, {
|
|||||||
this.terminal.clear();
|
this.terminal.clear();
|
||||||
this.terminal.writeln(`\x1b[1;32m Resuming conversation ${sessionId.slice(0, 8)}...\x1b[0m`);
|
this.terminal.writeln(`\x1b[1;32m Resuming conversation ${sessionId.slice(0, 8)}...\x1b[0m`);
|
||||||
|
|
||||||
// Generate a session name from the working dir
|
// Keep the session's own name when resuming; only synthesize a w<N>-<dir>
|
||||||
const dirName = workingDir.split('/').pop() || 'session';
|
// name when the source row had none (COD-143).
|
||||||
let startNumber = 1;
|
const name = this._resolveResumeName(existingName, workingDir);
|
||||||
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}`;
|
|
||||||
|
|
||||||
// Create session with resumeSessionId — include envOverrides so resumed
|
// Create session with resumeSessionId — include envOverrides so resumed
|
||||||
// conversations inherit current UI settings (effort, agent teams, etc.).
|
// conversations inherit current UI settings (effort, agent teams, etc.).
|
||||||
|
|||||||
@@ -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<N>-<dir>` name every time.
|
||||||
|
*
|
||||||
|
* Root cause: `resumeHistorySession(sessionId, workingDir)` ignored the row's `name` and
|
||||||
|
* always built `w<N>-<dir>` 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<N>-<dir>` 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<N>-<dir>`,
|
||||||
|
* 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<string, (...args: unknown[]) => 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<string, (...args: unknown[]) => unknown> }).__proto;
|
||||||
|
}
|
||||||
|
|
||||||
|
const proto = loadTerminalUiPrototype();
|
||||||
|
|
||||||
|
/** Minimal host carrying the real `_resolveResumeName` + a sessions Map. */
|
||||||
|
function makeApp(sessionNames: string[] = []) {
|
||||||
|
const sessions = new Map<string, { name: string }>();
|
||||||
|
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<N>-<dir> 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<N> 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');
|
||||||
|
});
|
||||||
|
});
|
||||||
Reference in New Issue
Block a user