diff --git a/src/session.ts b/src/session.ts index 21d5ac89..16720e59 100644 --- a/src/session.ts +++ b/src/session.ts @@ -1499,6 +1499,19 @@ export class Session extends EventEmitter { this._pinnedAt = pinned ? Date.now() : null; } + /** + * Restore a pin from a persisted record, keeping the moment it was pinned. + * + * `setPinned()` stamps `pinnedAt` with now, which is right for a user pinning a + * session and wrong for a restore: the session-manager orders its pinned group + * by that stamp, so a restored session would jump to the front of a list it had + * been sitting further down. + */ + restorePin(pinned: boolean, pinnedAt?: number): void { + this._pinned = pinned; + this._pinnedAt = pinned ? (pinnedAt ?? Date.now()) : null; + } + get flickerFilterEnabled(): boolean { return this._flickerFilterEnabled; } diff --git a/src/web/ports/session-port.ts b/src/web/ports/session-port.ts index 83918763..85add1d8 100644 --- a/src/web/ports/session-port.ts +++ b/src/web/ports/session-port.ts @@ -14,15 +14,27 @@ export interface SessionPort { persistSessionState(session: Session): void; persistSessionStateNow(session: Session): void; /** - * Re-apply the persisted state a freshly CONSTRUCTED session does not carry: - * the pin, token and cost totals, auto-compact, auto-clear, auto-resume, nice - * priority, the flicker filter and the custom-model selection. + * Re-apply the persisted state a freshly CONSTRUCTED session does not carry. * * A `Session` built from a record holds only what its constructor takes, so * persisting it would otherwise REPLACE the fuller record with the reduced one. - * Call this before the first persist, and before `startInteractive()`, because - * the custom-model selection has to reach the pane's environment. + * Two phases: `before-spawn` shapes the pane (the custom-model environment and + * the nice priority) and must precede `startInteractive()`; `after-spawn` is + * the session's own history (the pin, token and cost totals, auto-compact, + * auto-clear, auto-resume, colour, image watcher, flicker filter) and must NOT + * land on a session whose pane failed to start. */ - reapplyPersistedSessionState(session: Session, saved: SessionState): Promise; + reapplyPersistedSessionState( + session: Session, + saved: SessionState, + phase: 'before-spawn' | 'after-spawn' + ): Promise; + /** + * Undo a session that was registered but never got a working pane: the map + * entry, its tab-layout slot, and any pane the launch created before throwing. + * Unlike {@link cleanupSession} it leaves the persisted record, the lifetime + * token totals, the Ralph state and the workspace's own files untouched. + */ + discardPartiallyBuiltSession(sessionId: string): Promise; getSessionStateWithRespawn(session: Session): unknown; } diff --git a/src/web/public/mobile.css b/src/web/public/mobile.css index d324a94f..f364c701 100644 --- a/src/web/public/mobile.css +++ b/src/web/public/mobile.css @@ -3260,7 +3260,11 @@ html:is([data-skin="paper-gray"], [data-skin="solarized-light"], [data-skin="cat display: none; } + /* A flex item will not shrink below its content width at the default + `min-width: auto`, so without this the nowrap text pushes the buttons off a + 360px viewport and the ellipsis never engages. */ .reboot-restore-banner-text { + min-width: 0; overflow: hidden; text-overflow: ellipsis; } @@ -3273,9 +3277,7 @@ html:is([data-skin="paper-gray"], [data-skin="solarized-light"], [data-skin="cat .reboot-restore-banner-accept { margin-left: auto; } -} -@media (max-width: 599px) { .offline-banner { padding: 0.4rem 0.5rem; padding-left: calc(0.5rem + var(--safe-area-left)); diff --git a/src/web/public/reboot-restore-ui.js b/src/web/public/reboot-restore-ui.js index aec1de81..1cdc8da8 100644 --- a/src/web/public/reboot-restore-ui.js +++ b/src/web/public/reboot-restore-ui.js @@ -23,7 +23,7 @@ * * @mixin Extends CodemanApp.prototype via Object.assign * @dependency app.js (CodemanApp class, showToast) - * @dependency api-client.js at runtime (this._apiJson / this._apiPost) + * @dependency api-client.js at runtime (this._api / this._apiJson) * @loadorder 11.7 of 17, after approvals-ui.js */ @@ -89,9 +89,15 @@ Object.assign(CodemanApp.prototype, { async restoreRebootSessions() { const button = this.$('rebootRestoreBannerAccept'); if (button) button.disabled = true; - // _apiJson unwraps the { success, data } envelope every /api response carries; - // reading the outer object would report every count as zero. - const body = await this._apiJson('/api/reboot-restore/restore', { method: 'POST', body: {} }); + const res = await this._api('/api/reboot-restore/restore', { method: 'POST', body: {} }); + if (res && res.status === 409) { + if (button) button.disabled = false; + this.showToast?.('A restore is already running', 'info'); + return; + } + // The uniform envelope wraps every /api payload; reading the outer object + // would report every count as zero. + const body = res && res.ok ? (await res.json().catch(() => null))?.data : null; if (!body) { if (button) button.disabled = false; this.showToast?.('Could not restore the sessions', 'error'); @@ -99,8 +105,12 @@ Object.assign(CodemanApp.prototype, { } const restored = body.restored?.length ?? 0; const skipped = body.skipped?.length ?? 0; - this._rebootRestoreSessions = []; - this.renderRebootRestoreBanner(); + // Re-read rather than clearing: the server puts back anything it could not + // build for a reason that may pass, such as a session limit or an agent that + // would not start, and blanking the banner here would put those entries out + // of reach until a reload. + await this.refreshRebootRestoreBanner(); + if (button) button.disabled = false; if (restored > 0) { const noun = restored === 1 ? 'conversation' : 'conversations'; this.showToast?.(`Restored ${restored} ${noun}. Terminal history did not survive the reboot.`, 'success'); diff --git a/src/web/reboot-restore-registry.ts b/src/web/reboot-restore-registry.ts index 789217eb..28b4a1d7 100644 --- a/src/web/reboot-restore-registry.ts +++ b/src/web/reboot-restore-registry.ts @@ -47,6 +47,13 @@ export class RebootRestoreRegistry { private entries = new Map(); /** When the boot pass built the plan, in ms since the epoch. */ private builtAt = 0; + /** + * Bumped by anything that invalidates entries a restore is already holding. + * A Dismiss arriving mid-restore must win: without this the route's `finally` + * would put its unspent entries back and resurrect the offer the user just + * cleared, with a fresh 24-hour life. + */ + private generation = 0; /** * Owners with a restore in flight, between its take and its last pane. * Keyed by owner so one user's restore does not turn another user's click into @@ -59,6 +66,12 @@ export class RebootRestoreRegistry { set(entries: readonly RebootRestoreEntry[]): void { this.entries = new Map(entries.map((entry) => [entry.sessionId, entry])); this.builtAt = entries.length > 0 ? Date.now() : 0; + this.generation += 1; + } + + /** The current generation, for a caller that will later return entries. */ + currentGeneration(): number { + return this.generation; } /** @@ -104,7 +117,10 @@ export class RebootRestoreRegistry { * hand is NOT put back, because that one cannot stop being true, and an entry * the banner keeps re-offering forever is noise only Dismiss can clear. */ - restore(entries: readonly RebootRestoreEntry[]): void { + restore(entries: readonly RebootRestoreEntry[], generation?: number): void { + // A dismiss (or a fresh boot plan) since the caller took these entries means + // they are no longer wanted back. + if (generation !== undefined && generation !== this.generation) return; for (const entry of entries) this.entries.set(entry.sessionId, entry); if (entries.length > 0 && this.builtAt === 0) this.builtAt = Date.now(); } @@ -114,6 +130,8 @@ export class RebootRestoreRegistry { const removable = [...this.entries.values()].filter((entry) => canAccess(entry.owner)); for (const entry of removable) this.entries.delete(entry.sessionId); if (this.entries.size === 0) this.builtAt = 0; + // Any restore currently in flight must not put its entries back afterwards. + this.generation += 1; return removable.length; } @@ -137,6 +155,7 @@ export class RebootRestoreRegistry { this.entries.clear(); this.builtAt = 0; this.spending.clear(); + this.generation += 1; } private dropIfExpired(): void { diff --git a/src/web/routes/reboot-restore-routes.ts b/src/web/routes/reboot-restore-routes.ts index a03c74af..636f2cd5 100644 --- a/src/web/routes/reboot-restore-routes.ts +++ b/src/web/routes/reboot-restore-routes.ts @@ -32,7 +32,7 @@ import { getAuthUser, canAccessOwned, ownerFor, - isWorkingDirAllowed, + isWorkingDirAllowedForUsername, sessionCapacityMessage, } from '../route-helpers.js'; import { rebootRestoreRegistry } from '../reboot-restore-registry.js'; @@ -83,7 +83,6 @@ export function registerRebootRestoreRoutes(app: FastifyInstance, ctx: RebootRes app.post('/api/reboot-restore/restore', async (req, reply) => { const body = parseBody(RebootRestoreRequestSchema, req.body, 'Invalid reboot restore request'); - const user = getAuthUser(req); const canAccess = accessorFor(req); const owner = ownerFor(req); @@ -93,6 +92,7 @@ export function registerRebootRestoreRoutes(app: FastifyInstance, ctx: RebootRes if (!rebootRestoreRegistry.beginSpending(owner)) { return reply.code(409).send(createErrorResponse(ApiErrorCode.CONFLICT, 'A reboot restore is already running')); } + const generation = rebootRestoreRegistry.currentGeneration(); const taken = rebootRestoreRegistry.take(canAccess, body.sessionIds); // Entries nothing built a pane for, returned to the plan on every exit path // including a throw. Without this a failure between here and the loop would @@ -136,10 +136,15 @@ export function registerRebootRestoreRoutes(app: FastifyInstance, ctx: RebootRes // Multi-user workspace separation: the create route confines a non-admin's // workingDir to their own case space, and a grant can be withdrawn between // the session's creation and this restore, so the confinement is re-run - // rather than inherited from the record. - if (!isWorkingDirAllowed(user, entry.workingDir)) { + // rather than inherited from the record. Keyed on the OWNER, not on the + // caller: an admin spending another user's entry must be held to that + // user's confinement, and `isWorkingDirAllowed` would wave an admin + // through. The same reason the two grant re-checks below read + // `saved.owner`. + if (!(await isWorkingDirAllowedForUsername(entry.owner, entry.workingDir))) { + // Left on offer: a withdrawn grant can be restored, unlike an already-open + // conversation, so this is not the permanent kind of refusal. failures.push({ sessionId: entry.sessionId, reason: 'workspace-forbidden' }); - unspent.delete(entry); continue; } try { @@ -180,12 +185,16 @@ export function registerRebootRestoreRoutes(app: FastifyInstance, ctx: RebootRes await ctx.addSession(session); await ctx.setupSessionListeners(session); - // Before the pane spawns: the custom-model selection reaches it through - // the environment. Before the first persist: a constructed session holds - // none of this, so persisting it first would replace the fuller record - // with the reduced one and drop the pin that keeps it from being pruned. - await ctx.reapplyPersistedSessionState(session, saved); + // Shapes the pane, so it has to land before the CLI process starts. + await ctx.reapplyPersistedSessionState(session, saved, 'before-spawn'); await session.startInteractive(); + // The session's own history, applied only once the pane exists: on a + // failed start these totals would belong to a session that never ran. + // Both halves precede the first persist, because a constructed session + // carries none of this and `toState()` is written wholesale, so + // persisting first would replace the fuller record with the reduced one + // and drop the pin that keeps it from being pruned. + await ctx.reapplyPersistedSessionState(session, saved, 'after-spawn'); ctx.persistSessionState(session); // A session without its workspace hooks goes silently blind: no stop or @@ -211,10 +220,15 @@ export function registerRebootRestoreRoutes(app: FastifyInstance, ctx: RebootRes // listeners, and the commonest cause is a CLI binary that is not on the // PATH of a freshly booted machine. console.error(`[reboot-restore] failed to rebuild ${entry.sessionId}:`, err); + // Not cleanupSession(): that is the user-initiated delete, and it would + // count this session's historical tokens into the lifetime totals, demote + // a pinned record to `stopped` (which this pass reads as an intentional + // kill, making the session permanently unrestorable) and delete the + // workspace's `.claude-images`. This undoes only the construction. await ctx - .cleanupSession(entry.sessionId, true, 'reboot restore failed to start the session') - .catch((cleanupErr: unknown) => - console.error(`[reboot-restore] cleanup after a failed rebuild failed: ${getErrorMessage(cleanupErr)}`) + .discardPartiallyBuiltSession(entry.sessionId) + .catch((discardErr: unknown) => + console.error(`[reboot-restore] discarding a failed rebuild failed: ${getErrorMessage(discardErr)}`) ); failures.push({ sessionId: entry.sessionId, reason: 'rebuild-failed' }); // Left on offer: the user can put the binary back and click again. @@ -234,7 +248,8 @@ export function registerRebootRestoreRoutes(app: FastifyInstance, ctx: RebootRes } finally { // Anything that never became a pane goes back on offer, including after a // throw, so a transient failure costs a retry rather than the whole plan. - rebootRestoreRegistry.restore([...unspent]); + // Passing the generation makes a Dismiss that landed mid-restore win. + rebootRestoreRegistry.restore([...unspent], generation); rebootRestoreRegistry.endSpending(owner); } }); diff --git a/src/web/server.ts b/src/web/server.ts index a7339328..474d8cd2 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -672,6 +672,7 @@ export class WebServer extends EventEmitter { persistSessionState: this.persistSessionState.bind(this), persistSessionStateNow: this._persistSessionStateNow.bind(this), reapplyPersistedSessionState: this.reapplyPersistedSessionState.bind(this), + discardPartiallyBuiltSession: this.discardPartiallyBuiltSession.bind(this), getSessionStateWithRespawn: this.getSessionStateWithRespawn.bind(this), // EventPort broadcast: this.broadcast.bind(this), @@ -2919,19 +2920,42 @@ export class WebServer extends EventEmitter { * because `cleanupSessionsByIds()` keeps a record only while it is pinned, so * dropping the pin hands the record to the next stale sweep. * - * Respawn and Ralph are deliberately NOT re-armed here: a machine that just - * came up is the worst moment to turn an autonomous run loose, and the user - * re-arms what they want. + * Split in two phases because the two halves have opposite timing needs: + * + * - `before-spawn` shapes the pane itself, so it has to land before the CLI + * process starts. The custom-model selection is an environment injection and + * the nice priority is applied to the spawn. + * - `after-spawn` is the session's own accumulated history. It must NOT land + * on a session whose pane failed to start: the totals would then belong to a + * session that never ran, and any later cleanup would add them to the + * lifetime figures a second time. + * + * Respawn and Ralph are deliberately NOT re-armed: a machine that just came up + * is the worst moment to turn an autonomous run loose, and the user re-arms + * what they want. Ralph's loop CONFIGURATION does not survive either, because + * `toState()` reads `ralphEnabled` and the completion phrase off a live + * tracker, and there is no way to hold them without arming the loop. */ - async reapplyPersistedSessionState(session: Session, saved: SessionState): Promise { - // The custom-model env has to be rebuilt from the endpoint store: the persist - // deliberately keeps the injected VALUES out of state.json, so only the - // bookkeeping survives a restart and the values are re-derived here. - const savedCustomModel = (saved as { __customModel?: CustomModelBookkeeping }).__customModel; - if (savedCustomModel) { - session.setCustomModel(savedCustomModel, await this._rebuildCustomModelEnv(session, savedCustomModel)); + async reapplyPersistedSessionState( + session: Session, + saved: SessionState, + phase: 'before-spawn' | 'after-spawn' + ): Promise { + if (phase === 'before-spawn') { + // The custom-model env has to be rebuilt from the endpoint store: the persist + // deliberately keeps the injected VALUES out of state.json, so only the + // bookkeeping survives a restart and the values are re-derived here. + const savedCustomModel = (saved as { __customModel?: CustomModelBookkeeping }).__customModel; + if (savedCustomModel) { + session.setCustomModel(savedCustomModel, await this._rebuildCustomModelEnv(session, savedCustomModel)); + } + if (saved.niceEnabled !== undefined || saved.niceValue !== undefined) { + session.setNice({ enabled: saved.niceEnabled, niceValue: saved.niceValue }); + } + return; } - if (saved.pinned) session.setPinned(true); + + if (saved.pinned) session.restorePin(true, saved.pinnedAt); if (saved.autoCompactEnabled !== undefined || saved.autoCompactThreshold !== undefined) { session.setAutoCompact(saved.autoCompactEnabled ?? false, saved.autoCompactThreshold, saved.autoCompactPrompt); } @@ -2949,12 +2973,51 @@ export class WebServer extends EventEmitter { output: saved.outputTokens ?? 0, }); } - if (saved.niceEnabled !== undefined || saved.niceValue !== undefined) { - session.setNice({ enabled: saved.niceEnabled, niceValue: saved.niceValue }); - } + if (saved.color) session.setColor(saved.color); + if (saved.imageWatcherEnabled !== undefined) session.imageWatcherEnabled = saved.imageWatcherEnabled; if (saved.flickerFilterEnabled !== undefined) session.flickerFilterEnabled = saved.flickerFilterEnabled; } + /** + * Undo a session that was registered but never got a working pane. + * + * Deliberately NOT `cleanupSession()`, which is the user-initiated delete: that + * path adds the session's token totals to the lifetime figures, demotes a + * pinned record to `stopped` (the durable marker of an intentional kill, which + * would make the session permanently ineligible for a reboot restore), drops + * the persisted Ralph state, and recursively removes `.claude-images` from the + * WORKING DIRECTORY, which belongs to the workspace rather than to this session + * and may hold another live session's pasted images. + * + * This undoes only what the failed construction did: the map entry, the tab + * layout slot `registerSessionWithLayout()` took, and any pane the CLI launch + * managed to create before it threw. The persisted record is left exactly as it + * was, so the session stays restorable on the next attempt. + */ + async discardPartiallyBuiltSession(sessionId: string): Promise { + const session = this.sessions.get(sessionId); + if (!session) return; + this.sessions.delete(sessionId); + this.sse.cleanupSessionBatches(sessionId); + this.persistDeb.cancelKey(sessionId); + try { + session.removeAllListeners(); + await session.stop?.(); + } catch (err) { + console.warn(`[Server] stopping a partially built session failed: ${getErrorMessage(err)}`); + } + try { + await this.mux.killSession(sessionId); + } catch { + // The pane may never have been created; nothing to kill is the normal case. + } + try { + await this.tabLayouts.sessionsRemoved([{ id: sessionId, owner: session.owner }]); + } catch (err) { + console.warn(`[Server] releasing the tab layout slot failed: ${getErrorMessage(err)}`); + } + } + private async restoreMuxSessions(): Promise { try { // Reconcile mux sessions to find which ones are still alive (also discovers unknown ones) diff --git a/test/mocks/mock-route-context.ts b/test/mocks/mock-route-context.ts index 401538be..9cb26d6a 100644 --- a/test/mocks/mock-route-context.ts +++ b/test/mocks/mock-route-context.ts @@ -62,6 +62,9 @@ export function createMockRouteContext(options?: { persistSessionState: vi.fn(), persistSessionStateNow: vi.fn(), reapplyPersistedSessionState: vi.fn(async () => {}), + discardPartiallyBuiltSession: vi.fn(async (id: string) => { + sessions.delete(id); + }), getSessionStateWithRespawn: vi.fn((s: MockSession) => s.toState()), // -- EventPort -- diff --git a/test/routes/reboot-restore-rebuild-failure.test.ts b/test/routes/reboot-restore-rebuild-failure.test.ts index f8f58f58..f1a35417 100644 --- a/test/routes/reboot-restore-rebuild-failure.test.ts +++ b/test/routes/reboot-restore-rebuild-failure.test.ts @@ -18,6 +18,8 @@ import fastifyCookie from '@fastify/cookie'; /** Set per test: whether the mocked `startInteractive()` rejects. */ let startShouldThrow = false; +/** Ordering log, so a test can assert what ran before the pane spawned. */ +const callOrder: string[] = []; vi.mock('../../src/session.js', () => ({ Session: class { @@ -35,6 +37,7 @@ vi.mock('../../src/session.js', () => ({ this.owner = config.owner; } async startInteractive() { + callOrder.push('startInteractive'); if (startShouldThrow) throw new Error('spawn claude ENOENT'); } /** The mock route context projects a session through this on broadcast. */ @@ -101,6 +104,7 @@ async function createHarness(ctx: ReturnType): Pr beforeEach(() => { startShouldThrow = false; + callOrder.length = 0; }); afterEach(() => { @@ -131,7 +135,12 @@ describe('a rebuild that fails after the session is registered', () => { await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} }); // The session reached ctx.sessions via addSession; the route has to take it // back out, or the board shows a tab whose pane never existed. - expect(ctx.cleanupSession).toHaveBeenCalledWith('a', true, expect.any(String)); + expect(ctx.discardPartiallyBuiltSession).toHaveBeenCalledWith('a'); + expect(ctx.sessions.has('a')).toBe(false); + // NOT the user-initiated delete: that would bank this session's historical + // tokens into the lifetime totals, demote a pinned record to `stopped`, and + // delete the workspace's .claude-images. + expect(ctx.cleanupSession).not.toHaveBeenCalled(); await app.close(); }); @@ -166,6 +175,23 @@ describe('a rebuild that succeeds', () => { await app.close(); }); + it('shapes the pane before it spawns, and restores the history after', async () => { + rebootRestoreRegistry.set([offerEntry('a')]); + const ctx = createMockRouteContext({ workspaceHooksEnabled: false }); + (ctx.reapplyPersistedSessionState as ReturnType).mockImplementation( + async (_s: unknown, _saved: unknown, phase: string) => { + callOrder.push(`reapply:${phase}`); + } + ); + const app = await createHarness(ctx); + + await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} }); + // The custom-model environment has to reach the process; the token totals + // must not land on a session whose pane never started. + expect(callOrder).toEqual(['reapply:before-spawn', 'startInteractive', 'reapply:after-spawn']); + await app.close(); + }); + it('tells every other board about the rebuilt session', async () => { rebootRestoreRegistry.set([offerEntry('a')]); const ctx = createMockRouteContext({ workspaceHooksEnabled: false }); @@ -189,10 +215,30 @@ describe('a rebuild that succeeds', () => { }); describe('the session caps', () => { - it('stops restoring at the global cap and leaves the rest on offer', async () => { + it('counts the sessions it is itself creating, not just the ones it started with', async () => { + rebootRestoreRegistry.set([offerEntry('a'), offerEntry('b')]); + const ctx = createMockRouteContext({ workspaceHooksEnabled: false }); + // One seat short of the documented maximum of 50, counting the session the + // mock context seeds. A check that ran once before the loop would restore + // BOTH entries; only a per-iteration check refuses the second. + for (let i = 0; i < 48; i += 1) { + ctx.sessions.set(`filler-${i}`, { id: `filler-${i}`, owner: undefined } as never); + } + const app = await createHarness(ctx); + + const res = (await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} })).json().data; + expect(res.restored.map((s: { id: string }) => s.id)).toEqual(['a']); + expect(res.skipped).toEqual([{ sessionId: 'b', reason: 'capacity-reached' }]); + + // Refused rather than lost: closing a session and clicking again works. + const left = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data; + expect(left.sessions.map((s: { id: string }) => s.id)).toEqual(['b']); + await app.close(); + }); + + it('refuses every entry when the board is already at the cap', async () => { rebootRestoreRegistry.set([offerEntry('a'), offerEntry('b')]); const ctx = createMockRouteContext({ workspaceHooksEnabled: false }); - // Fill the board to the documented maximum of 50 concurrent sessions. for (let i = 0; i < 50; i += 1) { ctx.sessions.set(`filler-${i}`, { id: `filler-${i}`, owner: undefined } as never); } @@ -201,10 +247,53 @@ describe('the session caps', () => { const res = (await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} })).json().data; expect(res.restored).toEqual([]); expect(res.skipped.map((s: { reason: string }) => s.reason)).toEqual(['capacity-reached', 'capacity-reached']); + await app.close(); + }); +}); - // Refused rather than lost: closing a session and clicking again works. +describe('a failure before any entry is considered', () => { + it('returns the whole plan rather than spending it', async () => { + rebootRestoreRegistry.set([offerEntry('a'), offerEntry('b')]); + const ctx = createMockRouteContext({ workspaceHooksEnabled: false }); + (ctx.getWorkspaceHooksEnabled as ReturnType).mockRejectedValue(new Error('settings unreadable')); + const app = await createHarness(ctx); + + const res = await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} }); + expect(res.statusCode).toBeGreaterThanOrEqual(500); + + // The plan cannot be rebuilt once boot has pruned the records, so a throw + // anywhere in the route has to hand the entries back. const left = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data; expect(left.sessions.map((s: { id: string }) => s.id).sort()).toEqual(['a', 'b']); await app.close(); }); + + it('releases the single flight, so the next click is not refused', async () => { + rebootRestoreRegistry.set([offerEntry('a')]); + const ctx = createMockRouteContext({ workspaceHooksEnabled: false }); + (ctx.getWorkspaceHooksEnabled as ReturnType).mockRejectedValue(new Error('settings unreadable')); + const app = await createHarness(ctx); + + await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} }); + expect(rebootRestoreRegistry.beginSpending(undefined)).toBe(true); + rebootRestoreRegistry.endSpending(undefined); + await app.close(); + }); +}); + +describe('a dismiss that lands while a restore is running', () => { + it('wins, rather than being undone when the restore hands its entries back', async () => { + const entries = [offerEntry('a')]; + rebootRestoreRegistry.set(entries); + const generation = rebootRestoreRegistry.currentGeneration(); + const taken = rebootRestoreRegistry.take(() => true); + expect(taken).toHaveLength(1); + + // The user clears the banner while the restore is still working. + rebootRestoreRegistry.clear(() => true); + // The restore finishes and tries to put its unspent entry back. + rebootRestoreRegistry.restore(taken, generation); + + expect(rebootRestoreRegistry.list(() => true)).toEqual([]); + }); });