diff --git a/docs/architecture-invariants.md b/docs/architecture-invariants.md index b6c047a9..8753fea8 100644 --- a/docs/architecture-invariants.md +++ b/docs/architecture-invariants.md @@ -54,7 +54,7 @@ Model is NOT a session field: it is a composition entry in the profile's config ### Remote SSH cases -**Remote host wake-on-LAN from user input**: an optional `RemoteHost.wakeMac` (magic packet built and broadcast by Codeman) or `RemoteHost.wakeCommand` (a single executable path, run WITHOUT a shell, and the explicit override) lets the input route — and an explicit `POST /api/sessions/:id/wake` — wake a SLEEPING host instead of writing into a stalled ssh pane; `tmux send-keys` succeeds against a stalled pane, so the bytes used to vanish silently. The wake flow lives in `src/remote-wake.ts` and is reachable **only** from `POST /api/sessions/:id/input` and that explicit wake route: the COD-108 auto-reconnect watcher, `Server.handleRemoteSessionDropped` and boot recovery must never wake a host, or it would be re-woken seconds after each suspend and could never stay asleep (asserted by a wiring guard in `test/remote-wake.test.ts`, not just documented). `GET /api/sessions/:id/reachability` only ASKS — it never wakes — and feeds the amber "host unreachable" banner (`host-wake-ui.js`) whose action is either Wake or, with no target configured, "Configure WoL" → `#wakeConfigModal` (saved via `PUT /api/remote-hosts/:id`). Detection is a throttled bare TCP probe (no ssh, no `ServerAliveInterval` — keepalives would move bytes into an idle connection every interval), input is buffered and flushed in order after `reattachRemote()` (the send-and-wait path blocks instead), and the wake fields are re-read from `remote-hosts.json` on recovery AND (throttled, cached) live for a running session, because the persisted `remote` snapshot would never see a field added later (`rehydrateRemoteHostFields` + `RemoteWakeDeps.resolveRemote`). Design + invariants: `docs/remote-sessions.md` §Wake-on-LAN from user input. +**Remote host wake-on-LAN from user input**: an optional `RemoteHost.wakeMac` (magic packet built and broadcast by Codeman) or `RemoteHost.wakeCommand` (a single executable path, run WITHOUT a shell, and the explicit override) lets the input route — and an explicit `POST /api/sessions/:id/wake` — wake a SLEEPING host instead of writing into a stalled ssh pane; `tmux send-keys` succeeds against a stalled pane, so the bytes used to vanish silently. The wake flow lives in `src/remote-wake.ts` and is reachable **only** from an EXPLICIT user request: `POST /api/sessions/:id/input`, that explicit wake route, and the create/attach path (`POST /api/quick-start` for a remote case, `POST /api/sessions` with `attachRemoteSession`, via `ensureHostAwake`), because "the user pressed Run on a sleeping host" is the same kind of request and the tmux probe would otherwise fail with a misleading "needs tmux installed". Everything TIMER-driven must never wake a host: the COD-108 auto-reconnect watcher, `Server.handleRemoteSessionDropped` and boot recovery have no access to the registry, or a host would be re-woken seconds after each suspend and could never stay asleep (asserted by wiring guards in `test/remote-wake.test.ts`, not just documented — including that `ensureHostAwake` is called from the HTTP route only, since `cron-service.ts` builds sessions through the shared service with nobody waiting on the answer). `GET /api/sessions/:id/reachability` only ASKS — it never wakes — and feeds the amber "host unreachable" banner (`host-wake-ui.js`) whose action is either Wake or, with no target configured, "Configure WoL" → `#wakeConfigModal` (saved via `PUT /api/remote-hosts/:id`). Detection is a throttled bare TCP probe (no ssh, no `ServerAliveInterval` — keepalives would move bytes into an idle connection every interval), input is buffered and flushed in order after `reattachRemote()` (the send-and-wait path blocks instead, as does the create path, with a shorter request budget), and the wake fields are re-read from `remote-hosts.json` on recovery AND (throttled, cached) live for a running session, because the persisted `remote` snapshot would never see a field added later (`rehydrateRemoteHostFields` + `RemoteWakeDeps.resolveRemote`). Design + invariants: `docs/remote-sessions.md` §Wake-on-LAN from user input. **Remote SSH cases** (COD-94/#145): cases can point at a **remote host** (`~/.codeman/remote-hosts.json` + `remote-cases.json` via `src/remote-hosts.ts`; CRUD under `/api/cases` — cases route file). A remote session launches a LOCAL tmux pane running `ssh ` that creates a durable REMOTE tmux session on a **dedicated socket** `-L codeman-remote` with name `codeman-ssh-` — deliberately failing the remote Codeman's `SAFE_MUX_NAME_PATTERN` so a Codeman instance on the target host never adopts it; no `-g` global tmux options are set remotely. `remotePath`/`identityFile` are schema-guarded against shell injection (backticks/`$` rejected — same approach as `extraSshOptions`); remote tmux availability is probed via `checkRemoteTmuxAvailable()` in quick-start (ssh args carry `-o ConnectTimeout=10`). Remote claude defaults to an idempotent `claude --session-id || claude --resume ` pair under a login shell, so a respawn or reattach continues the SAME conversation rather than starting a fresh one (remote omp gets the same treatment via `--continue`; ⚠️ because the claude arm is an `a || b` pair under `-c`, that pane's PID is the login shell, not the agent); per-host `commands.*` override. Session kill best-effort kills the remote tmux too. `SessionState.remote`/`MuxSession.remote` round-trip through recovery (`restoreMuxSessions` passes `remote` back into the Session constructor). ⚠️ Run flows must route remote cases through `POST /api/quick-start` (which resolves the remote case and skips LOCAL CLI availability gates) — `POST /api/sessions` stat-validates `workingDir` locally and has no `caseName`. `envOverrides`/`effort`/`modelOverride`/`codexConfig`/`geminiConfig` are rejected for remote quick-starts (not silently dropped). UI: Create Case modal → Remote tab. Tests: `test/remote-hosts.test.ts`, `test/remote-ssh-options.test.ts`. ⚠️ **Reading a file in a remote case goes over ssh too** (#415): `src/remote-files.ts` is the single remote-READ layer (`buildRemoteFileCommand` = `buildSshConnectionArgs` + one shellescaped remote command; `remoteProbePaths` returns remote realpath + stat; `remoteCreateReadStream` streams a `Range` via `tail -c +N | head -c L` and its `close()` must be wired to the response's `close` or the ssh child outlives an aborted download). The guard order matches the local path exactly (`validateSessionFilePathLexical` → remote realpath of BOTH file and workspace root → containment → sensitive-path → size cap on the REMOTE size), a request path arrives from the browser and is only ever interpolated as a `shellescape`d token, and an unreachable host answers **502**, never a 404. ⚠️ The probe's symlink resolution FAILS CLOSED: `readlink -f` where it exists, otherwise a `cd -P`/`pwd -P` directory walk plus a bounded plain-`readlink` loop over the last component, and anything it cannot fully resolve is reported unresolvable (404), never as the unresolved string — the first version resolved the directory chain only, so on a host without `readlink -f` a `ws/notes.txt -> ~/.ssh/id_rsa` link passed containment under its own path while `cat` served the key. Records are NUL-separated and index-keyed so a newline in a filename cannot shift the mapping. ⚠️ ssh children are BOUNDED: probes and buffered reads go through `src/remote-ssh-limiter.ts` (a `document-conversion-limiter`-shaped semaphore, default 4), the attachment-history list probes its whole history in ONE batched call (`probeRemoteAttachmentHistory`, threaded into `registerExternalAttachment({remoteProbes})`), and probes chunk at 40 paths — a prompt-injected agent printing `codeman://attach` links in a remote session used to fork one `ssh` per link. `describeExecError` never returns Node's `Command failed: ` message (identity path + probe script in a 502 body). The `PUT /file-content` guard sits AHEAD of `validateSessionFilePath`, which resolves LOCALLY, or a same-named local directory (an sshfs mount) takes the write. Under `VITEST` the three IO functions refuse rather than connect. This covers the ATTACHMENT routes too, which is the half a clicked path needs when the file is OUTSIDE the case directory (`_isExternalPreviewPath` sends it to `POST …/attachments`): registration, by-id `raw`, metadata and the history list all resolve over ssh (`registerExternalAttachment({remote})`, `resolveServableRemoteAttachment`), and what decides the host is the SESSION, never the path string — the same absolute path means a different file on each host. Deliberately NOT supported over ssh: writes (`edit=1`/`PUT` answer 400, `editable` is always false), office previews/thumbnails, the file tree/picker, `tail-file`. Tests: `test/remote-files.test.ts`, `test/routes/file-routes-remote.test.ts`. diff --git a/docs/remote-sessions.md b/docs/remote-sessions.md index 6a73be42..5ef6c56a 100644 --- a/docs/remote-sessions.md +++ b/docs/remote-sessions.md @@ -368,6 +368,20 @@ it is unreachable it wakes it, polls until the host answers, reattaches the pane agent conversation is not restarted), and flushes the input that arrived meanwhile. Implementation: `src/remote-wake.ts`. +The same wake path also serves **opening** a session, which is where a sleeping host used to +be a dead end: pressing Run on a remote case (`POST /api/quick-start`) or Attach on a +discovered remote tmux session (`POST /api/sessions` + `attachRemoteSession`) probes the host +first, and on a sleeping one wakes it, waits for SSH and only then runs the tmux prereq probe. +Without that the run failed with `could not verify tmux on remote host …` — an ssh error that +blames tmux for a machine that is merely suspended. The wait is **blocking** (the caller gets +the session or the error) but bounded by `REMOTE_WAKE_REQUEST_READY_TIMEOUT_MS` (40 s) rather +than the 90 s session default, because the dashboard sits behind a reverse proxy whose default +`proxy_read_timeout` is 60 s: a longer wait would be cut off at the proxy while the session was +still being created. The budget covers the whole request, not just the wait (40 s wake + 1.5 s +probe + the tmux prereq probe's own 15 s timeout = 56.5 s worst case). A host with no wake target is not even probed on this path, so nothing +changes for it, and `remote:hostWaking` is broadcast without a `sessionId` (the toast then reads +"the session starts when it is back" — there is no session yet, and no input queued behind it). + Two wake paths, `wakeCommand` first because it is the explicit override: - **`wakeMac`** — Codeman builds the magic packet itself (`buildMagicPacket`, six `0xFF` @@ -387,12 +401,17 @@ banner comes from `GET /api/sessions/:id/reachability`, polled for the active re The invariants worth keeping: -- **Only real user input or an explicit wake request may wake a host.** The COD-108 watcher, - the server's dropped-session handler and boot recovery have no access to the wake registry — - a wake there would re-wake the host seconds after every suspend, so it could never stay - asleep (the same failure `hufflepuff-mcp-lazy` exists to prevent for MCP keepalives). A - reachability check never wakes: it is a question, not an action. Both are enforced by tests - in `test/remote-wake.test.ts` and `test/routes/session-remote-wake.test.ts`, not comments. +- **Only an EXPLICIT request may wake a host:** user input on an established session, the wake + button, or the user's own session create/attach request (`ensureHostAwake`). Everything that + runs on a TIMER must never wake one — the COD-108 watcher, the server's dropped-session + handler, boot recovery and session discovery have no access to the wake registry, and neither + has the shared session service, because `cron-service.ts` builds sessions there with nobody + waiting on the answer; a wake on such a path would re-wake the host seconds after every + suspend, so it could never stay asleep (the same failure `hufflepuff-mcp-lazy` exists to + prevent for MCP keepalives). A reachability check, a discovery listing and the tmux prereq + probe never wake: they are questions, not actions. All of it is enforced by tests in + `test/remote-wake.test.ts` (two wiring guards, one of them asserting `ensureHostAwake` has + exactly one caller file) and `test/routes/session-remote-wake.test.ts`, not by comments. - **Detection is a bare TCP connect** to the SSH port (then the configured `port`, else 22), throttled per session, and only for wake-enabled hosts. No `ServerAliveInterval` is added to the launch command: keepalives push bytes into an otherwise idle connection every interval, diff --git a/src/remote-wake.ts b/src/remote-wake.ts index 8022eead..fab22d15 100644 --- a/src/remote-wake.ts +++ b/src/remote-wake.ts @@ -8,11 +8,16 @@ * lost with no error anywhere — the failure this module exists to close. * * Design (deliberately narrow, see docs/remote-sessions.md §Wake-on-LAN): - * - ONLY real user input wakes a host. The auto-reconnect watcher and - * boot-recovery must never wake one, or a host would be re-woken ~45 s after - * each suspend and could never stay asleep (the "keepalive pings a sleeping - * host" failure already solved for a different consumer by - * `hufflepuff-mcp-lazy`). + * - An EXPLICIT request wakes a host, and nothing else: user input on an + * established session (`handleInput`), the wake button (`ensureAwake`), or the + * user's own session create/attach request (`ensureHostAwake`, wired in the HTTP + * routes). Everything that runs on a TIMER — the auto-reconnect watcher, boot + * recovery, the reachability probe, session discovery — must never wake one, or + * a host would be re-woken ~45 s after each suspend and could never stay asleep + * (the "keepalive pings a sleeping host" failure already solved for a different + * consumer by `hufflepuff-mcp-lazy`). The create path is deliberately wired in + * `session-routes.ts` and NOT in the shared session service, because + * `cron-service.ts` builds sessions there without a user waiting on the answer. * - Detection is a cheap TCP connect to the SSH port (no auth, no ssh client, * a few hundred bytes — below any meaningful activity threshold), throttled * per session. No SSH keepalive is added to the launch command: keepalives @@ -41,6 +46,17 @@ export const REMOTE_WAKE_PROBE_TIMEOUT_MS = 1_500; export const REMOTE_WAKE_READY_INTERVAL_MS = 1_500; /** Bounded wait for the host to come back after the wake command ran. */ export const REMOTE_WAKE_READY_TIMEOUT_MS = 90_000; +/** + * Budget for a wake that an HTTP REQUEST is waiting on (session create/attach). + * Deliberately shorter than {@link REMOTE_WAKE_READY_TIMEOUT_MS}: the dashboard is + * served through a reverse proxy whose default `proxy_read_timeout` is 60 s, so a + * 90 s wait would be cut off AT THE PROXY while the session was still being built — + * the browser reports a failure for a session that exists. The budget has to cover + * the WHOLE request, not just the wait: 40 s here + the 1.5 s reachability probe + + * the tmux prereq probe's own 15 s timeout = 56.5 s worst case, still under 60 s. + * A warm S3 resume measures ~12 s, so 40 s is >3× the observed wake. + */ +export const REMOTE_WAKE_REQUEST_READY_TIMEOUT_MS = 40_000; /** The wake command itself must not hang the wake flow. */ export const REMOTE_WAKE_COMMAND_TIMEOUT_MS = 10_000; /** @@ -224,7 +240,7 @@ export interface RemoteWakeDeps { /** Run the resolved wake target (magic packet or host command). Resolves false on failure. */ wake(target: NonNullable): Promise; /** Poll until the woken host accepts connections again. */ - waitUntilReady(remote: WakeableRemote): Promise; + waitUntilReady(remote: WakeableRemote, opts?: { timeoutMs?: number }): Promise; /** Sleep helper (injected for tests). */ delay(ms: number): Promise; /** Notify the COD-108 watcher so an exhausted backoff is reset. */ @@ -264,6 +280,25 @@ export function wakeConfigured(remote: WakeableRemote | undefined): WakeConfigur return target.kind; } +/** + * Outcome of waking a host for a caller that has NO session yet (the create/attach + * routes). A union rather than a boolean because the three cases need different + * handling: `'no-target'` must leave the caller's behavior byte-identical (no probe, + * no extra latency for a host without WoL), and only `'failed'` is an error that + * deserves its own message instead of the caller's usual one. + */ +export type HostWakeOutcome = 'no-target' | 'ready' | 'failed'; + +/** + * State key for a host-scoped wake. Prefixed so it can never collide with a session + * id, and keyed on the HOST rather than the case: two cases on one host share a + * single in-flight wake and one probe verdict. Such an entry is tiny (no input + * buffer) and bounded by the number of configured hosts, so it is never dropped. + */ +function hostWakeKey(hostId: string): string { + return `host:${hostId}`; +} + /** Per-session wake bookkeeping. */ interface WakeState { probedAt: number; @@ -391,6 +426,67 @@ export class RemoteWakeRegistry { return this.wake(session); } + /** + * Host-scoped reachability, for a caller that has no session yet (create/attach). + * Shares the per-HOST probe state with {@link ensureHostAwake}, so the probe the + * wake flow just paid for also answers "was that ssh failure really a sleeping + * machine?". Never wakes anything — it is a question, not an action. + */ + async checkHostReachable(remote: WakeableRemote, opts: { force?: boolean; ttlMs?: number } = {}): Promise { + const state = this._state(hostWakeKey(remote.hostId)); + const ttl = opts.force ? 0 : (opts.ttlMs ?? REMOTE_WAKE_REACHABILITY_TTL_MS); + if (Date.now() - state.probedAt >= ttl) { + state.probedAt = Date.now(); + state.reachable = await this.deps.probe(remote); + } + return state.reachable === true; + } + + /** + * Wake a host for a REQUEST that is waiting on it — the session create/attach + * routes, where there is no session to reattach and no input to buffer yet. + * + * `'no-target'` returns without probing, so a host without WoL config costs + * nothing and behaves exactly as before. Single-flight per host, so a double click + * (or two cases on the same host) sends one packet and shares one readiness poll. + */ + async ensureHostAwake(remote: WakeableRemote, opts: { timeoutMs?: number } = {}): Promise { + if (!resolveWakeTarget(remote)) return 'no-target'; + const state = this._state(hostWakeKey(remote.hostId)); + if (state.waking) return (await state.waking) ? 'ready' : 'failed'; + + state.probedAt = Date.now(); + state.reachable = await this.deps.probe(remote); + if (state.reachable) return 'ready'; + this.deps.log?.(`[RemoteWake] ${remote.label} (${remote.host}) is unreachable — waking it for a new session`); + return (await this.wakeHost(remote, opts)) ? 'ready' : 'failed'; + } + + /** + * Single-flight wake for a host with no session (see {@link ensureHostAwake}). + * A write into the same `waking` slot the session flow uses, so the two can never + * run two readiness polls against one host from the same key space. + */ + private async wakeHost(remote: WakeableRemote, opts: { timeoutMs?: number }): Promise { + const state = this._state(hostWakeKey(remote.hostId)); + if (state.waking) return state.waking; + state.waking = (async (): Promise => { + try { + return await this._wakeAndWait(remote, state, { timeoutMs: opts.timeoutMs, forNewSession: true }); + } catch (err) { + // Injected IO is documented not to throw, but a rejected promise here would + // surface as an unhandled rejection AND take the route down with it (the + // session path catches for exactly this reason). A broken wake target must + // fail the wake, never the create route beyond its own error response. + this.deps.log?.(`[RemoteWake] unexpected failure: ${err instanceof Error ? err.message : String(err)}`); + return false; + } finally { + state.waking = null; + } + })(); + return state.waking; + } + /** * Single-flight wake: probe-free (the caller already knows the host is down), * run the wake command, poll for readiness, reattach the pane, flush the buffer. @@ -405,29 +501,9 @@ export class RemoteWakeRegistry { state.waking = (async (): Promise => { const id = session.id; try { - this.deps.broadcast?.('remote:hostWaking', { sessionId: id, hostId: remote.hostId, label: remote.label }); - this.deps.log?.(`[RemoteWake] waking ${remote.label} (${remote.host}) via ${target.kind} for session ${id}`); + const ready = await this._wakeAndWait(remote, state, { sessionId: id }); + if (!ready) return false; - const woke = await this.deps.wake(target); - if (!woke) { - this.deps.log?.( - `[RemoteWake] wake failed for ${remote.label}: ${target.kind === 'command' ? target.command : 'magic packet'}` - ); - } - - const ready = await this.deps.waitUntilReady(remote); - if (!ready) { - this.deps.log?.(`[RemoteWake] ${remote.label} did not come back — input stays buffered`); - this.deps.broadcast?.('remote:hostWakeFailed', { sessionId: id, hostId: remote.hostId, label: remote.label }); - // Reset the probe state so the NEXT user input probes and retries - // instead of trusting a stale "down" verdict forever. - state.probedAt = 0; - state.reachable = undefined; - return false; - } - - state.reachable = true; - state.probedAt = Date.now(); const reattached = await session.reattachRemote(); if (!reattached) { this.deps.log?.(`[RemoteWake] ${remote.label} is up but the pane could not be reattached`); @@ -453,6 +529,57 @@ export class RemoteWakeRegistry { return state.waking; } + /** + * Broadcast + run the wake target + wait for SSH. Shared by the session flow (which + * then reattaches and flushes the buffer) and the create/attach flow (which has no + * pane yet). On failure the probe state is reset so the NEXT attempt probes and + * retries instead of trusting a stale "down" verdict forever. + */ + private async _wakeAndWait( + remote: WakeableRemote, + state: WakeState, + opts: { sessionId?: string; timeoutMs?: number; forNewSession?: boolean } = {} + ): Promise { + const target = resolveWakeTarget(remote); + if (!target) return true; + const forWhat = opts.sessionId ? `for session ${opts.sessionId}` : 'for a new session'; + // No `sessionId` for a create-path wake: the toast handler is then the only one + // that acts (a banner for a session that does not exist yet would have no target), + // which is exactly the `forNewSession` distinction the UI renders. + this.deps.broadcast?.('remote:hostWaking', { + ...(opts.sessionId ? { sessionId: opts.sessionId } : { forNewSession: true }), + hostId: remote.hostId, + label: remote.label, + }); + this.deps.log?.(`[RemoteWake] waking ${remote.label} (${remote.host}) via ${target.kind} ${forWhat}`); + + const woke = await this.deps.wake(target); + if (!woke) { + this.deps.log?.( + `[RemoteWake] wake failed for ${remote.label}: ${target.kind === 'command' ? target.command : 'magic packet'}` + ); + } + + const ready = await this.deps.waitUntilReady(remote, { timeoutMs: opts.timeoutMs }); + if (!ready) { + this.deps.log?.( + `[RemoteWake] ${remote.label} did not come back — ${opts.forNewSession ? 'the session was not started' : 'input stays buffered'}` + ); + this.deps.broadcast?.('remote:hostWakeFailed', { + ...(opts.sessionId ? { sessionId: opts.sessionId } : { forNewSession: true }), + hostId: remote.hostId, + label: remote.label, + }); + state.probedAt = 0; + state.reachable = undefined; + return false; + } + + state.reachable = true; + state.probedAt = Date.now(); + return true; + } + private _state(sessionId: string): WakeState { let state = this.states.get(sessionId); if (!state) { @@ -675,7 +802,7 @@ export function createDefaultRemoteWakeDeps(overrides: Partial = return { probe: probeRemoteHostReachable, wake: (target) => (target.kind === 'command' ? runRemoteWakeCommand(target.command) : sendWakePackets(target.macs)), - waitUntilReady: (remote) => waitUntilRemoteReady(remote), + waitUntilReady: (remote, opts) => waitUntilRemoteReady(remote, opts), delay, ...overrides, }; diff --git a/src/web/public/host-wake-ui.js b/src/web/public/host-wake-ui.js index 636c8df1..bade31d7 100644 --- a/src/web/public/host-wake-ui.js +++ b/src/web/public/host-wake-ui.js @@ -326,9 +326,16 @@ Object.assign(CodemanApp.prototype, { */ _onRemoteHostWaking(data) { const label = data && data.label ? data.label : 'Remote host'; + // A create-path wake (the user pressed Run / Attach) has no session yet, so + // nothing is queued behind it — the wording has to say what actually happens. + const forNewSession = Boolean(data && data.forNewSession); // Long enough to cover the wake + attach (~10s measured on a warm S3), and it // is replaced by `remote:sessionReconnected` the moment the pane is back. - this.showToast(`Waking ${label} … input is queued`, 'info', { duration: 12000 }); + this.showToast( + forNewSession ? `Waking ${label} … the session starts when it is back` : `Waking ${label} … input is queued`, + 'info', + { duration: 12000 } + ); const state = this._hostWake; if (!state || !data || state.sessionId !== data.sessionId) return; state.waking = true; @@ -340,7 +347,14 @@ Object.assign(CodemanApp.prototype, { /** SSE `remote:hostWakeFailed` — the host did not come back in time. */ _onRemoteHostWakeFailed(data) { const label = data && data.label ? data.label : 'Remote host'; - this.showToast(`${label} did not wake up — queued input is still held`, 'error', { duration: 15000 }); + const forNewSession = Boolean(data && data.forNewSession); + this.showToast( + forNewSession + ? `${label} did not wake up — no session was started` + : `${label} did not wake up — queued input is still held`, + 'error', + { duration: 15000 } + ); const state = this._hostWake; if (!state || !data || state.sessionId !== data.sessionId) return; state.waking = false; diff --git a/src/web/routes/session-routes.ts b/src/web/routes/session-routes.ts index 87d0dc27..c6ae27bd 100644 --- a/src/web/routes/session-routes.ts +++ b/src/web/routes/session-routes.ts @@ -28,6 +28,7 @@ import { type GrokConfig, type DeepSeekConfig, type OmpConfig, + type RemoteHost, } from '../../types.js'; import { Session, isAltScreenStripMode, isExternalCliMode, isMuxAltScreenOnlyStripMode } from '../../session.js'; import { SseEvent } from '../sse-events.js'; @@ -67,7 +68,12 @@ import { type WaitSignal, type SignalWaitResult, } from '../session-wait-registry.js'; -import { RemoteWakeRegistry, createDefaultRemoteWakeDeps } from '../../remote-wake.js'; +import { + RemoteWakeRegistry, + REMOTE_WAKE_REQUEST_READY_TIMEOUT_MS, + createDefaultRemoteWakeDeps, + type WakeableRemote, +} from '../../remote-wake.js'; import { clampWaitMs, MAX_BUFFER_SCAN_BYTES } from '../../config/agent-wait.js'; import { autoConfigureRalph, @@ -816,6 +822,22 @@ export function resolveOmpConfigForCreate( return resolvedId ? { ...ompConfig, resumeSessionId: resolvedId } : ompConfig; } +/** + * `RemoteHost` → the wake registry's host shape. They differ in one field name only + * (`id` in host config vs `hostId` on a session's `remote`), but the rename is load- + * bearing: the registry keys its per-host wake state on `hostId`. + */ +function wakeableHost(host: RemoteHost): WakeableRemote { + return { + hostId: host.id, + label: host.label, + host: host.host, + port: host.port, + wakeMac: host.wakeMac, + wakeCommand: host.wakeCommand, + }; +} + export function registerSessionRoutes( app: FastifyInstance, ctx: SessionPort & EventPort & ConfigPort & InfraPort & AuthPort & TabLayoutPort, @@ -928,6 +950,19 @@ export function registerSessionRoutes( const { hostId, remoteSessionName } = body.attachRemoteSession; const host = (await readRemoteHosts(CODEMAN_CONFIG_DIR)).find((item) => item.id === hostId); if (!host) return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Remote host not found'); + // An explicit wake request is the only thing that may wake a host, and the user + // pressing Attach IS one (see quick-start for the same gate, and + // `remote-wake.ts` for what must never call this). Without it a sleeping host + // answers with an ssh failure that blames anything but the machine being asleep. + const hostWake = await remoteWake.ensureHostAwake(wakeableHost(host), { + timeoutMs: REMOTE_WAKE_REQUEST_READY_TIMEOUT_MS, + }); + if (hostWake === 'failed') { + return createErrorResponse( + ApiErrorCode.OPERATION_FAILED, + `${host.label} did not come back after a wake-on-LAN request — nothing was attached` + ); + } workingDir = `${host.username}@${host.host}:${remoteSessionName}`; remote = toAttachedSessionRemote(host, remoteSessionName, workingDir); } @@ -3272,11 +3307,39 @@ export function registerSessionRoutes( ); } + // The user pressing "Run" on a case whose host is asleep IS an explicit wake + // request (docs/remote-sessions.md §Wake-on-LAN), and the tmux probe below would + // otherwise fail with "could not verify tmux on remote host …" — an ssh failure + // that blames tmux for a machine that is merely suspended. Wired HERE, in the HTTP + // route, and deliberately NOT in the shared session service: `cron-service.ts` + // builds sessions through the service, and a wake down there would re-wake the + // host on every schedule (the failure invariant #1 exists to prevent). + const hostWake = await remoteWake.ensureHostAwake(wakeableHost(host), { + timeoutMs: REMOTE_WAKE_REQUEST_READY_TIMEOUT_MS, + }); + if (hostWake === 'failed') { + return createErrorResponse( + ApiErrorCode.OPERATION_FAILED, + `${host.label} did not come back after a wake-on-LAN request — the session was not started` + ); + } + // tmux is a hard prerequisite on the remote host (the agent runs inside a remote // tmux server so it survives ssh drops). Probe before spawning so a missing tmux // surfaces a clear, structured error instead of a dead "tmux: command not found" pane. const tmuxCheck = await checkRemoteTmuxAvailable(host); if (!tmuxCheck.ok) { + // An unreachable host and a host without tmux fail the same way over ssh, so the + // probe's own message would send the user hunting for a tmux install. Ask the + // registry (which just probed, when it woke the host) which of the two it is. + if (!(await remoteWake.checkHostReachable(wakeableHost(host)))) { + return createErrorResponse( + ApiErrorCode.OPERATION_FAILED, + hostWake === 'no-target' + ? `${host.label} (${host.host}) is not reachable, and this host has no wake-on-LAN target — configure a MAC address or a wake command first` + : `${host.label} (${host.host}) is not reachable` + ); + } return createErrorResponse(ApiErrorCode.OPERATION_FAILED, tmuxCheck.error || 'remote host is missing tmux'); } diff --git a/test/remote-wake.test.ts b/test/remote-wake.test.ts index 8c018251..57443320 100644 --- a/test/remote-wake.test.ts +++ b/test/remote-wake.test.ts @@ -26,6 +26,7 @@ import { sendWakePackets, wakeConfigured, REMOTE_WAKE_PENDING_MAX_BYTES, + REMOTE_WAKE_REQUEST_READY_TIMEOUT_MS, type RemoteWakeDeps, type WakeableRemote, type WakeableSession, @@ -439,6 +440,98 @@ describe('RemoteWakeRegistry', () => { }); }); +// ========== Host-scoped wake (session create/attach) ========== + +describe('RemoteWakeRegistry — host-scoped wake for a request that waits on it', () => { + const hostRemote: WakeableRemote = { + hostId: 'hufflepuff', + label: 'Hufflepuff', + host: '192.168.50.137', + wakeMac: '04:d9:f5:80:c6:58', + }; + + it('does not even probe a host without a wake target (byte-identical to no feature)', async () => { + const h = harness({ remote: { hostId: 'x', label: 'X', host: '10.0.0.9' } }); + await expect(h.registry.ensureHostAwake(h.session.remote!)).resolves.toBe('no-target'); + expect(h.probe).not.toHaveBeenCalled(); + expect(h.wake).not.toHaveBeenCalled(); + }); + + it('reports ready without waking when the host already answers', async () => { + const h = harness({ remote: hostRemote }); + h.probe.mockResolvedValue(true); + await expect(h.registry.ensureHostAwake(hostRemote)).resolves.toBe('ready'); + expect(h.wake).not.toHaveBeenCalled(); + }); + + it('wakes a sleeping host and waits with the caller’s budget, not the 90 s default', async () => { + const h = harness({ remote: hostRemote }); + h.probe.mockResolvedValue(false); + + await expect( + h.registry.ensureHostAwake(hostRemote, { timeoutMs: REMOTE_WAKE_REQUEST_READY_TIMEOUT_MS }) + ).resolves.toBe('ready'); + + expect(h.wake).toHaveBeenCalledWith({ kind: 'mac', macs: [[4, 217, 245, 128, 198, 88]] }); + // The budget has to reach the readiness poll: the reverse proxy cuts a request at + // 60 s, so a create-path wake must not inherit the 90 s session default. + expect(h.waitUntilReady).toHaveBeenCalledWith(hostRemote, { + timeoutMs: REMOTE_WAKE_REQUEST_READY_TIMEOUT_MS, + }); + expect(h.events).toContain('remote:hostWaking'); + }); + + it('reports failed when the host never comes back, and probes again on the next attempt', async () => { + const h = harness({ remote: hostRemote }); + h.probe.mockResolvedValue(false); + h.waitUntilReady.mockResolvedValue(false); + + await expect(h.registry.ensureHostAwake(hostRemote)).resolves.toBe('failed'); + expect(h.events).toContain('remote:hostWakeFailed'); + + // The failure resets the probe verdict, so a second Run probes instead of + // trusting a stale "down" forever. + h.waitUntilReady.mockResolvedValue(true); + h.probe.mockClear(); + await expect(h.registry.ensureHostAwake(hostRemote)).resolves.toBe('ready'); + expect(h.probe).toHaveBeenCalled(); + }); + + it('single-flights two concurrent create-path wakes for the same host', async () => { + const h = harness({ remote: hostRemote }); + h.probe.mockResolvedValue(false); + let release: (value: boolean) => void = () => {}; + h.waitUntilReady.mockImplementation(() => new Promise((resolve) => (release = resolve))); + + const first = h.registry.ensureHostAwake(hostRemote); + const second = h.registry.ensureHostAwake(hostRemote); + await vi.waitFor(() => expect(h.wake).toHaveBeenCalledTimes(1)); + release(true); + + await expect(Promise.all([first, second])).resolves.toEqual(['ready', 'ready']); + // One magic packet for a double click, not two. + expect(h.wake).toHaveBeenCalledTimes(1); + }); + + it('checkHostReachable is a question, never an action', async () => { + const h = harness({ remote: hostRemote }); + h.probe.mockResolvedValue(false); + + await expect(h.registry.checkHostReachable(hostRemote)).resolves.toBe(false); + expect(h.wake).not.toHaveBeenCalled(); + }); + + it('reports failed instead of rejecting when the wake IO itself throws', async () => { + // A create route must answer with its own error, not a 500 from an unexpected + // rejection — the session flow catches for the same reason. + const h = harness({ remote: hostRemote }); + h.probe.mockResolvedValue(false); + h.wake.mockRejectedValue(new Error('udp socket exploded')); + + await expect(h.registry.ensureHostAwake(hostRemote)).resolves.toBe('failed'); + }); +}); + // ========== Wiring guard ========== const SRC = fileURLToPath(new URL('../src', import.meta.url)); @@ -468,4 +561,20 @@ describe('wake wiring guard', () => { expect(importers.sort()).toEqual([...allowed].sort()); }); + + it('wakes a host for a create/attach request ONLY from the HTTP route', () => { + // The create-path wake (`ensureHostAwake`) is a USER request, so it belongs to the + // HTTP route. `cron-service.ts` builds sessions through the shared service with + // nobody waiting on the answer, so a wake down there would power the host on for + // every schedule — the failure invariant #1 exists to prevent. Asserted across the + // source tree, so a future caller has to come through this test. + // `remote-wake.ts` names itself: that is the definition, not a caller, and the + // import guard above already pins the file to the route. + const allowed = new Set([join('web', 'routes', 'session-routes.ts'), 'remote-wake.ts']); + const callers = walkTs(SRC) + .filter((full) => /ensureHostAwake\s*\(/.test(readFileSync(full, 'utf-8'))) + .map((full) => relative(SRC, full)); + + expect(callers.sort()).toEqual([...allowed].sort()); + }); }); diff --git a/test/routes/session-routes.test.ts b/test/routes/session-routes.test.ts index 763a4f8c..1fa0bb3a 100644 --- a/test/routes/session-routes.test.ts +++ b/test/routes/session-routes.test.ts @@ -55,6 +55,7 @@ vi.mock('../../src/remote-hosts.js', async (orig) => { }); import { registerSessionRoutes } from '../../src/web/routes/session-routes.js'; +import { RemoteWakeRegistry, REMOTE_WAKE_REQUEST_READY_TIMEOUT_MS } from '../../src/remote-wake.js'; import { resolveTerminalHistoryConfig } from '../../src/config/terminal-history.js'; interface LocalHarness { @@ -62,6 +63,15 @@ interface LocalHarness { ctx: MockRouteContext; } +// Wake-on-LAN seam: the production registry opens a real TCP connection to the host +// and can run a real wake command, so every route registered here gets a fake one +// (the same seam `test/routes/session-remote-wake.test.ts` uses). Default: the host +// answers, so nothing ever wakes. +const wakeProbe = vi.fn(async () => true); +const wakeCommandRun = vi.fn(async () => true); +const wakeWaitUntilReady = vi.fn(async () => true); +let wakeRegistry: RemoteWakeRegistry; + /** * Build a Fastify instance that mirrors production's uniform-envelope behavior * (server.ts preSerialization hook) so the test wire format matches the contract: @@ -108,7 +118,17 @@ describe('session-routes', () => { let harness: LocalHarness; beforeEach(async () => { - harness = await createEnvelopeHarness(registerSessionRoutes); + wakeProbe.mockReset().mockResolvedValue(true); + wakeCommandRun.mockReset().mockResolvedValue(true); + wakeWaitUntilReady.mockReset().mockResolvedValue(true); + wakeRegistry = new RemoteWakeRegistry({ + probe: wakeProbe, + wake: wakeCommandRun, + waitUntilReady: wakeWaitUntilReady, + delay: async () => {}, + log: () => {}, + }); + harness = await createEnvelopeHarness((app, ctx) => registerSessionRoutes(app, ctx, { remoteWake: wakeRegistry })); // Reset remote store so tests start with empty hosts/cases and a passing tmux probe remoteStore.hosts = []; remoteStore.cases = []; @@ -1897,6 +1917,132 @@ describe('session-routes', () => { expect(JSON.parse(res.body)).toMatchObject({ success: false, errorCode: ApiErrorCode.OPERATION_FAILED }); }); + describe('remote create/attach wakes a sleeping host (Wake-on-LAN)', () => { + const host = (extra: Record = {}) => ({ + id: 'hufflepuff', + label: 'Hufflepuff', + host: '192.168.50.137', + username: 'j', + wakeMac: '04:d9:f5:80:c6:58', + ...extra, + }); + const remoteCase = { name: 'hufflepuff-work', type: 'remote', hostId: 'hufflepuff', remotePath: '/home/j/work' }; + const quickStart = () => + harness.app.inject({ + method: 'POST', + url: '/api/quick-start', + payload: { caseName: 'hufflepuff-work', mode: 'shell' }, + }); + + it('wakes the host before the tmux probe when the user runs a remote case', async () => { + const startShell = vi.spyOn(Session.prototype, 'startShell').mockResolvedValue(undefined); + try { + remoteStore.hosts = [host()]; + remoteStore.cases = [remoteCase]; + wakeProbe.mockResolvedValue(false); // asleep + + const res = await quickStart(); + + expect(res.statusCode).toBe(200); + expect(JSON.parse(res.body).success).toBe(true); + expect(wakeCommandRun).toHaveBeenCalledWith({ kind: 'mac', macs: [[4, 217, 245, 128, 198, 88]] }); + // The request budget, not the 90 s session default: the reverse proxy would + // cut the request at 60 s while the session was still being built. + expect(wakeWaitUntilReady).toHaveBeenCalledWith(expect.objectContaining({ hostId: 'hufflepuff' }), { + timeoutMs: REMOTE_WAKE_REQUEST_READY_TIMEOUT_MS, + }); + } finally { + startShell.mockRestore(); + } + }); + + it('does not wake a host that answers, and never probes a host without a wake target', async () => { + const startShell = vi.spyOn(Session.prototype, 'startShell').mockResolvedValue(undefined); + try { + remoteStore.hosts = [host()]; + remoteStore.cases = [remoteCase]; + // The fake probe answers `true` by default — a reachable host. + expect((await quickStart()).statusCode).toBe(200); + expect(wakeCommandRun).not.toHaveBeenCalled(); + + // No wake target at all: not even a probe, so hosts without WoL keep the + // exact behavior (and latency) they had before this feature. + wakeProbe.mockClear(); + remoteStore.hosts = [host({ wakeMac: undefined })]; + expect((await quickStart()).statusCode).toBe(200); + expect(wakeProbe).not.toHaveBeenCalled(); + expect(wakeCommandRun).not.toHaveBeenCalled(); + } finally { + startShell.mockRestore(); + } + }); + + it('refuses the run when the host never comes back, and starts no session', async () => { + remoteStore.hosts = [host()]; + remoteStore.cases = [remoteCase]; + wakeProbe.mockResolvedValue(false); + wakeWaitUntilReady.mockResolvedValue(false); + const sessionsBefore = harness.ctx.sessions.size; + + const res = await quickStart(); + + expect(res.statusCode).toBe(httpStatusForErrorCode(ApiErrorCode.OPERATION_FAILED)); + expect(JSON.parse(res.body).error).toMatch(/did not come back after a wake-on-LAN request/); + // No half-created session: the failure is the answer, not a dead tab. + expect(harness.ctx.sessions.size).toBe(sessionsBefore); + }); + + it('blames the sleeping host, not tmux, when the host has no wake target', async () => { + remoteStore.hosts = [host({ wakeMac: undefined })]; + remoteStore.cases = [remoteCase]; + remoteStore.tmuxCheck = { + ok: false, + error: 'remote host 192.168.50.137 needs tmux installed for durable remote sessions', + }; + wakeProbe.mockResolvedValue(false); + + const res = await quickStart(); + + expect(res.statusCode).toBe(httpStatusForErrorCode(ApiErrorCode.OPERATION_FAILED)); + expect(JSON.parse(res.body).error).toMatch(/has no wake-on-LAN target/); + }); + + it('keeps the tmux error when the host is up but tmux is really missing', async () => { + remoteStore.hosts = [host()]; + remoteStore.cases = [remoteCase]; + remoteStore.tmuxCheck = { + ok: false, + error: 'remote host 192.168.50.137 needs tmux installed for durable remote sessions', + }; + // Probe answers `true`: the ssh failure is genuinely about tmux. + + const res = await quickStart(); + + expect(JSON.parse(res.body).error).toMatch(/needs tmux installed/); + }); + + it('wakes the host when attaching to a discovered remote session', async () => { + const startInteractive = vi.spyOn(Session.prototype, 'startInteractive').mockResolvedValue(undefined); + const startShell = vi.spyOn(Session.prototype, 'startShell').mockResolvedValue(undefined); + try { + remoteStore.hosts = [host()]; + wakeProbe.mockResolvedValue(false); + + const res = await harness.app.inject({ + method: 'POST', + url: '/api/sessions', + payload: { attachRemoteSession: { hostId: 'hufflepuff', remoteSessionName: 'codeman-abc12345' } }, + }); + + expect(res.statusCode).toBe(200); + expect(wakeCommandRun).toHaveBeenCalledTimes(1); + } finally { + startInteractive.mockRestore(); + startShell.mockRestore(); + } + }); + }); + it('does not run local codex availability check for a remote codex case', async () => { // A remote codex case must NOT be blocked by the LOCAL codex availability gate // (the CLI runs on the remote host). Probe is stubbed ok in remoteStore.tmuxCheck.