From 035bfbc2fef36994bd43251624372a0043835c73 Mon Sep 17 00:00:00 2001 From: Codeman maintainer Date: Sat, 19 Sep 2026 12:07:57 +0200 Subject: [PATCH] fix(remote): merge-time fixes for Wake-on-LAN The MAC-count limit lived in two places that disagreed. RemoteHostSchema.wakeMac's 128-character cap admits seven comma-separated MACs while parseMacList takes at most four, all-or-nothing, so a five-MAC value validated, was written to remote-hosts.json, and then resolved to NO wake target: POST /api/sessions/:id/wake answered "No wake-on-LAN target configured for this host" and the banner offered "Configure WoL" for a host the user had just configured. MAX_WAKE_MACS now lives in src/config/remote-wake-limits.ts and both sides refine against it. Its own module because src/remote-wake.ts is import-fenced to session-routes.ts and server.ts (the wiring guard that stops a watcher waking a host), and because schemas.ts must not drag dgram/net/child_process into every request-validating module. The documented 40 s request budget also omitted the wake's own cost. A `command` target is bounded by REMOTE_WAKE_COMMAND_TIMEOUT_MS and runs BEFORE the readiness poll, so a slow one pushed a wakeCommand host's worst case to ~68 s, past the 60 s proxy_read_timeout the budget exists to stay under. _wakeAndWait now subtracts the wake's measured elapsed time from the readiness budget, floored at one poll interval so a wake that ate the whole budget still gets one probe. A magic packet is effectively instant and is unaffected, which is why live testing never saw it. Also: the two new endpoints are documented in docs/api-reference.md with the import fence stated as the rule it is, CLAUDE.md's frontend module count moves to 34, and the release changesets carry the Thanks section. Co-Authored-By: Claude Opus 5 (1M context) --- .changeset/release-thanks.md | 9 ++++ .changeset/remote-wake-on-lan.md | 18 +++++++ CLAUDE.md | 2 +- docs/api-reference.md | 14 +++++ src/config/remote-wake-limits.ts | 23 +++++++++ src/remote-wake.ts | 23 ++++++++- src/web/schemas.ts | 7 +++ test/remote-wake.test.ts | 89 +++++++++++++++++++++++++++++--- 8 files changed, 176 insertions(+), 9 deletions(-) create mode 100644 .changeset/release-thanks.md create mode 100644 .changeset/remote-wake-on-lan.md create mode 100644 src/config/remote-wake-limits.ts diff --git a/.changeset/release-thanks.md b/.changeset/release-thanks.md new file mode 100644 index 00000000..fc475716 --- /dev/null +++ b/.changeset/release-thanks.md @@ -0,0 +1,9 @@ +--- +"aicodeman": patch +--- + +### Thanks + +- @irisitymichaelgrundberg for three terminal fixes in one release: keeping the output a pane capture could not contain (#436), replaying a capture at the geometry it was taken at (#435, five rounds and a Playwright suite that fails against the merge base), and trimming the padding out of a copied selection (#451), where the scan-instead-of-regex call avoided a 2.9s freeze nobody would have traced back to a copy. +- @timkjr for a first contribution that found a real silent failure: the Instance count stepper next to the Run button had only ever applied to Claude, so on the other eight run modes it launched one session and said nothing (#454). +- @Randalix for Wake-on-LAN on remote hosts (#439), built and live-tested against a real sleeping machine, and for reading the whole diff again between rounds rather than only the parts that were asked about. diff --git a/.changeset/remote-wake-on-lan.md b/.changeset/remote-wake-on-lan.md new file mode 100644 index 00000000..ee2f8585 --- /dev/null +++ b/.changeset/remote-wake-on-lan.md @@ -0,0 +1,18 @@ +--- +"aicodeman": minor +--- + +feat(remote): wake a sleeping remote host from Codeman + +A remote SSH case pointing at a machine that suspends used to fail the same way every +time: the session was there, the host was not, and typing into it went nowhere. A host +can now carry a wake target, either a MAC address for Wake-on-LAN (Codeman builds the +magic packet itself, so nothing reaches a shell) or a wake command of your own, and +Codeman uses it when you ask for the host: when you type into a sleeping session, when +you press the wake button on the banner, or when you start or attach a session on that +host. Input you type while it wakes is buffered and flushed once it is back, up to 4 KB, +and a chunk over that is refused outright rather than delivered as a fragment. + +Waking only ever happens because you asked. No watcher, dropped-session handler or +boot-recovery path can reach it, since a machine woken by a reconnect watcher would come +back seconds after every suspend. diff --git a/CLAUDE.md b/CLAUDE.md index 739c8e08..0a9c0834 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -171,7 +171,7 @@ Codeman is a Claude Code session manager with web interface and autonomous Ralph | **Attachments** | `src/attachment-registry.ts`, `attachment-magic`, `generated-artifact-attachments`, `session-attachment-history`, `document-preview-cache`, `document-thumbnailer`, `document-conversion-limiter`, `config/attachment-guard` | See Key Patterns | | **Plan** | `src/plan-orchestrator.ts`, `src/prompts/*.ts`, `src/templates/` (`claude-md.ts` + `case-template.md`) | `templates/` holds the CLAUDE.md scaffold generated into new cases | | **Web** | `src/web/server.ts` ★, `sse-events.ts`, `routes/*.ts` (27 modules + barrel; `session-routes.ts` ★), `route-helpers.ts`, `ports/*.ts`, `middleware/auth.ts`, `schemas.ts`, `self-update.ts`, `plan-usage-latest.ts`, `ws-connection-registry.ts`, `heic-jpeg-converter.ts` + `heic-jpeg-worker.ts` | | -| **Frontend** | `src/web/public/app.js` (~6.9K lines, core) + 33 modules + `sw.js` (+ `voice-pcm-worklet.js`, fetched from JS, not in the load order) | See Frontend section for the load order, which is authoritative | +| **Frontend** | `src/web/public/app.js` (~6.9K lines, core) + 34 modules + `sw.js` (+ `voice-pcm-worklet.js`, fetched from JS, not in the load order) | See Frontend section for the load order, which is authoritative | | **Types** | `src/types/index.ts` (barrel) → 22 domain files; also `src/types.ts` root re-export | See `@fileoverview` in index.ts | ★ = Large, central file (>50KB) — read its `@fileoverview` first. All files have `@fileoverview` JSDoc — read that before diving in. Discovery aid: `grep -l '@fileoverview' src/web/routes/*.ts` lists all route modules; same grep works for `src/types/`, `src/web/public/*.js`. diff --git a/docs/api-reference.md b/docs/api-reference.md index b1b87894..3f9ee410 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -334,6 +334,20 @@ bare `{}`. With `wait`, the route blocks on the wake instead and answers sent") when the host never returns, rather than writing into the stalled pane and reporting `delivered:true` plus a timeout. +Two endpoints back that flow directly, both scoped to one session's remote host and +both refusing a session that is not remote (`400 INVALID_INPUT`): + +| Method | Path | Purpose | +| --- | --- | --- | +| `GET` | `/api/sessions/:id/reachability` | Whether the session's remote host answers SSH right now, plus whether a wake target is configured. Read-only: it never wakes. `{"reachable": true\|false\|null, "wakeConfigured": "mac"\|"command"\|"none"}`, where `null` means the answer is unknown (a proxied host, where a TCP probe proves nothing). | +| `POST` | `/api/sessions/:id/wake` | Wake the host and wait for it to accept SSH again, bounded by the request budget. `422 OPERATION_FAILED` when it does not come back; `400 INVALID_INPUT` with "No wake-on-LAN target configured for this host" when nothing is set. | + +⚠️ Waking is deliberately reachable only from an explicit user action (this route, a +session create/attach, or typing into a sleeping session). No watcher, dropped-session +handler or boot-recovery path may wake a host, or a suspended machine would be woken +again seconds after every suspend; `test/remote-wake.test.ts` pins that as an import +fence around `src/remote-wake.ts`. + ### Response All three nest the wait result under `data.wait`, so one client helper works against diff --git a/src/config/remote-wake-limits.ts b/src/config/remote-wake-limits.ts new file mode 100644 index 00000000..14b840de --- /dev/null +++ b/src/config/remote-wake-limits.ts @@ -0,0 +1,23 @@ +/** + * @fileoverview Limits shared between Wake-on-LAN parsing and its request schema. + * + * Its own module because `src/remote-wake.ts` is import-fenced: only + * `web/routes/session-routes.ts` and `web/server.ts` may import it, so that no + * watcher or boot-recovery path can WAKE a host (pinned by the wiring guard in + * `test/remote-wake.test.ts`). `web/schemas.ts` needs the same MAC-count limit and + * must not become a third importer, and it would drag `dgram`/`net`/`child_process` + * into every module that validates a request body. A plain constant satisfies both. + */ + +/** + * How many comma-separated MACs one `wakeMac` may carry. + * + * ⚠ Single source for `parseMacList()` and `RemoteHostSchema.wakeMac`. The two used + * to disagree: the schema's 128-character cap admits seven MACs while the parser + * rejected more than four all-or-nothing, so a five-MAC value validated, persisted to + * `remote-hosts.json`, and then resolved to NO wake target. The host read as + * unconfigured and the banner offered "Configure WoL" for a host the user had just + * configured, which is the worst shape a validation gap can take: accepted, stored, + * silently inert. + */ +export const MAX_WAKE_MACS = 4; diff --git a/src/remote-wake.ts b/src/remote-wake.ts index 884a83cb..6226c1fc 100644 --- a/src/remote-wake.ts +++ b/src/remote-wake.ts @@ -37,6 +37,7 @@ import { spawn } from 'node:child_process'; import dgram from 'node:dgram'; import net from 'node:net'; +import { MAX_WAKE_MACS } from './config/remote-wake-limits.js'; /** Minimum spacing between two reachability probes for the same session. */ export const REMOTE_WAKE_PROBE_MIN_INTERVAL_MS = 30_000; @@ -54,6 +55,10 @@ export const REMOTE_WAKE_READY_TIMEOUT_MS = 90_000; * 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. + * ⚠ The wake ITSELF counts against this, which the original arithmetic omitted: a + * `command` target can spend REMOTE_WAKE_COMMAND_TIMEOUT_MS before the readiness poll + * begins, which would have made the real worst case ~68 s. `_wakeAndWait` therefore + * subtracts the wake's measured elapsed time from this budget rather than adding to it. * 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; @@ -200,7 +205,7 @@ export function resolveWakeTarget(remote: WakeableRemote | undefined): WakeTarge * Parse a comma-separated MAC list into byte arrays. Pure; returns null when any * entry is malformed (all-or-nothing, so a typo cannot half-arm a host). */ -export function parseMacList(value: string, maxMacs = 4): number[][] | null { +export function parseMacList(value: string, maxMacs = MAX_WAKE_MACS): number[][] | null { const parts = value .split(',') .map((part) => part.trim()) @@ -669,6 +674,7 @@ export class RemoteWakeRegistry { }); this.deps.log?.(`[RemoteWake] waking ${remote.label} (${remote.host}) via ${target.kind} ${forWhat}`); + const wakeStartedAt = Date.now(); const woke = await this.deps.wake(target); if (!woke) { this.deps.log?.( @@ -676,8 +682,21 @@ export class RemoteWakeRegistry { ); } + // The request-scoped budget has to cover the WHOLE request, and the wake is part + // of it. A `command` target is bounded by REMOTE_WAKE_COMMAND_TIMEOUT_MS, so a slow + // one burned 10 s before the readiness poll even started and pushed a wakeCommand + // host's worst case to ~68 s, past the 60 s proxy_read_timeout this budget exists to + // stay under. A magic packet is effectively instant, so this subtracts nothing there. + // Floored at one poll interval so a wake that ate the whole budget still gets one + // probe rather than being declared unreachable without asking. + const wakeElapsedMs = Date.now() - wakeStartedAt; + const readyTimeoutMs = + opts.timeoutMs === undefined + ? undefined + : Math.max(REMOTE_WAKE_READY_INTERVAL_MS, opts.timeoutMs - wakeElapsedMs); + const ready = await this.deps.waitUntilReady(remote, { - timeoutMs: opts.timeoutMs, + timeoutMs: readyTimeoutMs, signal: this.shutdown.signal, }); if (!ready) { diff --git a/src/web/schemas.ts b/src/web/schemas.ts index c8c3a273..5570332f 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -19,6 +19,7 @@ import { } from '../config/terminal-history.js'; import { MAX_EDITABLE_BYTES } from '../config/file-editing.js'; import { MIN_MATCH_LENGTH, MAX_MATCH_LENGTH } from '../config/agent-wait.js'; +import { MAX_WAKE_MACS } from '../config/remote-wake-limits.js'; import { enabledCliIds, enabledClis } from '../config/cli-registry/registry.js'; import type { SessionMode } from '../types.js'; @@ -760,6 +761,12 @@ export const RemoteHostSchema = z.object({ /^[0-9a-fA-F]{2}([:-][0-9a-fA-F]{2}){5}(\s*,\s*[0-9a-fA-F]{2}([:-][0-9a-fA-F]{2}){5})*$/, 'Wake MAC must be one or more MAC addresses, comma-separated' ) + // ⚠ The character cap admits seven MACs while parseMacList takes at most + // MAX_WAKE_MACS, all-or-nothing. Without this the extra ones validated, persisted, + // and then resolved to NO wake target, so the host read as unconfigured. + .refine((value) => value.split(',').length <= MAX_WAKE_MACS, { + message: `Wake MAC accepts at most ${MAX_WAKE_MACS} comma-separated addresses`, + }) .optional(), }); diff --git a/test/remote-wake.test.ts b/test/remote-wake.test.ts index 43b2f820..f1fe7f06 100644 --- a/test/remote-wake.test.ts +++ b/test/remote-wake.test.ts @@ -31,11 +31,14 @@ import { waitUntilRemoteReady, wakeConfigured, REMOTE_WAKE_PENDING_MAX_BYTES, + REMOTE_WAKE_READY_INTERVAL_MS, REMOTE_WAKE_REQUEST_READY_TIMEOUT_MS, type RemoteWakeDeps, type WakeableRemote, type WakeableSession, } from '../src/remote-wake.js'; +import { RemoteHostSchema } from '../src/web/schemas.js'; +import { MAX_WAKE_MACS } from '../src/config/remote-wake-limits.js'; // ========== Pure decisions ========== @@ -564,6 +567,36 @@ describe('RemoteWakeRegistry', () => { // ========== Host-scoped wake (session create/attach) ========== +describe('the MAC-count limit lives in one place', () => { + // The schema's 128-character cap admits seven MACs while parseMacList takes at most + // MAX_WAKE_MACS, all-or-nothing. They used to disagree, so a five-MAC wakeMac + // validated, persisted to remote-hosts.json, and then resolved to NO wake target: + // the host read as unconfigured and the banner offered "Configure WoL" for a host + // the user had just set up. + const mac = (n: number) => `04:d9:f5:80:c6:${n.toString(16).padStart(2, '0')}`; + + it('parses exactly MAX_WAKE_MACS', () => { + const value = Array.from({ length: MAX_WAKE_MACS }, (_, i) => mac(i)).join(','); + expect(parseMacList(value)).toHaveLength(MAX_WAKE_MACS); + expect( + RemoteHostSchema.safeParse({ id: 'h', label: 'H', host: '10.0.0.5', username: 'joe', wakeMac: value }).success + ).toBe(true); + }); + + it('rejects one more in BOTH the schema and the parser, so neither can admit what the other drops', () => { + const value = Array.from({ length: MAX_WAKE_MACS + 1 }, (_, i) => mac(i)).join(','); + expect(parseMacList(value)).toBeNull(); + const parsed = RemoteHostSchema.safeParse({ + id: 'h', + label: 'H', + host: '10.0.0.5', + username: 'joe', + wakeMac: value, + }); + expect(parsed.success).toBe(false); + }); +}); + describe('isProbeable', () => { const base: WakeableRemote = { hostId: 'h', label: 'H', host: '10.0.0.9', wakeMac: '04:d9:f5:80:c6:58' }; @@ -750,15 +783,59 @@ describe('RemoteWakeRegistry — host-scoped wake for a request that waits on it 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, - // The shutdown signal rides along so `WebServer.stop()` can end the poll. - signal: expect.any(AbortSignal), - }); + // 60 s, so a create-path wake must not inherit the 90 s session default. A magic + // packet is effectively instant, so the poll gets essentially the whole budget; + // it is not asserted to the millisecond because the wake's own elapsed time is + // subtracted (see the wakeCommand case below). + const [, readyOpts] = h.waitUntilReady.mock.calls[0] as [unknown, { timeoutMs: number; signal: AbortSignal }]; + expect(readyOpts.timeoutMs).toBeLessThanOrEqual(REMOTE_WAKE_REQUEST_READY_TIMEOUT_MS); + expect(readyOpts.timeoutMs).toBeGreaterThan(REMOTE_WAKE_REQUEST_READY_TIMEOUT_MS - 1_000); + // The shutdown signal rides along so `WebServer.stop()` can end the poll. + expect(readyOpts.signal).toEqual(expect.any(AbortSignal)); expect(h.events).toContain('remote:hostWaking'); }); + it('spends a slow wake command out of the request budget rather than on top of it', async () => { + // REMOTE_WAKE_COMMAND_TIMEOUT_MS is 10 s and runs BEFORE the readiness poll, so the + // original arithmetic (40 s poll + 1.5 s probe + 15 s tmux prereq) understated a + // wakeCommand host's worst case by the whole wake: ~68 s against the 60 s + // proxy_read_timeout this budget exists to stay under. + const h = harness({ remote: { ...hostRemote, wakeMac: undefined, wakeCommand: '/usr/bin/whuff' } }); + h.probe.mockResolvedValue(false); + h.wake.mockImplementation(async () => { + await new Promise((r) => setTimeout(r, 60)); + return true; + }); + + await expect( + h.registry.ensureHostAwake( + { ...hostRemote, wakeMac: undefined, wakeCommand: '/usr/bin/whuff' }, + { timeoutMs: 5_000 } + ) + ).resolves.toBe('ready'); + + const [, readyOpts] = h.waitUntilReady.mock.calls[0] as [unknown, { timeoutMs: number }]; + expect(readyOpts.timeoutMs).toBeLessThan(5_000); + expect(readyOpts.timeoutMs).toBeGreaterThanOrEqual(5_000 - 2_000); + }); + + it('still gives a wake that ate the whole budget one readiness probe', async () => { + const h = harness({ remote: { ...hostRemote, wakeMac: undefined, wakeCommand: '/usr/bin/whuff' } }); + h.probe.mockResolvedValue(false); + h.wake.mockImplementation(async () => { + await new Promise((r) => setTimeout(r, 40)); + return true; + }); + + await h.registry.ensureHostAwake( + { ...hostRemote, wakeMac: undefined, wakeCommand: '/usr/bin/whuff' }, + { timeoutMs: 10 } + ); + + const [, readyOpts] = h.waitUntilReady.mock.calls[0] as [unknown, { timeoutMs: number }]; + expect(readyOpts.timeoutMs).toBe(REMOTE_WAKE_READY_INTERVAL_MS); + }); + 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);