diff --git a/docs/api-reference.md b/docs/api-reference.md index dcd0d631..85349ee3 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -66,17 +66,17 @@ The single source of truth is `ErrorStatus` / `httpStatusForErrorCode()` in `src/types/api.ts`. Clients should branch on `errorCode` (stable) and may rely on the HTTP status. -| `errorCode` | HTTP | Meaning | -|-------------|------|---------| -| `INVALID_INPUT` | 400 | Malformed request / failed validation | -| `UNAUTHORIZED` | 401 | Authentication required or failed | -| `NOT_FOUND` | 404 | Resource does not exist | -| `SESSION_BUSY` | 409 | Session is busy | -| `CONFLICT` | 409 | Conflicts with current state (e.g. already running) | -| `ALREADY_EXISTS` | 409 | Resource already exists | -| `OPERATION_FAILED` | 422 | Well-formed but could not be completed | -| `RATE_LIMITED` | 429 | Too many requests | -| `INTERNAL_ERROR` | 500 | Unexpected server error | +| `errorCode` | HTTP | Meaning | +| ------------------ | ---- | --------------------------------------------------- | +| `INVALID_INPUT` | 400 | Malformed request / failed validation | +| `UNAUTHORIZED` | 401 | Authentication required or failed | +| `NOT_FOUND` | 404 | Resource does not exist | +| `SESSION_BUSY` | 409 | Session is busy | +| `CONFLICT` | 409 | Conflicts with current state (e.g. already running) | +| `ALREADY_EXISTS` | 409 | Resource already exists | +| `OPERATION_FAILED` | 422 | Well-formed but could not be completed | +| `RATE_LIMITED` | 429 | Too many requests | +| `INTERNAL_ERROR` | 500 | Unexpected server error | Adding a new error code is non-breaking; removing or renaming one is a major change. @@ -87,10 +87,10 @@ exist because SSE is Codeman's only other "tell me when" channel, and an agent driving the API from a shell tool cannot practically hold a stream and parse events inline. -| Call | Blocks until | -|------|--------------| -| `GET /api/v1/sessions/:id/wait` | one of a set of lifecycle signals fires | -| `GET /api/v1/sessions/:id/wait-output` | a literal string appears in the session's output | +| Call | Blocks until | +| --------------------------------------------- | -------------------------------------------------- | +| `GET /api/v1/sessions/:id/wait` | one of a set of lifecycle signals fires | +| `GET /api/v1/sessions/:id/wait-output` | a literal string appears in the session's output | | `POST /api/v1/sessions/:id/input` with `wait` | the input is delivered **and then** a signal fires | `POST .../input` with `wait` is not the same as a `POST` followed by a separate @@ -140,13 +140,13 @@ contract is a **marker unique to each call** (`MARK="DONE_$RANDOM"`, send ### Signals -| Signal | Source | Actually fires for | -|--------|--------|--------------------| -| `idle` | the session's own `idle` event | `claude`: yes, on ❯-prompt detection after activity. `shell`: **once only**, ~500 ms after start, and never again. External CLIs: not guaranteed (they render their own TUIs and readiness is output stabilization) | -| `working` | the session's own `working` event | `claude` only in practice (spinner and work-keyword detection are Claude output formats) | -| `stop` | the Claude Code `stop` hook, the definitive end-of-turn signal | `claude` only | -| `blocked` | a `permission_prompt` or `elicitation_dialog` hook | `claude` only, and rarer than it looks: see below | -| `exit` | no process is behind the session | every mode | +| Signal | Source | Actually fires for | +| --------- | -------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `idle` | the session's own `idle` event | `claude`: yes, on ❯-prompt detection after activity. `shell`: **once only**, ~500 ms after start, and never again. External CLIs: not guaranteed (they render their own TUIs and readiness is output stabilization) | +| `working` | the session's own `working` event | `claude` only in practice (spinner and work-keyword detection are Claude output formats) | +| `stop` | the Claude Code `stop` hook, the definitive end-of-turn signal | `claude` only | +| `blocked` | a `permission_prompt` or `elicitation_dialog` hook | `claude` only, and rarer than it looks: see below | +| `exit` | no process is behind the session | every mode | `stop` is the signal to orchestrate on where it exists; `idle` is a heuristic fallback that can flap mid-turn when a spinner pauses. The default set when `until` @@ -156,12 +156,12 @@ can no longer happen). On a `claude` worker, prefer an explicit `until=stop,exit once the session is up: the default set's `idle` also resolves on a spinner pause, and on a fresh session the **startup** `idle` (emitted when the CLI first comes up) can land inside your first wait window and report a turn that never ran. Measured: -a session parked on the trust dialog emits no *further* `idle`, so it is the +a session parked on the trust dialog emits no _further_ `idle`, so it is the startup transition, not the dialog, that produces the false success below. ⚠️ **`exit` means "nothing is running", which includes "not started yet".** The server answers from `pid === null` plus a mux-layer pane-death probe, and that -covers a session that exited — including a worker that died *inside* its tmux pane +covers a session that exited — including a worker that died _inside_ its tmux pane while the local attach client (and therefore `pid`) lives on — one that was detached, and one that was **created but never started**. So the first wait after `POST /api/v1/sessions` returns `{"signal":"exit","immediate":true}` in @@ -184,7 +184,7 @@ blocked, and polling `blocked` alone will sit at its timeout. ⚠️ **On a `shell` session, only `exit` and marker-matching are dependable.** A shell session emits its one `idle` at startup and then stays `status: "idle"` forever, -whatever the pane is doing, so it never emits a *transition*. Since send-and-wait +whatever the pane is doing, so it never emits a _transition_. Since send-and-wait requires a transition (and so does `fresh=1`), both can only time out there: a documented default `wait` on a shell worker running `sleep 4` times out at the full 25 s. Synchronize hook-less sessions with `wait-output` and a unique marker @@ -218,11 +218,11 @@ with `from=buffer` keeps matching long after the dialog is gone. A worked versio ### `GET /api/v1/sessions/:id/wait` -| Param | Type | Default | Notes | -|-------|------|---------|-------| -| `until` | comma-separated list of `idle,working,stop,blocked,exit` | `stop,idle,exit` | resolves on the first to fire. An unknown token is a `400` naming it, never a silent fallback | -| `timeout` | positive integer ms | `60000` | **validated first, clamped second.** `0`, a negative value and a fractional value are all `400`s, not clamps; a valid value outside `[1000, 600000]` is clamped and echoed as `wait.timeoutMs` | -| `fresh` | `0` \| `1` \| `false` \| `true` | `0` | `1` requires an actual transition, ignoring the state at call time | +| Param | Type | Default | Notes | +| --------- | -------------------------------------------------------- | ---------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `until` | comma-separated list of `idle,working,stop,blocked,exit` | `stop,idle,exit` | resolves on the first to fire. An unknown token is a `400` naming it, never a silent fallback | +| `timeout` | positive integer ms | `60000` | **validated first, clamped second.** `0`, a negative value and a fractional value are all `400`s, not clamps; a valid value outside `[1000, 600000]` is clamped and echoed as `wait.timeoutMs` | +| `fresh` | `0` \| `1` \| `false` \| `true` | `0` | `1` requires an actual transition, ignoring the state at call time | ```bash curl -s "$API/api/v1/sessions/$SID/wait?until=stop,exit&timeout=60000" @@ -239,12 +239,12 @@ a plain signal wait, so check the endpoint path before blaming the parameters. ### `GET /api/v1/sessions/:id/wait-output` -| Param | Type | Default | Notes | -|-------|------|---------|-------| -| `match` | literal string, 1 to 200 chars | required | substring match against the PTY stream with ANSI escapes stripped. A match spanning two PTY chunks is found | -| `nocase` | `0` \| `1` \| `false` \| `true` | `0` | case-insensitive compare. The returned snippet keeps the terminal's original casing | -| `from` | `now` \| `buffer` | `now` | `buffer` scans the tail of the existing terminal buffer (bounded, 256 KB by default) before blocking | -| `timeout` | positive integer ms | `60000` | same validation and clamp as `/wait` | +| Param | Type | Default | Notes | +| --------- | ------------------------------- | -------- | ----------------------------------------------------------------------------------------------------------- | +| `match` | literal string, 1 to 200 chars | required | substring match against the PTY stream with ANSI escapes stripped. A match spanning two PTY chunks is found | +| `nocase` | `0` \| `1` \| `false` \| `true` | `0` | case-insensitive compare. The returned snippet keeps the terminal's original casing | +| `from` | `now` \| `buffer` | `now` | `buffer` scans the tail of the existing terminal buffer (bounded, 256 KB by default) before blocking | +| `timeout` | positive integer ms | `60000` | same validation and clamp as `/wait` | **Matching is literal, never a pattern.** A `regex` parameter is rejected with a `400` rather than ignored, so a caller that assumed otherwise finds out immediately @@ -296,10 +296,10 @@ hand-written query string decodes to a space. Two optional fields on the existing endpoint: -| Field | Type | Notes | -|-------|------|-------| -| `wait` | `true` or the same comma grammar as `until` | `true` means the default signal set. Omitted keeps the historical fire-and-forget behavior, unchanged. `null`, `false` and an empty string are all read as **absent**, not as an error and not as "wait for the default" | -| `waitTimeout` | positive integer ms | same validation **and** clamp as `timeout`: `0`, a negative and a fractional value are `400`s, anything valid is clamped into `[1000, 600000]` and echoed as `wait.timeoutMs` | +| Field | Type | Notes | +| ------------- | ------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `wait` | `true` or the same comma grammar as `until` | `true` means the default signal set. Omitted keeps the historical fire-and-forget behavior, unchanged. `null`, `false` and an empty string are all read as **absent**, not as an error and not as "wait for the default" | +| `waitTimeout` | positive integer ms | same validation **and** clamp as `timeout`: `0`, a negative and a fractional value are `400`s, anything valid is clamped into `[1000, 600000]` and echoed as `wait.timeoutMs` | Both are `nullish`, so an explicit `null` from `JSON.stringify` is accepted as "absent" rather than failing validation. That is deliberate: `.optional()` would @@ -330,16 +330,24 @@ All three nest the wait result under `data.wait`, so one client helper works aga any of them: ```json -{ "success": true, "data": { - "sessionId": "28325fd3-caa7-4178-82bf-87dfebf0f464", - "status": "idle", - "limitPaused": false, - "wait": { - "signal": "stop", "until": ["stop", "idle", "exit"], - "timedOut": false, "immediate": false, "ended": false, "aborted": false, - "waitedMs": 8421, "timeoutMs": 60000 +{ + "success": true, + "data": { + "sessionId": "28325fd3-caa7-4178-82bf-87dfebf0f464", + "status": "idle", + "limitPaused": false, + "wait": { + "signal": "stop", + "until": ["stop", "idle", "exit"], + "timedOut": false, + "immediate": false, + "ended": false, + "aborted": false, + "waitedMs": 8421, + "timeoutMs": 60000 + } } -}} +} ``` `POST .../input` returns the same `wait` object alongside `delivered`, `duplicate`, @@ -353,21 +361,21 @@ redelivery (harmless, the turn it refers to may be long over), while with client that reads `delivered === false` as "duplicate" silently treats a failed send as a success. -| Field | Type | Meaning | -|-------|------|---------| -| `wait.signal` | signal \| `null` | the signal that fired (`/wait` and `/input` only) | -| `wait.until` | array of signals | what the server actually waited on, after narrowing the default set for the session's mode (`/wait` and `/input` only) | -| `wait.matched` | boolean | the string appeared (`/wait-output` only) | -| `wait.match` | string | the literal that was searched for (`/wait-output` only) | -| `wait.snippet` | string \| `null` | bounded window of output around the match, blank runs collapsed for readability (`/wait-output` only) | -| `wait.timedOut` | boolean | the wait hit its timeout. Still a `200` | -| `wait.immediate` | boolean | the condition already held at call time, so nothing was waited for (`waitedMs` is 0) | -| `wait.ended` | boolean | the session went away (deleted or torn down) before the condition was met | -| `wait.aborted` | boolean | the client hung up, so the waiter was released without resolving — and by that definition a client never reads `true`. When the **server** abandons a wait itself (send-and-wait against a session with no PTY), it answers in about a millisecond with `ended: true`, `delivered: false`, `duplicate: false` and `aborted: false`: `delivered`/`ended` carry that story, and `aborted` stays the transport flag. Present for completeness; treat a `true` as "this wait answered nothing", never as an outcome | -| `wait.waitedMs` | number | wall-clock ms actually spent waiting | -| `wait.timeoutMs` | number | the timeout **after clamping**, which is what was applied | -| `status` | `SessionStatus` | the session's status after the wait, so a caller that timed out still learns where things stand | -| `limitPaused` | boolean | the session is paused on a usage limit and will emit nothing until its reset, so a timeout here is expected rather than a stall worth retrying hard | +| Field | Type | Meaning | +| ---------------- | ---------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `wait.signal` | signal \| `null` | the signal that fired (`/wait` and `/input` only) | +| `wait.until` | array of signals | what the server actually waited on, after narrowing the default set for the session's mode (`/wait` and `/input` only) | +| `wait.matched` | boolean | the string appeared (`/wait-output` only) | +| `wait.match` | string | the literal that was searched for (`/wait-output` only) | +| `wait.snippet` | string \| `null` | bounded window of output around the match, blank runs collapsed for readability (`/wait-output` only) | +| `wait.timedOut` | boolean | the wait hit its timeout. Still a `200` | +| `wait.immediate` | boolean | the condition already held at call time, so nothing was waited for (`waitedMs` is 0) | +| `wait.ended` | boolean | the session went away (deleted or torn down) before the condition was met | +| `wait.aborted` | boolean | the client hung up, so the waiter was released without resolving — and by that definition a client never reads `true`. When the **server** abandons a wait itself (send-and-wait against a session with no PTY), it answers in about a millisecond with `ended: true`, `delivered: false`, `duplicate: false` and `aborted: false`: `delivered`/`ended` carry that story, and `aborted` stays the transport flag. Present for completeness; treat a `true` as "this wait answered nothing", never as an outcome | +| `wait.waitedMs` | number | wall-clock ms actually spent waiting | +| `wait.timeoutMs` | number | the timeout **after clamping**, which is what was applied | +| `status` | `SessionStatus` | the session's status after the wait, so a caller that timed out still learns where things stand | +| `limitPaused` | boolean | the session is paused on a usage limit and will emit nothing until its reset, so a timeout here is expected rather than a stall worth retrying hard | Read the outcome by discriminator, in this order: @@ -390,12 +398,12 @@ read the timeout as "the worker is wedged" and kill a session that was working f ### Errors -| `errorCode` | HTTP | When | -|-------------|------|------| -| `INVALID_INPUT` | 400 | unknown `until` / `wait` token; `stop` or `blocked` requested explicitly on a mode that installs no hooks (the message names the mode); `regex=` on `/wait-output`; `match` outside 1 to 200 chars; a non-numeric `timeout` | -| `NOT_FOUND` | 404 | no such session, or one this caller does not own | -| `SESSION_BUSY` | 409 | this session's waiter cap is full | -| `RATE_LIMITED` | 429 | a per-owner or process-wide waiter cap is full. Retry later; the session you named is not the problem | +| `errorCode` | HTTP | When | +| --------------- | ---- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `INVALID_INPUT` | 400 | unknown `until` / `wait` token; `stop` or `blocked` requested explicitly on a mode that installs no hooks (the message names the mode); `regex=` on `/wait-output`; `match` outside 1 to 200 chars; a non-numeric `timeout` | +| `NOT_FOUND` | 404 | no such session, or one this caller does not own | +| `SESSION_BUSY` | 409 | this session's waiter cap is full | +| `RATE_LIMITED` | 429 | a per-owner or process-wide waiter cap is full. Retry later; the session you named is not the problem | The two capacity codes are deliberately different. A process-wide cap reported as `SESSION_BUSY` would tell the caller to switch sessions, which cannot help. The @@ -446,9 +454,9 @@ Design: [`approvals-inbox-plan.md`](approvals-inbox-plan.md). - `GET /api/v1/approvals` → `{ approvals: ApprovalItem[] }`, oldest first, ownership-scoped in multi-user mode. `ApprovalItem`: `{ id, sessionId, - sessionName, kind: 'permission'|'question'|'idle', createdAt, toolName?, - toolSummary?, message?, cwd?, context?, options?: {n, label}[], - acknowledgedAt? }`. `context` is the ANSI-stripped visible pane frame; +sessionName, kind: 'permission'|'question'|'idle', createdAt, toolName?, +toolSummary?, message?, cwd?, context?, options?: {n, label}[], +acknowledgedAt? }`. `context` is the ANSI-stripped visible pane frame; `options` is present only when the dialog's numbered choices parsed confidently; `acknowledgedAt` marks an item a human has already looked at (see `/viewed` below) and tells clients not to re-arm its tab alert. Listing @@ -466,7 +474,7 @@ Design: [`approvals-inbox-plan.md`](approvals-inbox-plan.md). first, `422 OPERATION_FAILED` when the session refused input. - `POST /api/v1/approvals/:id/dismiss` removes the item without keystrokes. - `POST /api/v1/approvals/session/:sessionId/viewed` → `{ sessionId, - acknowledged: itemId | null }`. Marks the session's pending **idle** item as +acknowledged: itemId | null }`. Marks the session's pending **idle** item as seen by a human (the web UI calls it when you open the session's tab): the item stays pending and answerable, but stops arming the yellow tab alert on every client, including after a reload. Permission/question items are never @@ -479,6 +487,47 @@ re-captured, or the item acknowledged), `approval:resolved` (`{ id, sessionId, k `resolution` one of `answered | resolved_in_terminal | superseded | session_ended | dismissed | expired`). +## Reboot restore + +A host reboot takes the tmux server down with it, so every pane dies and the +board comes up empty. At boot Codeman works out which sessions the reboot +destroyed and holds that plan in memory, and these endpoints let a client offer +it to the user. Nothing creates a pane until the user asks: the boot-time reboot +heuristic decides whether to ASK, never whether to act. + +Claude-mode sessions only (others carry their conversation id in their own +config object); remote and docker sessions are never offered, because both need +another host or container to be up. The plan is in-memory, so a server restart +drops it and the offer is gone — the conversations themselves are unaffected, +since they live in the CLI's own transcript store and stay reachable from the +Resume list. A plan nobody spends expires after 24 hours. + +- `GET /api/v1/reboot-restore` → `{ sessions: RestorableSession[], +scrollbackRestored: false }`, ownership-scoped in multi-user mode. + `RestorableSession`: `{ id, name?, workingDir, mode, owner? }`. The persisted + record itself is never sent. `scrollbackRestored` is always `false` and exists + so a client states it: a restored session is a NEW pane, so the conversation + continues and the terminal history does not. +- `POST /api/v1/reboot-restore/restore` with `{ sessionIds?: string[] }` (omit + to restore everything the caller can see) → `{ restored: RestorableSession[], +skipped: { sessionId, reason }[] }`. `reason` is one of `workspace-missing` + (the directory is gone), `workspace-forbidden` (it is outside the caller's + workspace in multi-user mode), `already-live` (the conversation is already + open, typically resumed by hand from the Resume list), `capacity-reached` + (the global or per-user session cap), or `rebuild-failed` (the agent would not + start, most often a CLI binary missing from the server's PATH). + `409 CONFLICT` when that caller already has a restore running. Entries are + removed from the plan before any pane is built, so a double-click cannot put + two panes on one conversation; anything that never became a pane goes back on + offer, except `already-live`, which cannot stop being true. A restored session + comes back attached, idle and disarmed — respawn controllers and Ralph loops + are never re-armed automatically. +- `POST /api/v1/reboot-restore/dismiss` → `{ dismissed: n }`. Drops the offer + for everything the caller can see. + +Each rebuilt session also emits the ordinary `session:created` SSE event, so +clients other than the one that clicked pick it up without refetching. + ## Read My Mind intent profiles Per-case profiles of what the user is trying to accomplish: user/agent-stated @@ -491,7 +540,7 @@ user guide: [`readmymind.md`](readmymind.md). - `GET /api/v1/sessions/:id/intent` -> `{ intent: IntentProfile }` for the session's case. `IntentProfile`: `{ key, workingDir, updatedAt, goals, - recentPrompts: { ts, sessionId, text }[] }` (prompts oldest first, FIFO cap +recentPrompts: { ts, sessionId, text }[] }` (prompts oldest first, FIFO cap 50, each <= 500 chars). A case with nothing recorded answers an empty profile with `updatedAt: 0`; nothing is persisted by reads. - `PUT /api/v1/sessions/:id/intent` with `{ goals }` (<= 8192 chars, strict @@ -524,7 +573,7 @@ same speech-to-text service the CLI's own `/voice` mode uses. Gated on the synce [`claude-voice-plan.md`](claude-voice-plan.md). - `GET /api/v1/voice/status` -> `{ available, reason?, subscriptionType?, - expiresAt? }`. `reason` is `disabled` (setting off), `no-credentials` (nobody +expiresAt? }`. `reason` is `disabled` (setting off), `no-credentials` (nobody signed in to Claude Code on the server), `expired` (the access token elapsed; running any Claude session refreshes it) or `malformed`. The OAuth token itself is never returned by this or any other endpoint. diff --git a/src/reboot-restore.ts b/src/reboot-restore.ts index c8b6bd5e..e226138c 100644 --- a/src/reboot-restore.ts +++ b/src/reboot-restore.ts @@ -57,6 +57,12 @@ export interface RebootEvidence { * * This heuristic decides whether to ASK, never whether to act. A wrong yes costs * the user a banner they dismiss, because the restore itself waits for a click. + * + * ⚠️ `os.uptime()` reports the HOST's uptime, which a container shares. A Codeman + * running in Docker therefore sees a long uptime after its own container restarts, + * the boot test fails, and no banner appears. The feature is effectively off for + * containerized installs. That is the safe direction to fail in, and fixing it + * needs a boot signal the container actually owns rather than a wider heuristic. */ export function looksLikeHostReboot(evidence: RebootEvidence): boolean { if (evidence.deadSessionCount === 0) return false; @@ -80,7 +86,14 @@ export function resolveResumeConversationId(state: SessionState): string { return chainTail || state.resumeSessionId || state.id; } -/** Why one session was passed over. Reported for logging and assertions. */ +/** + * Why one session was passed over. Reported for logging and shown to the user. + * + * The first six are decided before anything is built. `capacity-reached` and + * `rebuild-failed` can only happen once a click is spending the plan, and they + * are the two the banner must not confuse with a missing workspace: one means + * "try again after closing something", the other means the CLI would not start. + */ export interface RebootRestoreRejection { sessionId: string; reason: @@ -91,7 +104,10 @@ export interface RebootRestoreRejection { | 'unsupported-mode' | 'no-working-dir' | 'workspace-missing' - | 'already-live'; + | 'workspace-forbidden' + | 'already-live' + | 'capacity-reached' + | 'rebuild-failed'; } /** One restorable session, as the banner shows it and the rebuild replays it. */ diff --git a/src/session-env-clamp.ts b/src/session-env-clamp.ts index edaecd5c..b9abf3a6 100644 --- a/src/session-env-clamp.ts +++ b/src/session-env-clamp.ts @@ -3,10 +3,16 @@ * * A session's `envOverrides` can hand back privilege that the per-CLI config * clamp removed, so a non-granted owner's overrides get the privileged keys - * stripped before the session is built. Two callers need that today. The create - * and resume routes clamp what a request asked for, and the reboot-restore route - * clamps what a persisted record carried, because a record written while its - * owner held a grant must not replay that grant after the grant is gone. + * stripped before the session is built. The create and resume routes are what + * this bites on: they clamp what a request asked for. + * + * The reboot-restore route calls it as defence in depth, and today it can strip + * nothing. `Session.getEnvOverridesForPersist()` keeps only `CLAUDE_CODE_*` and + * `CLAUDE_CONFIG_DIR` out of a session's overrides, claude's `privilegedEnvKeys` + * are the five `ANTHROPIC_*` names, and that pass admits claude alone — so a + * persisted record cannot carry a clamped key. The call is there for the day the + * persisted set widens. The grant re-resolution that does bite on that path is + * `resolveClaudeModeForUsername`, which recomputes the permission mode. * * This lives outside `web/routes` on purpose. The question it answers is about * session privilege rather than about HTTP, and `cron/cron-service.ts` sets the diff --git a/src/web/ports/session-port.ts b/src/web/ports/session-port.ts index 61e02be3..83918763 100644 --- a/src/web/ports/session-port.ts +++ b/src/web/ports/session-port.ts @@ -4,6 +4,7 @@ */ import type { Session } from '../../session.js'; +import type { SessionState } from '../../types.js'; export interface SessionPort { readonly sessions: ReadonlyMap; @@ -12,5 +13,16 @@ export interface SessionPort { setupSessionListeners(session: Session): Promise; 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. + * + * 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. + */ + reapplyPersistedSessionState(session: Session, saved: SessionState): Promise; getSessionStateWithRespawn(session: Session): unknown; } diff --git a/src/web/public/app.js b/src/web/public/app.js index 1f0ae409..7c670497 100644 --- a/src/web/public/app.js +++ b/src/web/public/app.js @@ -957,7 +957,9 @@ class CodemanApp { this.registerServiceWorker(); // Fetch tunnel status for header indicator (desktop only) this.loadTunnelStatus(); - // Ask whether a host reboot left sessions worth rebuilding (banner, never automatic) + // Ask whether a host reboot left sessions worth rebuilding (banner, never + // automatic). handleInit() re-reads it on every SSE init; this covers the + // path where that event never arrives. this.initRebootRestoreBanner?.(); // Share a single settings fetch between both consumers const settingsPromise = fetch('/api/settings').then(r => r.ok ? r.json() : null).then(env => env?.data ?? null).catch(() => null); @@ -3761,6 +3763,12 @@ class CodemanApp { // a fresh load / reconnect (authoritative; wins over the localStorage restore). if (data.planUsage) this.updatePlanUsageChip(data.planUsage); + // A board left open across a host reboot reconnects HERE, to a server that came + // back with an empty session list. The reboot-restore offer is built at boot, + // before any client could be listening, so re-read it on every init rather than + // only on the page-load path. + this.refreshRebootRestoreBanner?.(); + // Update version displays (header and toolbar) if (data.version) { const versionEl = this.$('versionDisplay'); diff --git a/src/web/public/mobile.css b/src/web/public/mobile.css index 9be29153..d324a94f 100644 --- a/src/web/public/mobile.css +++ b/src/web/public/mobile.css @@ -3240,6 +3240,41 @@ html:is([data-skin="paper-gray"], [data-skin="solarized-light"], [data-skin="cat already reserves that space), so it needs the same safe-area padding as the other banners. The overlay is fixed and handles its own insets. ============================================================================ */ +@media (max-width: 599px) { + /* Reboot-restore banner: the same treatment as the offline banner below. Its + text and note are nowrap and the two buttons cannot shrink, so without this + the actions are pushed off a phone-width viewport and become unreachable. */ + .reboot-restore-banner { + padding: 0.4rem 0.5rem; + padding-left: calc(0.5rem + var(--safe-area-left)); + padding-right: calc(0.5rem + var(--safe-area-right)); + font-size: 0.7rem; + gap: 0.4rem; + } + + /* The session names and the scrollback note are the first things to go. The + count plus the two buttons carry the message on their own, and the note + survives as the accept button's title. */ + .reboot-restore-banner-detail, + .reboot-restore-banner-note { + display: none; + } + + .reboot-restore-banner-text { + overflow: hidden; + text-overflow: ellipsis; + } + + .reboot-restore-banner-accept, + .reboot-restore-banner-dismiss { + padding: 0.25rem 0.5rem; + } + + .reboot-restore-banner-accept { + margin-left: auto; + } +} + @media (max-width: 599px) { .offline-banner { padding: 0.4rem 0.5rem; diff --git a/src/web/public/reboot-restore-ui.js b/src/web/public/reboot-restore-ui.js index 0869eebe..aec1de81 100644 --- a/src/web/public/reboot-restore-ui.js +++ b/src/web/public/reboot-restore-ui.js @@ -8,7 +8,9 @@ * reboot guess is a heuristic and a wrong automatic restore would spawn CLI * processes nobody asked for. * - * Seeded once from `GET /api/reboot-restore` on init. Restore posts to + * Seeded from `GET /api/reboot-restore` on init and again on every SSE reconnect, + * because the tab most likely to want this is one that was open across the reboot + * and reconnects to a server that came back up with an empty board. Restore posts to * `POST /api/reboot-restore/restore`, Dismiss posts to * `POST /api/reboot-restore/dismiss`, and either way the banner goes away. The * restored sessions arrive as ordinary `session:created` events, so no extra @@ -25,6 +27,24 @@ * @loadorder 11.7 of 17, after approvals-ui.js */ +/** Plain-language wording for one skip reason, for the toast after a restore. */ +function rebootSkipReason(reason) { + switch (reason) { + case 'workspace-missing': + return 'workspace is gone'; + case 'workspace-forbidden': + return 'workspace is outside your space'; + case 'already-live': + return 'already open'; + case 'capacity-reached': + return 'session limit reached'; + case 'rebuild-failed': + return 'the agent would not start'; + default: + return reason; + } +} + Object.assign(CodemanApp.prototype, { /** Ask the server whether a reboot left anything on offer, and show the banner if so. */ async initRebootRestoreBanner() { @@ -59,6 +79,9 @@ Object.assign(CodemanApp.prototype, { detail.textContent = count > 4 ? `${names}, …` : names; detail.title = sessions.map((s) => `${s.name || s.id}\n${s.workingDir}`).join('\n\n'); } + const accept = this.$('rebootRestoreBannerAccept'); + // The note is hidden at phone width, so the warning travels on the button too. + if (accept) accept.title = 'Conversations return; terminal history does not.'; banner.hidden = false; }, @@ -66,8 +89,9 @@ Object.assign(CodemanApp.prototype, { async restoreRebootSessions() { const button = this.$('rebootRestoreBannerAccept'); if (button) button.disabled = true; - const res = await this._apiPost('/api/reboot-restore/restore', {}); - const body = res && res.ok ? await res.json().catch(() => null) : null; + // _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: {} }); if (!body) { if (button) button.disabled = false; this.showToast?.('Could not restore the sessions', 'error'); @@ -82,10 +106,21 @@ Object.assign(CodemanApp.prototype, { this.showToast?.(`Restored ${restored} ${noun}. Terminal history did not survive the reboot.`, 'success'); } if (skipped > 0) { - this.showToast?.(`${skipped} could not be restored (workspace gone, or already open)`, 'warning'); + // Each reason means a different next step for the user, so they are not + // collapsed into one message: capacity clears by closing something, a + // failed start usually means the CLI is not on the server's PATH. + const reasons = new Set((body.skipped ?? []).map((s) => s.reason)); + this.showToast?.(`${skipped} not restored: ${[...reasons].map(rebootSkipReason).join('; ')}`, 'warning'); } }, + /** Re-read the offer after a reconnect, for a tab that was open across the reboot. */ + async refreshRebootRestoreBanner() { + const data = await this._apiJson('/api/reboot-restore'); + this._rebootRestoreSessions = data?.sessions ?? []; + this.renderRebootRestoreBanner(); + }, + /** Drop the offer. The Resume list still reaches every one of these conversations. */ async dismissRebootRestore() { this._rebootRestoreSessions = []; diff --git a/src/web/reboot-restore-registry.ts b/src/web/reboot-restore-registry.ts index e265fa62..789217eb 100644 --- a/src/web/reboot-restore-registry.ts +++ b/src/web/reboot-restore-registry.ts @@ -24,8 +24,9 @@ * - Spending is take-then-build: `take()` removes entries synchronously, before * the route's first `await`, so a double-click or two devices cannot both * reach the same entry and put two panes on one conversation. - * - One restore runs at a time. `beginSpending()` single-flights the route, so - * two concurrent clicks cannot interleave pane creation. + * - One restore runs at a time per owner. `beginSpending()` single-flights the + * route, so two concurrent clicks cannot interleave pane creation for the same + * user, while two different users never block each other. * * @dependencies reboot-restore (RebootRestoreEntry) * @consumedby web/server (plan build at boot), web/routes/reboot-restore-routes @@ -46,8 +47,13 @@ export class RebootRestoreRegistry { private entries = new Map(); /** When the boot pass built the plan, in ms since the epoch. */ private builtAt = 0; - /** True while a restore route call is between its take and its last pane. */ - private spending = false; + /** + * 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 + * a conflict; `take()` already guarantees no two callers get the same entry. + * Single-user mode has one key, `undefined`, so it behaves as one global flight. + */ + private spending = new Set(); /** Replace the plan with what the boot pass found. An empty list clears it. */ set(entries: readonly RebootRestoreEntry[]): void { @@ -92,9 +98,11 @@ export class RebootRestoreRegistry { /** * Put entries back after a rebuild never got as far as creating a pane. * - * Used for the click-time rejections, so a conversation the user resumed by - * hand meanwhile does not silently vanish from the banner while a workspace - * that came back stays offered. + * Used for the click-time rejections that may resolve themselves: a workspace + * that comes back, a capacity limit the user makes room under, a CLI that + * starts once its binary is on the PATH. A conversation the user resumed by + * 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 { for (const entry of entries) this.entries.set(entry.sessionId, entry); @@ -110,24 +118,25 @@ export class RebootRestoreRegistry { } /** - * Claim the right to run a restore, or report that one is already running. - * Callers that get `true` must call `endSpending()` in a `finally`. + * Claim the right to run a restore for one owner, or report that owner already + * has one running. Callers that get `true` must call `endSpending()` in a + * `finally` with the same owner. */ - beginSpending(): boolean { - if (this.spending) return false; - this.spending = true; + beginSpending(owner?: string): boolean { + if (this.spending.has(owner)) return false; + this.spending.add(owner); return true; } - endSpending(): void { - this.spending = false; + endSpending(owner?: string): void { + this.spending.delete(owner); } /** Test hook: forget everything, including the single-flight claim. */ reset(): void { this.entries.clear(); this.builtAt = 0; - this.spending = false; + this.spending.clear(); } private dropIfExpired(): void { diff --git a/src/web/routes/reboot-restore-routes.ts b/src/web/routes/reboot-restore-routes.ts index 580b17c6..a03c74af 100644 --- a/src/web/routes/reboot-restore-routes.ts +++ b/src/web/routes/reboot-restore-routes.ts @@ -27,7 +27,14 @@ import { FastifyInstance } from 'fastify'; import { existsSync } from 'node:fs'; import { ApiErrorCode, createErrorResponse, getErrorMessage } from '../../types.js'; import { RebootRestoreRequestSchema } from '../schemas.js'; -import { parseBody, getAuthUser, canAccessOwned } from '../route-helpers.js'; +import { + parseBody, + getAuthUser, + canAccessOwned, + ownerFor, + isWorkingDirAllowed, + sessionCapacityMessage, +} from '../route-helpers.js'; import { rebootRestoreRegistry } from '../reboot-restore-registry.js'; import { rejectAlreadyLive, type RebootRestoreEntry, type RebootRestoreRejection } from '../../reboot-restore.js'; import { clampEnvOverridesForOwner } from '../../session-env-clamp.js'; @@ -76,38 +83,65 @@ 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); // Take BEFORE the first await: a second click must find nothing to spend. - if (!rebootRestoreRegistry.beginSpending()) { + // The flight is per owner, because `take()` already guarantees two callers + // never receive the same entry, so one user's restore need not block another's. + if (!rebootRestoreRegistry.beginSpending(owner)) { return reply.code(409).send(createErrorResponse(ApiErrorCode.CONFLICT, 'A reboot restore is already running')); } 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 + // spend the offer and rebuild nothing, and the plan cannot be rebuilt. + const unspent = new Set(taken); try { if (taken.length === 0) return { restored: [], skipped: [] }; // The plan was built at boot and the board has moved on since. A conversation // the user resumed by hand from the Resume list is already on screen, and a - // second pane on it would fight the first for the same transcript. + // second pane on it would fight the first for the same transcript. This one + // is never re-offered: unlike a missing workspace, it cannot stop being true. const liveSessionIds = new Set(ctx.sessions.keys()); const liveConversationIds = new Set( [...ctx.sessions.values()].map((session) => session.claudeSessionId).filter((id): id is string => !!id) ); const { restore, skipped } = rejectAlreadyLive(taken, liveSessionIds, liveConversationIds); - // An entry nothing rebuilt stays on offer rather than disappearing silently. - rebootRestoreRegistry.restore(skipped.map((s) => taken.find((e) => e.sessionId === s.sessionId)!)); + for (const entry of taken) { + if (skipped.some((s) => s.sessionId === entry.sessionId)) unspent.delete(entry); + } const restored: ReturnType[] = []; const failures: RebootRestoreRejection[] = [...skipped]; const workspaceHooksEnabled = await ctx.getWorkspaceHooksEnabled(); for (const entry of restore) { + // Capacity is re-checked per iteration, because this loop is itself + // creating the sessions it counts. The offer can be a day old, so the + // board may be fuller now than the plan assumed. + const capMsg = sessionCapacityMessage(ctx.sessions, entry.owner); + if (capMsg) { + failures.push({ sessionId: entry.sessionId, reason: 'capacity-reached' }); + continue; + } // A repo can be deleted between the boot that planned this and the click. if (!existsSync(entry.workingDir)) { failures.push({ sessionId: entry.sessionId, reason: 'workspace-missing' }); continue; } + // 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)) { + failures.push({ sessionId: entry.sessionId, reason: 'workspace-forbidden' }); + unspent.delete(entry); + continue; + } try { const saved = entry.state; const claudeModeConfig = await ctx.getClaudeModeConfig(); @@ -145,9 +179,14 @@ export function registerRebootRestoreRoutes(app: FastifyInstance, ctx: RebootRes }); await ctx.addSession(session); - ctx.persistSessionState(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); await session.startInteractive(); + ctx.persistSessionState(session); // A session without its workspace hooks goes silently blind: no stop or // idle events for respawn, no Approvals Inbox item, no red tab on a @@ -166,10 +205,22 @@ export function registerRebootRestoreRoutes(app: FastifyInstance, ctx: RebootRes ctx.broadcast(SseEvent.SessionCreated, ctx.getSessionStateWithRespawn(session)); restored.push(toBannerItem(entry)); } catch (err) { - // One workspace that has gone missing must not stop the rest of the pass. + // One entry that will not start must not stop the rest of the pass, and + // must not leave a registered session with no pane behind it: by this + // point the session is in `ctx.sessions`, holds a tab-layout slot and has + // 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); - failures.push({ sessionId: entry.sessionId, reason: 'workspace-missing' }); + 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)}`) + ); + failures.push({ sessionId: entry.sessionId, reason: 'rebuild-failed' }); + // Left on offer: the user can put the binary back and click again. + continue; } + unspent.delete(entry); } if (restored.length > 0) { @@ -181,7 +232,10 @@ export function registerRebootRestoreRoutes(app: FastifyInstance, ctx: RebootRes return { restored, skipped: failures }; } finally { - rebootRestoreRegistry.endSpending(); + // 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]); + rebootRestoreRegistry.endSpending(owner); } }); diff --git a/src/web/server.ts b/src/web/server.ts index 065172d5..a7339328 100644 --- a/src/web/server.ts +++ b/src/web/server.ts @@ -671,6 +671,7 @@ export class WebServer extends EventEmitter { setupSessionListeners: this.setupSessionListeners.bind(this), persistSessionState: this.persistSessionState.bind(this), persistSessionStateNow: this._persistSessionStateNow.bind(this), + reapplyPersistedSessionState: this.reapplyPersistedSessionState.bind(this), getSessionStateWithRespawn: this.getSessionStateWithRespawn.bind(this), // EventPort broadcast: this.broadcast.bind(this), @@ -2907,6 +2908,53 @@ export class WebServer extends EventEmitter { return restore.length; } + /** + * Re-apply the persisted state that a `Session` constructor does not take. + * + * The reboot-restore route builds a session from a record rather than + * attaching to a surviving pane, so everything the constructor has no + * parameter for starts at its default. Persisting such a session writes + * `toState()` wholesale, which would REPLACE the record with the reduced + * version — and for a pinned session that is worse than losing a setting, + * 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. + */ + 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)); + } + if (saved.pinned) session.setPinned(true); + if (saved.autoCompactEnabled !== undefined || saved.autoCompactThreshold !== undefined) { + session.setAutoCompact(saved.autoCompactEnabled ?? false, saved.autoCompactThreshold, saved.autoCompactPrompt); + } + if (saved.autoClearEnabled !== undefined || saved.autoClearThreshold !== undefined) { + session.setAutoClear(saved.autoClearEnabled ?? false, saved.autoClearThreshold); + } + if (saved.autoResumeEnabled) { + session.restoreAutoResume(true, saved.autoResumeAt); + } + if (saved.inputTokens !== undefined || saved.outputTokens !== undefined || saved.totalCost !== undefined) { + session.restoreTokens(saved.inputTokens ?? 0, saved.outputTokens ?? 0, saved.totalCost ?? 0); + // Seed the daily-usage baseline, or the restored totals are counted again as new usage. + this.lastRecordedTokens.set(session.id, { + input: saved.inputTokens ?? 0, + output: saved.outputTokens ?? 0, + }); + } + if (saved.niceEnabled !== undefined || saved.niceValue !== undefined) { + session.setNice({ enabled: saved.niceEnabled, niceValue: saved.niceValue }); + } + if (saved.flickerFilterEnabled !== undefined) session.flickerFilterEnabled = saved.flickerFilterEnabled; + } + 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 8a1bd884..401538be 100644 --- a/test/mocks/mock-route-context.ts +++ b/test/mocks/mock-route-context.ts @@ -61,6 +61,7 @@ export function createMockRouteContext(options?: { setupSessionListeners: vi.fn(async () => {}), persistSessionState: vi.fn(), persistSessionStateNow: vi.fn(), + reapplyPersistedSessionState: vi.fn(async () => {}), getSessionStateWithRespawn: vi.fn((s: MockSession) => s.toState()), // -- EventPort -- @@ -149,6 +150,7 @@ export function createMockRouteContext(options?: { clearRespawnConfig: vi.fn(), updateRespawnConfig: vi.fn(), setHistoryLimit: vi.fn(async () => {}), + startStatsCollection: vi.fn(), }, runSummaryTrackers: new Map(), activePlanOrchestrators: new Map(), diff --git a/test/routes/reboot-restore-rebuild-failure.test.ts b/test/routes/reboot-restore-rebuild-failure.test.ts new file mode 100644 index 00000000..f8f58f58 --- /dev/null +++ b/test/routes/reboot-restore-rebuild-failure.test.ts @@ -0,0 +1,210 @@ +/** + * Reboot-restore route: what happens when a rebuild gets part-way and then fails. + * + * The other route test file deliberately uses workspaces that do not exist, so it + * never reaches `new Session()`. This one mocks the `Session` module so the route + * runs its whole construction path — `addSession`, `setupSessionListeners`, + * `reapplyPersistedSessionState`, `startInteractive` — and then throws where a + * real one would when the CLI binary is missing from a freshly booted machine's + * PATH. Without the mock there is no way to exercise that path, which is how the + * original version of this route shipped a session leak the tests could not see. + * + * It also covers the session caps, because those too are only reachable once the + * route is actually willing to build something. + */ +import { describe, it, expect, afterEach, vi, beforeEach } from 'vitest'; +import Fastify, { type FastifyInstance } from 'fastify'; +import fastifyCookie from '@fastify/cookie'; + +/** Set per test: whether the mocked `startInteractive()` rejects. */ +let startShouldThrow = false; + +vi.mock('../../src/session.js', () => ({ + Session: class { + id: string; + mode: string; + name?: string; + workingDir: string; + owner?: string; + claudeSessionId: string | null = null; + constructor(config: { id: string; mode?: string; name?: string; workingDir: string; owner?: string }) { + this.id = config.id; + this.mode = config.mode ?? 'claude'; + this.name = config.name; + this.workingDir = config.workingDir; + this.owner = config.owner; + } + async startInteractive() { + if (startShouldThrow) throw new Error('spawn claude ENOENT'); + } + /** The mock route context projects a session through this on broadcast. */ + toState() { + return { id: this.id, mode: this.mode, name: this.name, workingDir: this.workingDir, owner: this.owner }; + } + }, +})); + +const { registerRebootRestoreRoutes } = await import('../../src/web/routes/reboot-restore-routes.js'); +const { rebootRestoreRegistry } = await import('../../src/web/reboot-restore-registry.js'); +const { installRouteErrorHandler } = await import('../../src/web/route-error-handler.js'); +const { httpStatusForErrorCode } = await import('../../src/types.js'); +const { createMockRouteContext } = await import('../mocks/index.js'); +type ApiErrorCode = import('../../src/types.js').ApiErrorCode; +type RebootRestoreEntry = import('../../src/reboot-restore.js').RebootRestoreEntry; +type SessionState = import('../../src/types.js').SessionState; + +/** A real directory, so the route's workspace checks pass and it reaches the build. */ +const WORKSPACE = process.cwd(); + +function offerEntry(sessionId: string, owner?: string): RebootRestoreEntry { + return { + sessionId, + name: `session ${sessionId}`, + workingDir: WORKSPACE, + owner, + mode: 'claude', + resumeConversationId: `conv-${sessionId}`, + state: { + id: sessionId, + pid: null, + status: 'idle', + workingDir: WORKSPACE, + currentTaskId: null, + createdAt: 1_760_000_000_000, + mode: 'claude', + owner, + } as SessionState, + }; +} + +async function createHarness(ctx: ReturnType): Promise { + const app = Fastify({ logger: false }); + await app.register(fastifyCookie); + registerRebootRestoreRoutes(app, ctx as never); + app.addHook('preSerialization', (req, reply, payload: unknown, done) => { + if (!req.url.startsWith('/api')) return done(null, payload); + if (payload === null || typeof payload !== 'object') return done(null, payload); + const p = payload as { success?: unknown; errorCode?: unknown }; + if (p.success === false) { + if (reply.statusCode === 200 && typeof p.errorCode === 'string') { + reply.code(httpStatusForErrorCode(p.errorCode as ApiErrorCode)); + } + return done(null, payload); + } + if (p.success === true) return done(null, payload); + return done(null, { success: true, data: payload }); + }); + installRouteErrorHandler(app); + await app.ready(); + return app; +} + +beforeEach(() => { + startShouldThrow = false; +}); + +afterEach(() => { + rebootRestoreRegistry.reset(); + vi.clearAllMocks(); +}); + +describe('a rebuild that fails after the session is registered', () => { + it('reports why it failed rather than blaming the workspace', async () => { + startShouldThrow = true; + rebootRestoreRegistry.set([offerEntry('a')]); + const ctx = createMockRouteContext({ workspaceHooksEnabled: false }); + const app = await createHarness(ctx); + + const res = (await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} })).json().data; + expect(res.restored).toEqual([]); + // Not `workspace-missing`: the directory is there, the agent would not start. + expect(res.skipped).toEqual([{ sessionId: 'a', reason: 'rebuild-failed' }]); + await app.close(); + }); + + it('does not leave a registered session with no pane behind it', async () => { + startShouldThrow = true; + rebootRestoreRegistry.set([offerEntry('a')]); + const ctx = createMockRouteContext({ workspaceHooksEnabled: false }); + const app = await createHarness(ctx); + + 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)); + await app.close(); + }); + + it('keeps the entry on offer, so the user can fix the PATH and click again', async () => { + startShouldThrow = true; + rebootRestoreRegistry.set([offerEntry('a')]); + const ctx = createMockRouteContext({ workspaceHooksEnabled: false }); + const app = await createHarness(ctx); + + await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} }); + const left = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data; + expect(left.sessions.map((s: { id: string }) => s.id)).toEqual(['a']); + await app.close(); + }); +}); + +describe('a rebuild that succeeds', () => { + it('re-applies the persisted state before the record is written again', async () => { + rebootRestoreRegistry.set([offerEntry('a')]); + const ctx = createMockRouteContext({ workspaceHooksEnabled: false }); + 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']); + // A session built from a record carries none of the pin, token totals or + // custom-model selection, so persisting it first would replace the fuller + // record with the reduced one. + expect(ctx.reapplyPersistedSessionState).toHaveBeenCalled(); + const reapplyOrder = (ctx.reapplyPersistedSessionState as ReturnType).mock.invocationCallOrder[0]; + const persistOrder = (ctx.persistSessionState as ReturnType).mock.invocationCallOrder[0]; + expect(reapplyOrder).toBeLessThan(persistOrder); + await app.close(); + }); + + it('tells every other board about the rebuilt session', async () => { + rebootRestoreRegistry.set([offerEntry('a')]); + const ctx = createMockRouteContext({ workspaceHooksEnabled: false }); + const app = await createHarness(ctx); + + await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} }); + expect(ctx.broadcast).toHaveBeenCalledWith('session:created', expect.anything()); + await app.close(); + }); + + it('spends the entry, so it is no longer on offer', async () => { + rebootRestoreRegistry.set([offerEntry('a')]); + const ctx = createMockRouteContext({ workspaceHooksEnabled: false }); + const app = await createHarness(ctx); + + await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} }); + const left = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data; + expect(left.sessions).toEqual([]); + await app.close(); + }); +}); + +describe('the session caps', () => { + it('stops restoring at the global cap and leaves the rest on offer', 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); + } + const app = await createHarness(ctx); + + 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']); + + // 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).sort()).toEqual(['a', 'b']); + await app.close(); + }); +}); diff --git a/test/routes/reboot-restore-routes.test.ts b/test/routes/reboot-restore-routes.test.ts index c16cddac..dcb7d7ed 100644 --- a/test/routes/reboot-restore-routes.test.ts +++ b/test/routes/reboot-restore-routes.test.ts @@ -23,6 +23,13 @@ import type { RebootRestoreEntry } from '../../src/reboot-restore.js'; import type { SessionState } from '../../src/types.js'; async function createHarness(authUser?: { username: string; role: 'admin' | 'user' }): Promise { + return createHarnessWithCtx(createMockRouteContext(), authUser); +} + +async function createHarnessWithCtx( + ctx: ReturnType, + authUser?: { username: string; role: 'admin' | 'user' } +): Promise { const app = Fastify({ logger: false }); await app.register(fastifyCookie); if (authUser) { @@ -30,7 +37,7 @@ async function createHarness(authUser?: { username: string; role: 'admin' | 'use (req as unknown as { authUser: typeof authUser }).authUser = authUser; }); } - registerRebootRestoreRoutes(app, createMockRouteContext() as never); + registerRebootRestoreRoutes(app, ctx as never); app.addHook('preSerialization', (req, reply, payload: unknown, done) => { if (!req.url.startsWith('/api')) return done(null, payload); @@ -106,18 +113,36 @@ describe('GET /api/reboot-restore', () => { }); describe('POST /api/reboot-restore/restore', () => { - it('spends the offer, so a second click finds nothing left to spend', async () => { + it('reports a workspace that is gone, and keeps offering it in case it comes back', async () => { rebootRestoreRegistry.set([offerEntry('a')]); const app = await createHarness(); const first = (await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} })).json().data; - // The workspace is gone, so nothing was rebuilt — but the entry was taken. expect(first.restored).toEqual([]); expect(first.skipped).toEqual([{ sessionId: 'a', reason: 'workspace-missing' }]); - const second = (await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} })).json().data; - expect(second.restored).toEqual([]); - expect(second.skipped).toEqual([]); + // Nothing was built, so the entry goes back: a repo can be restored from a + // backup between two clicks, and losing the offer would be unrecoverable. + const left = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data; + expect(left.sessions.map((s: { id: string }) => s.id)).toEqual(['a']); + await app.close(); + }); + + it('never re-offers a conversation that is already open', async () => { + const entry = offerEntry('a'); + rebootRestoreRegistry.set([entry]); + const app = await createHarness(); + const ctx = createMockRouteContext({ sessionId: entry.sessionId }); + // A session with that id is live, which is what the Resume list would produce. + const liveApp = await createHarnessWithCtx(ctx); + + const res = (await liveApp.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} })).json().data; + expect(res.skipped).toEqual([{ sessionId: 'a', reason: 'already-live' }]); + + // Unlike a missing workspace, this one is dropped: it cannot stop being true. + const left = (await liveApp.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data; + expect(left.sessions).toEqual([]); + await liveApp.close(); await app.close(); }); @@ -132,8 +157,9 @@ describe('POST /api/reboot-restore/restore', () => { }); expect(res.json().data.skipped).toEqual([{ sessionId: 'b', reason: 'workspace-missing' }]); + // 'a' was never taken, and 'b' came back because no pane was built for it. const left = (await app.inject({ method: 'GET', url: '/api/reboot-restore' })).json().data; - expect(left.sessions.map((s: { id: string }) => s.id)).toEqual(['a']); + expect(left.sessions.map((s: { id: string }) => s.id).sort()).toEqual(['a', 'b']); await app.close(); }); @@ -150,12 +176,13 @@ describe('POST /api/reboot-restore/restore', () => { it('turns a second concurrent restore away rather than interleaving it', async () => { rebootRestoreRegistry.set([offerEntry('a')]); - // Claimed by a restore already in flight. - expect(rebootRestoreRegistry.beginSpending()).toBe(true); + // Claimed by a restore already in flight for this same owner (undefined in + // single-user mode, which is what the harness runs as). + expect(rebootRestoreRegistry.beginSpending(undefined)).toBe(true); const app = await createHarness(); const res = await app.inject({ method: 'POST', url: '/api/reboot-restore/restore', payload: {} }); expect(res.statusCode).toBe(409); - rebootRestoreRegistry.endSpending(); + rebootRestoreRegistry.endSpending(undefined); await app.close(); }); });