mirror of
https://github.com/Ark0N/Codeman.git
synced 2026-10-02 05:29:42 +02:00
fix(rename): apply the server's confirmed name locally instead of waiting on SSE
Renaming a tab appeared to do nothing: the new name only showed after a full page reload. The PUT always succeeded; what was broken is how the tab strip learns the result. `finishRename()` re-renders the strip from the client-side `app.sessions` map, and nothing wrote the new name into that map, so the rename depended on the `session:updated` SSE frame to carry its own write back. On a page whose stream has gone quiet without erroring, that frame never lands and the re-render repaints the stale label. - `_applyLocalSessionName()` writes the confirmed name into `this.sessions` and refreshes cached subagent parent names, mirroring `_onSessionUpdated`. - `_putSessionName()` returns the stored name or null. `_apiPut` turns a network error into a null Response and an API failure into a non-ok status, so a rejected rename previously read as success and silently dropped the edit (the old try/catch could never fire). - Both surfaces use them: `startInlineRename()`'s `finishRename` and `saveSessionName()`. Two regression tests: the commit applies the name with no SSE frame dispatched, and a 500 restores the old label, leaves the map untouched, and toasts. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1355,9 +1355,54 @@ Object.assign(CodemanApp.prototype, {
|
||||
this.activeFocusTrap.activate();
|
||||
},
|
||||
|
||||
/**
|
||||
* Write a name the server has just confirmed into the local session map.
|
||||
*
|
||||
* Both rename surfaces re-render the tab strip from `this.sessions` right
|
||||
* after their PUT, so without this they depended on the `session:updated` SSE
|
||||
* frame to carry their own write back. On a page whose SSE stream has gone
|
||||
* quiet without erroring (a proxy that idle-closed it, a laptop resumed from
|
||||
* sleep) that frame never lands: the PUT stores the new name, the re-render
|
||||
* repaints the stale one, and the rename looks like it did nothing until a
|
||||
* full page reload. The response body is authoritative, so apply it directly.
|
||||
* The SSE frame, when it does arrive, replaces the object with the same name.
|
||||
*/
|
||||
_applyLocalSessionName(sessionId, name) {
|
||||
if (typeof name !== 'string') return;
|
||||
const session = this.sessions.get(sessionId);
|
||||
if (!session) return;
|
||||
session.name = name;
|
||||
this.sessions.set(sessionId, session);
|
||||
// Mirrors _onSessionUpdated: subagent windows cache their parent's name.
|
||||
this.updateSubagentParentNames?.(sessionId);
|
||||
},
|
||||
|
||||
/**
|
||||
* PUT a session name and return the name the server stored, or null if the
|
||||
* request failed. `_apiPut` swallows network errors into a null Response and
|
||||
* an API-level failure arrives as a non-ok status or `{success:false}`, so a
|
||||
* rename that silently did nothing has to be detected here, not thrown.
|
||||
*/
|
||||
async _putSessionName(sessionId, name) {
|
||||
const res = await this._apiPut(`/api/sessions/${sessionId}/name`, { name });
|
||||
if (!res || !res.ok) return null;
|
||||
let payload = null;
|
||||
try {
|
||||
payload = await res.json();
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
if (payload && payload.success === false) return null;
|
||||
const confirmed = payload?.data?.name;
|
||||
return typeof confirmed === 'string' ? confirmed : name;
|
||||
},
|
||||
|
||||
async saveSessionName() {
|
||||
if (!this.editingSessionId) return;
|
||||
const session = this.sessions.get(this.editingSessionId);
|
||||
// Captured: the modal can be closed (or switched to another session) while
|
||||
// the PUT is in flight, and the name belongs to the session that was open.
|
||||
const sessionId = this.editingSessionId;
|
||||
const session = this.sessions.get(sessionId);
|
||||
const parsed = session ? parseSessionPrefix(session.name) : null;
|
||||
const inputVal = document.getElementById('modalSessionName').value.trim();
|
||||
let name;
|
||||
@@ -1366,11 +1411,13 @@ Object.assign(CodemanApp.prototype, {
|
||||
} else {
|
||||
name = inputVal;
|
||||
}
|
||||
try {
|
||||
await this._apiPut(`/api/sessions/${this.editingSessionId}/name`, { name });
|
||||
} catch (err) {
|
||||
this.showToast('Failed to save session name: ' + err.message, 'error');
|
||||
const confirmed = await this._putSessionName(sessionId, name);
|
||||
if (confirmed === null) {
|
||||
this.showToast('Failed to save session name', 'error');
|
||||
return;
|
||||
}
|
||||
this._applyLocalSessionName(sessionId, confirmed);
|
||||
this.renderSessionTabs();
|
||||
},
|
||||
|
||||
async autoSaveAutoCompact() {
|
||||
@@ -1680,15 +1727,14 @@ Object.assign(CodemanApp.prototype, {
|
||||
// Skip the API call if the session vanished between focus and blur.
|
||||
const stillExists = this.sessions.has(sessionId);
|
||||
if (stillExists && fullName !== session.name) {
|
||||
try {
|
||||
await fetch(`/api/sessions/${sessionId}/name`, {
|
||||
method: 'PUT',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify({ name: fullName })
|
||||
});
|
||||
} catch (err) {
|
||||
const confirmed = await this._putSessionName(sessionId, fullName);
|
||||
if (confirmed === null) {
|
||||
tabName.textContent = originalContent;
|
||||
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
|
||||
|
||||
@@ -358,6 +358,83 @@ describe('Inline rename input', () => {
|
||||
expect(result.editingAfter).toBe(null);
|
||||
});
|
||||
|
||||
it('Commit writes the confirmed name into app.sessions WITHOUT any session:updated frame', async () => {
|
||||
await resetState();
|
||||
expect(await startRename('no-sse', 'w9-case')).toBe(true);
|
||||
|
||||
// finishRename() re-renders the tab strip from app.sessions, so the rename
|
||||
// used to depend on the session:updated SSE frame to carry its own write
|
||||
// back. On a page whose stream has gone quiet without erroring, the PUT
|
||||
// stored the new name, the re-render repainted the stale one, and the tab
|
||||
// only showed it after a full reload. No SSE is dispatched here at all.
|
||||
const result = await page.evaluate(async () => {
|
||||
const app = (
|
||||
window as unknown as {
|
||||
app: { sessions: Map<string, { id: string; name: string }> };
|
||||
}
|
||||
).app;
|
||||
const origFetch = window.fetch;
|
||||
window.fetch = (async () =>
|
||||
new Response('{"success":true,"data":{"name":"w9-case: fresh"}}', {
|
||||
status: 200,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
})) as typeof window.fetch;
|
||||
|
||||
const inputEl = document.querySelector('input.tab-rename-input') as HTMLInputElement;
|
||||
inputEl.value = 'fresh';
|
||||
inputEl.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }));
|
||||
await new Promise((r) => setTimeout(r, 60));
|
||||
|
||||
window.fetch = origFetch;
|
||||
return { mapName: app.sessions.get('no-sse')?.name ?? null };
|
||||
});
|
||||
|
||||
expect(result.mapName).toBe('w9-case: fresh');
|
||||
});
|
||||
|
||||
it('A rejected rename restores the old label and leaves app.sessions untouched', async () => {
|
||||
await resetState();
|
||||
expect(await startRename('rename-500', 'w9-case')).toBe(true);
|
||||
|
||||
// _apiPut turns a network error into a null Response and an API-level
|
||||
// failure arrives as a non-ok status, neither of which throws, so a
|
||||
// rejected rename has to be detected from the response, or it reports
|
||||
// success and silently discards the user's edit.
|
||||
const result = await page.evaluate(async () => {
|
||||
const app = (
|
||||
window as unknown as {
|
||||
app: { sessions: Map<string, { id: string; name: string }>; showToast: (m: string, k: string) => void };
|
||||
}
|
||||
).app;
|
||||
const toasts: string[] = [];
|
||||
const origToast = app.showToast;
|
||||
app.showToast = (msg: string) => void toasts.push(msg);
|
||||
const origFetch = window.fetch;
|
||||
window.fetch = (async () =>
|
||||
new Response('{"success":false,"error":"boom","errorCode":"INTERNAL"}', {
|
||||
status: 500,
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
})) as typeof window.fetch;
|
||||
|
||||
const inputEl = document.querySelector('input.tab-rename-input') as HTMLInputElement;
|
||||
inputEl.value = 'never-stored';
|
||||
inputEl.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter', bubbles: true }));
|
||||
await new Promise((r) => setTimeout(r, 60));
|
||||
|
||||
window.fetch = origFetch;
|
||||
app.showToast = origToast;
|
||||
return {
|
||||
mapName: app.sessions.get('rename-500')?.name ?? null,
|
||||
label: document.querySelector('.tab-name[data-session-id="rename-500"]')?.textContent ?? null,
|
||||
toasts,
|
||||
};
|
||||
});
|
||||
|
||||
expect(result.mapName).toBe('w9-case');
|
||||
expect(result.label).toBe('w9-case');
|
||||
expect(result.toasts).toContain('Failed to rename');
|
||||
});
|
||||
|
||||
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