feat(hub+web): add POST /sessions/:id/reopen + Reopen button on inactive rows (#826)

* feat(hub+web): add POST /sessions/:id/reopen + Reopen button on inactive rows Archived sessions retain their full transcript and metadata in the DB, but today there is no path back to them from the web UI; the only way to revive one is shell access plus sqlite metadata patching plus a manual /resume call. This change adds a single one-click affordance: - Hub: new POST /api/sessions/:id/reopen route on the existing sessions router. The route delegates to a new engine method `reopenSession` that: - is idempotent (active session -> 200 with `resumed:false`), - validates Cursor sessions still have a `cursorSessionId` once they have any messages (otherwise we cannot resume the agent thread), - clears `lifecycleState='archived'`, `archivedBy`, `archiveReason` via a versioned metadata update, and stamps `lifecycleStateSince`, - defaults `cursorSessionProtocol='stream-json'` for pre-#799 Cursor sessions (sessions that have a `cursorSessionId` but no protocol set), so routing still reaches the legacy launcher; ACP sessions keep their explicit protocol, - forwards to the same `resumeSession` path the existing /resume route uses, including the `canFreshSpawnNeverStartedSession` fallback. 422 is returned with `{ missing: [...] }` when the agent metadata needed to resume is gone; other engine errors map to 404/409/503/500 with the existing shape (mirrors /resume). - Web: a "Reopen" entry in the SessionActionMenu that appears next to "Delete" on inactive sessions only. Wired into both the SessionList rows and the SessionHeader more-menu, with a small dismissable error dialog for the 422 missing-metadata case. - Tests: route-level coverage for the four response shapes (200 reopen, 200 idempotent, 404, 422) plus 409/503 error mappings; sessionCache tests for the archive-metadata clear (including the legacy Cursor protocol default); React component test for the menu item rendering on inactive vs active sessions; mutation hook test for the api wiring and the ApiError surface needed by the UI. Closes #819 Co-authored-by: Cursor <cursoragent@cursor.com> * fix(reopen): address codex review findings on fork PR #33 Four P2 findings from the cold-review bot, three fixed and one explained: 1. Mutation now returns the reopen response so the UI can route to a possibly different sessionId. SyncEngine.resumeSession may merge the row into a freshly-spawned session id (matching the send-message resume flow); the chat view now navigates there, the row list calls onSelect on the new id. 2. reopenSession on the client now goes through `request()` instead of a hand-rolled fetch, so 401 + onUnauthorized refresh works the same as every other session action. `request()` now throws `ApiError` (with status/code/body) on non-401 errors - backward compatible because ApiError extends Error. 3. (Reply only) Pre-#799 Cursor protocol propagates correctly without the extra plumbing the bot suggested: `clearSessionArchiveMetadata` writes `cursorSessionProtocol='stream-json'` to the DB; the CLI's `bootstrapExistingSession` preserves it via `pickExistingSessionMetadata`; if it's still absent at the launcher, `isLegacyCursorSession` defaults to stream-json whenever `cursorSessionId` is present. 4. Archive metadata is now restored when resume fails. `reopenSession` captures a snapshot of `lifecycleState`/`archivedBy`/`archiveReason`/ `lifecycleStateSince` before the clear; if `resumeSession` returns an error (no machine online, spawn timeout, etc.), the snapshot is put back via the new `SessionCache.restoreSessionArchiveMetadata`. Engine test covers both the rollback and the no-rollback-on-success cases. Error rendering helper moved to `web/src/lib/reopenError.ts` so the chat header and the session row share one implementation, and gained a unit test. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(web): preserve engine error codes in ApiError.code on /reopen `/sessions/:id/reopen` returns `{ error, code }` where `code` is the stable taxonomy (`no_machine_online`, `resume_unavailable`, etc.) and `error` is the human-readable message. The generic `request()` error path was reading only `parsed.error`, so `ApiError.code` ended up being a message like "No machine online" rather than `no_machine_online`, breaking taxonomy-based branching in web callers. `parseErrorCode` now prefers `parsed.code` and falls back to `parsed.error` for legacy routes that only set `error`. Added api/client.test.ts covering the three response shapes /reopen actually emits (503 with code, 500 without code, 422 with missing[]). Addresses upstream codex-action review on tiann/hapi#826. Co-authored-by: Cursor <cursoragent@cursor.com> * fix(reopen): restore archive metadata exactly on rollback (drop fresh lifecycleStateSince) For an archived session that predates `lifecycleStateSince` (the field is absent from its metadata), `clearSessionArchiveMetadata` stamps a fresh timestamp. If `resumeSession` then fails, the rollback was leaving that fresh timestamp in place, making the rolled-back row look like it was just archived rather than preserving the original lifecycle age. `restoreSessionArchiveMetadata` now does an EXACT restore: when a snapshot field is undefined the corresponding key on the metadata is deleted, not left alone. Applies symmetrically to lifecycleState / archivedBy / archiveReason / lifecycleStateSince. Test updated to assert the deletion of the fresh timestamp. Addresses upstream codex-action review on tiann/hapi#826. Co-authored-by: Cursor <cursoragent@cursor.com> --------- Co-authored-by: Cursor <cursoragent@cursor.com>
2026-08-05 06:24:37 +00:00 · 2026-06-08 13:29:31 +08:00
parent cd99cfbc25
commit cb72703649
19 changed files with 1237 additions and 15 deletions
@@ -0,0 +1,82 @@
+import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
+import { ApiClient, ApiError } from './client'
+
+describe('ApiClient error mapping', () => {
+    let originalFetch: typeof globalThis.fetch
+    let fetchMock: ReturnType<typeof vi.fn>
+
+    beforeEach(() => {
+        originalFetch = globalThis.fetch
+        fetchMock = vi.fn()
+        globalThis.fetch = fetchMock as unknown as typeof globalThis.fetch
+    })
+
+    afterEach(() => {
+        globalThis.fetch = originalFetch
+    })
+
+    it('prefers the stable `code` field over the human-readable `error` message in ApiError.code', async () => {
+        // Match the shape /sessions/:id/reopen actually returns on a 503.
+        fetchMock.mockResolvedValueOnce(
+            new Response(
+                JSON.stringify({ error: 'No machine online', code: 'no_machine_online' }),
+                { status: 503, statusText: 'Service Unavailable' }
+            )
+        )
+
+        const api = new ApiClient('test-token')
+        try {
+            await api.reopenSession('session-X')
+            expect.unreachable('expected reopenSession to throw')
+        } catch (error) {
+            expect(error).toBeInstanceOf(ApiError)
+            const apiError = error as ApiError
+            expect(apiError.status).toBe(503)
+            // The stable taxonomy must survive into ApiError.code so callers can
+            // branch on `no_machine_online` rather than parsing the message text.
+            expect(apiError.code).toBe('no_machine_online')
+            expect(apiError.body).toContain('no_machine_online')
+        }
+    })
+
+    it('falls back to `parsed.error` when `code` is absent (legacy route shape)', async () => {
+        fetchMock.mockResolvedValueOnce(
+            new Response(
+                JSON.stringify({ error: 'something broke' }),
+                { status: 500, statusText: 'Internal Server Error' }
+            )
+        )
+
+        const api = new ApiClient('test-token')
+        try {
+            await api.reopenSession('session-Y')
+            expect.unreachable('expected reopenSession to throw')
+        } catch (error) {
+            expect(error).toBeInstanceOf(ApiError)
+            expect((error as ApiError).code).toBe('something broke')
+        }
+    })
+
+    it('passes the 422 missing-metadata body through unchanged so the UI can show the missing fields', async () => {
+        fetchMock.mockResolvedValueOnce(
+            new Response(
+                JSON.stringify({
+                    error: 'Cursor session id is missing from metadata; reopen requires the original cursor chat id',
+                    missing: ['cursorSessionId']
+                }),
+                { status: 422, statusText: 'Unprocessable Entity' }
+            )
+        )
+
+        const api = new ApiClient('test-token')
+        try {
+            await api.reopenSession('session-Z')
+            expect.unreachable('expected reopenSession to throw')
+        } catch (error) {
+            expect(error).toBeInstanceOf(ApiError)
+            const apiError = error as ApiError
+            expect(apiError.status).toBe(422)
+            expect(apiError.body).toContain('cursorSessionId')
+        }
+    })
+})