From d331db114113102d9fe1262db8a7ae0774432889 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Tue, 6 Oct 2026 18:09:18 +0200 Subject: [PATCH] fix(tiles): Attach reads the response envelope; an exited agent gets no Attach Found live: the attach and shell routes report a refusal in the envelope of a 200 ({success: false}), and Attach read only res.ok, so a refused attach remounted the tile as if it had worked. It now reads the envelope. The refusal in question: an agent that exited in a live pane (paneExit, e.g. a shell ended with `exit 3`) still has the pane's tmux client running, so both routes refuse to start anything ("Session already has a running process"), and the single view has no restart for it either. Its tile now shows the exit with a pointer to Close session instead of an Attach button that cannot work. A session with no PTY attached, or one whose socket closed because it exited (4009), still gets Attach. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/web/public/styles.css | 9 +++++++- src/web/public/tile-grid.js | 43 ++++++++++++++++++++++++----------- test/tile-grid-attach.test.ts | 23 +++++++++++++++++-- 3 files changed, 59 insertions(+), 16 deletions(-) diff --git a/src/web/public/styles.css b/src/web/public/styles.css index 0cc5c850..a3da0dbb 100644 --- a/src/web/public/styles.css +++ b/src/web/public/styles.css @@ -19645,10 +19645,17 @@ body.tile-grid-resizing--row * { color: var(--text-muted); } -.tile-attach[hidden] { +.tile-attach[hidden], +.tile-attach [hidden] { display: none; } +.tile-attach-hint { + max-width: 80%; + font-size: 12px; + text-align: center; +} + .tile-attach-btn { padding: 4px 14px; font: inherit; diff --git a/src/web/public/tile-grid.js b/src/web/public/tile-grid.js index bb083a56..48d85af4 100644 --- a/src/web/public/tile-grid.js +++ b/src/web/public/tile-grid.js @@ -916,21 +916,27 @@ Object.assign(CodemanApp.prototype, { }, /** - * What the tile's body should say instead of a terminal, or '' for none: a - * session with no PTY attached (pid null), an agent that exited in a live - * pane (paneExit), or a socket the server closed because the session exited - * (4009). Attach was just pressed: nothing, while the server catches up. + * What the tile's body should say instead of a terminal, or null for none: a + * session with no PTY attached (pid null) or a socket the server closed + * because the session exited (4009), both of which Attach can start again; + * or an agent that exited in a live pane (paneExit), which it cannot: the + * attach and shell routes refuse while the pane's tmux client still runs + * ("already has a running process"), and the single view has no restart for + * it either, so the tile says so and points at Close session instead. Attach + * was just pressed: nothing, while the server catches up. + * + * @returns {{text: string, attachable: boolean}|null} */ _tileAttachReason(sessionId, tile) { const session = this.sessions.get(sessionId); - if (!session) return ''; + if (!session) return null; const pending = this._tileAttachPending?.get(sessionId); - if (pending && Date.now() - pending < 15000) return ''; + if (pending && Date.now() - pending < 15000) return null; const exited = typeof paneExitLabel === 'function' ? paneExitLabel(session.paneExit) : ''; - if (exited) return `The agent ${exited}`; - if (session.pid === null) return 'Not attached'; - if (tile?._stoppedCode === 4009) return 'The session ended'; - return ''; + if (exited) return { text: `The agent ${exited}`, attachable: false }; + if (session.pid === null) return { text: 'Not attached', attachable: true }; + if (tile?._stoppedCode === 4009) return { text: 'The session ended', attachable: true }; + return null; }, /** @@ -961,15 +967,22 @@ Object.assign(CodemanApp.prototype, { e.stopPropagation(); void this.attachTileSession(sessionId); }); - overlay.append(text, btn); + const hint = document.createElement('span'); + hint.className = 'tile-attach-hint'; + hint.textContent = 'It cannot be restarted in place: close it from \u22EF (Close session).'; + overlay.append(text, btn, hint); entry.body.appendChild(overlay); entry.overlay = overlay; entry.overlayText = text; entry.overlayBtn = btn; + entry.overlayHint = hint; } entry.overlay.hidden = false; - const text = busy ? 'Attaching\u2026' : reason; + const text = busy ? 'Attaching\u2026' : reason.text; if (entry.overlayText.textContent !== text) entry.overlayText.textContent = text; + const attachable = busy || reason.attachable; + entry.overlayBtn.hidden = !attachable; + entry.overlayHint.hidden = attachable; entry.overlayBtn.disabled = busy; }, @@ -984,6 +997,8 @@ Object.assign(CodemanApp.prototype, { async attachTileSession(sessionId) { const session = this.sessions.get(sessionId); if (!session) return false; + // An agent that exited in a live pane cannot be started again in place. + if (this._tileAttachReason(sessionId, this._tileFor(sessionId))?.attachable === false) return false; this._tileAttachInFlight ||= new Set(); if (this._tileAttachInFlight.has(sessionId)) return false; let url = `/api/sessions/${sessionId}/${session.mode === 'shell' ? 'shell' : 'interactive'}`; @@ -1003,7 +1018,9 @@ Object.assign(CodemanApp.prototype, { let ok = false; try { const res = await fetch(url, init); - ok = !!res?.ok; + // The routes report a refusal in the envelope of a 200. + const body = await res?.json?.().catch(() => null); + ok = !!res?.ok && body?.success !== false; } catch { ok = false; } finally { diff --git a/test/tile-grid-attach.test.ts b/test/tile-grid-attach.test.ts index 14aa078a..9d5a281b 100644 --- a/test/tile-grid-attach.test.ts +++ b/test/tile-grid-attach.test.ts @@ -63,10 +63,20 @@ describe('when the overlay shows', () => { expect(visible('s-a')).toBe(false); }); - it('an agent that exited in a live pane', () => { - gridWith((app) => (app.sessions.get('s-c').paneExit = { status: 2 })); + it('an agent that exited in a live pane: the reason, and no Attach (the server cannot restart it in place)', async () => { + const app = gridWith((a) => (a.sessions.get('s-c').paneExit = { status: 2 })); expect(visible('s-c')).toBe(true); expect(textOf('s-c')).toBe('The agent exited (2)'); + expect(attachButton('s-c').hidden).toBe(true); + expect(overlayOf('s-c')!.children[2].hidden).toBe(false); + expect(await app.attachTileSession('s-c')).toBe(false); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it('a session with no PTY gets the button, not the close hint', () => { + gridWith((app) => (app.sessions.get('s-b').pid = null)); + expect(attachButton('s-b').hidden).toBe(false); + expect(overlayOf('s-b')!.children[2].hidden).toBe(true); }); it('a socket closed because the session exited (4009) keeps the tile and shows it', () => { @@ -133,6 +143,15 @@ describe('Attach', () => { await first; }); + it('a refusal in the envelope of a 200 is a failure, not a success', async () => { + const app = gridWith((a) => (a.sessions.get('s-b').pid = null)); + fetchSpy.mockImplementation(async () => ({ ok: true, json: async () => ({ success: false, error: 'busy' }) })); + expect(await app.attachTileSession('s-b')).toBe(false); + expect(app.showToast).toHaveBeenCalledWith('Could not attach the session', 'error'); + expect(tilesFor('s-b')).toHaveLength(1); + expect(visible('s-b')).toBe(true); + }); + it('a failed attach keeps the overlay and says so', async () => { const app = gridWith((a) => (a.sessions.get('s-b').pid = null)); fetchSpy.mockImplementation(async () => ({ ok: false, json: async () => ({}) }));