mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-09-30 12:39:42 +02:00
fix(ui): stop dropping the session name typed in the options modal
Two independent ways a tab description could be typed in and silently lost. 1. Session Options modal (deterministic). The Session Name input saves on blur, and every autosave handler in the modal bails on a null editingSessionId. closeSessionOptions() cleared that id BEFORE hiding the modal, and hiding it is what blurs the input, so the save always ran too late and returned early. Escape and backdrop-click lost the name with no PUT at all; only the X button worked, because mousedown blurs the input before the click handler runs. Fix: blur the focused modal field first, then clear the id. That also covers the auto-compact prompt, which saves on change and had the same fate. 2. Right-click inline rename (racy). The _inlineRenameActive guard from #81 sits in renderSessionTabs() (the scheduler) and _fullRenderSessionTabs(), but not in _renderSessionTabsImmediate() (the debounced executor). A render queued in the ~100ms before the rename opened still fires and the incremental branch rewrites .tab-name's innerHTML, destroying the input mid-keystroke: it commits a truncated name, or, if it lands before the first keystroke, closes the rename so everything typed after goes nowhere. Fix: guard the executor too. finishRename() re-renders on both commit and cancel, so a render dropped there is picked back up. Verified end-to-end against a live server on an isolated instance: all three modal close paths now persist the name, and the rename input survives a render mid-typing. Both regression tests were checked to fail with their fix reverted; the render one was vacuous at first because the synthetic tab sat on <body> instead of inside #sessionTabs, so it now builds the tab in the real container. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Codeman maintainer
@@ -3290,6 +3290,13 @@ class CodemanApp {
|
||||
}
|
||||
|
||||
_renderSessionTabsImmediate() {
|
||||
// Same guard as renderSessionTabs()/_fullRenderSessionTabs(): the incremental
|
||||
// branch below rewrites .tab-name's innerHTML, which destroys the inline rename
|
||||
// <input> mid-keystroke. Guarding only the scheduler is not enough: a render
|
||||
// debounced just BEFORE the rename opened still fires ~100ms later and lands
|
||||
// here directly. finishRename() re-renders on both commit and cancel, so a
|
||||
// render dropped here is picked back up when the rename settles.
|
||||
if (this._inlineRenameActive) return;
|
||||
const container = this.$('sessionTabs');
|
||||
const existingTabs = container.querySelectorAll('.session-tab[data-id]');
|
||||
const existingIds = new Set([...existingTabs].map(t => t.dataset.id));
|
||||
|
||||
@@ -848,6 +848,18 @@ Object.assign(CodemanApp.prototype, {
|
||||
},
|
||||
|
||||
closeSessionOptions() {
|
||||
// Commit the field the user was still editing BEFORE editingSessionId is
|
||||
// cleared. The Session Name input saves on blur (and the auto-compact prompt
|
||||
// on change), and every autosave handler bails out on `!this.editingSessionId`.
|
||||
// Hiding the modal blurs the focused input on its own, but that happens after
|
||||
// the id is gone, so Escape / backdrop-click silently dropped what was typed.
|
||||
// (Clicking the X worked only because mousedown blurs the input first.)
|
||||
const modal = document.getElementById('sessionOptionsModal');
|
||||
const focused = document.activeElement;
|
||||
if (focused && modal && modal.contains(focused) && typeof focused.blur === 'function') {
|
||||
focused.blur();
|
||||
}
|
||||
|
||||
this.editingSessionId = null;
|
||||
// Stop run summary auto-refresh if it was running
|
||||
this.stopRunSummaryAutoRefresh();
|
||||
|
||||
@@ -245,6 +245,119 @@ describe('Inline rename input', () => {
|
||||
expect(result.threw).toBe(false);
|
||||
});
|
||||
|
||||
it('Render guard: _renderSessionTabsImmediate() does not destroy an open rename input', async () => {
|
||||
await resetState();
|
||||
|
||||
// The debounced tab render is scheduled by renderSessionTabs() but EXECUTED by
|
||||
// _renderSessionTabsImmediate(). A render queued just before the rename opened
|
||||
// still fires ~100ms later and lands in the executor directly, so the guard has
|
||||
// to live there too, otherwise the incremental branch rewrites .tab-name's
|
||||
// innerHTML and the user's half-typed description is lost.
|
||||
//
|
||||
// The tab MUST live inside the real #sessionTabs container and be the only
|
||||
// session in app.sessions: the renderer walks that container, so a synthetic
|
||||
// node parked on <body> would make this test pass with the guard removed.
|
||||
const result = await page.evaluate(() => {
|
||||
const app = (
|
||||
window as unknown as {
|
||||
app: {
|
||||
sessions: Map<string, { id: string; name: string; status: string }>;
|
||||
sessionOrder: string[];
|
||||
startInlineRename: (id: string) => void;
|
||||
_renderSessionTabsImmediate: () => void;
|
||||
_activeRename: unknown;
|
||||
};
|
||||
}
|
||||
).app;
|
||||
const id = 'render-race';
|
||||
app.sessions.set(id, { id, name: 'w9-case', status: 'idle' });
|
||||
app.sessionOrder = [id];
|
||||
|
||||
const container = document.getElementById('sessionTabs') as HTMLElement;
|
||||
const tab = document.createElement('div');
|
||||
tab.setAttribute('data-test-tab', '1');
|
||||
tab.className = 'session-tab';
|
||||
tab.dataset.id = id;
|
||||
tab.innerHTML =
|
||||
'<span class="tab-status idle"></span><span class="tab-info"><span class="tab-name-row">' +
|
||||
`<span class="tab-name" data-session-id="${id}">w9-case</span>` +
|
||||
'</span></span>';
|
||||
container.appendChild(tab);
|
||||
|
||||
app.startInlineRename(id);
|
||||
const input = document.querySelector('input.tab-rename-input') as HTMLInputElement | null;
|
||||
if (!input) return { opened: false };
|
||||
input.value = 'half-typed';
|
||||
|
||||
// Exactly what a debounce timer queued before the rename would do.
|
||||
app._renderSessionTabsImmediate();
|
||||
|
||||
const after = document.querySelector('input.tab-rename-input') as HTMLInputElement | null;
|
||||
return {
|
||||
opened: true,
|
||||
stillInDom: !!after && document.body.contains(after),
|
||||
value: after?.value ?? null,
|
||||
renameStillActive: !!app._activeRename,
|
||||
};
|
||||
});
|
||||
|
||||
expect(result.opened).toBe(true);
|
||||
expect(result.stillInDom).toBe(true);
|
||||
expect(result.value).toBe('half-typed');
|
||||
expect(result.renameStillActive).toBe(true);
|
||||
});
|
||||
|
||||
it('Modal: closeSessionOptions() commits the Session Name field before clearing the id', async () => {
|
||||
await resetState();
|
||||
|
||||
// Every autosave handler in the session-options modal bails on a null
|
||||
// editingSessionId, and hiding the modal blurs the focused input. If the id is
|
||||
// cleared first, the blur-driven save is dropped and the typed name vanishes,
|
||||
// which is what Escape and backdrop-click used to do.
|
||||
const result = await page.evaluate(async () => {
|
||||
const app = (
|
||||
window as unknown as {
|
||||
app: {
|
||||
editingSessionId: string | null;
|
||||
sessions: Map<string, { id: string; name: string }>;
|
||||
closeSessionOptions: () => void;
|
||||
};
|
||||
}
|
||||
).app;
|
||||
app.sessions.set('modal-id', { id: 'modal-id', name: 'w9-case' });
|
||||
app.editingSessionId = 'modal-id';
|
||||
|
||||
const nameInput = document.getElementById('modalSessionName') as HTMLInputElement;
|
||||
const modal = document.getElementById('sessionOptionsModal') as HTMLElement;
|
||||
modal.classList.add('active');
|
||||
// The Session Name field lives on the modal's Context tab, which is hidden
|
||||
// until selected: a hidden input cannot take focus.
|
||||
document.getElementById('context-tab')?.classList.remove('hidden');
|
||||
nameInput.value = 'mydesc';
|
||||
nameInput.focus();
|
||||
const wasFocused = document.activeElement === nameInput;
|
||||
|
||||
let putBody: string | null = null;
|
||||
const origFetch = window.fetch;
|
||||
window.fetch = (async (input: RequestInfo | URL, init?: RequestInit) => {
|
||||
if (String(input).includes('/api/sessions/modal-id/name')) putBody = String(init?.body ?? '');
|
||||
return new Response('{"success":true}', { status: 200 });
|
||||
}) as typeof window.fetch;
|
||||
|
||||
app.closeSessionOptions();
|
||||
await new Promise((r) => setTimeout(r, 30));
|
||||
window.fetch = origFetch;
|
||||
modal.classList.remove('active');
|
||||
|
||||
return { wasFocused, putBody, editingAfter: app.editingSessionId };
|
||||
});
|
||||
|
||||
expect(result.wasFocused).toBe(true);
|
||||
// Prefixed session: the suffix the user typed is appended to the w9-case prefix.
|
||||
expect(result.putBody).toContain('w9-case: mydesc');
|
||||
expect(result.editingAfter).toBe(null);
|
||||
});
|
||||
|
||||
it('Re-entry: starting rename while one is active aborts the previous one', async () => {
|
||||
await resetState();
|
||||
expect(await startRename('first-id', 'First')).toBe(true);
|
||||
|
||||
Reference in New Issue
Block a user