Conversation
A durable remote session survives SSH drops (COD-104/108), but nothing brought the HOST back: after the remote machine suspended, the local tmux pane's ssh child stalled silently and `send-keys` SUCCEEDS against it, so typed input vanished with no error anywhere. Add an optional per-host `wakeCommand` (Wake-on-LAN wrapper, e.g. whuff) that the input route runs when a wake-enabled host is unreachable: input is buffered, the host is woken, the pane is reattached, and the buffer is flushed in order. Detection is a throttled bare TCP probe on wake-enabled hosts only, and only REAL user input may wake a host - the auto-reconnect watcher and boot recovery deliberately cannot, or the host would be re-woken seconds after every suspend and could never stay asleep.
…sions A session's remote block is persisted at launch time and recovery uses that snapshot, so a wakeCommand added to remote-hosts.json afterwards never reached an already-running session - not even across a Codeman restart (observed: the live Hufflepuff session came back with no wakeCommand). Merge the host-level field in on restore, with the host config authoritative.
… bulk delete Self-review pass: the input-ladder's two 'buffer' branches were the same three lines, and bulk delete left a session's (bounded, per-random-uuid) wake state behind. Documents the design where the code refers to it - remote-sessions.md section, the architecture invariant, and the CLAUDE.md key pattern.
…ke-on-LAN The reactive wake (typing into a session whose host slept) left the state invisible: nothing told the user the machine was asleep, and with no wake target configured there was nothing to do about it. Adds: - RemoteHost.wakeMac (comma-separated) - Codeman builds and broadcasts the magic packet itself (UDP port 9), so the common case needs no external script. The existing wakeCommand stays as the explicit override. - GET /api/sessions/:id/reachability - probes (throttled, cached, and it never wakes) and reports HOW the host can be woken, or that nothing is configured. - POST /api/sessions/:id/wake - wakes, waits, reattaches the pane and flushes buffered input; 400 with a routable message when no target is configured. - The amber host-unreachable banner + its 'Wake' / 'Configure WoL' action, and a small config dialog that saves via PUT /api/remote-hosts/:id. - RemoteWakeDeps.resolveRemote: host config is re-resolved for LIVE sessions (throttled + cached), so saving the dialog takes effect without a restart.
setBroadcast() on an unbound dgram socket throws EBADF on Linux and the following send fails with EACCES, so the magic packet silently never left the machine — the feature reported a wake that never happened. Caught by waking a real sleeping host (a unit test with a real UDP broadcast would not be welcome in CI, so the socket is injectable and the bind-before-setBroadcast ORDER is asserted).
A configured-but-broken target (host replaced NIC, command removed) had no way out: the dialog hung off the 'no target configured' branch only, so the banner would keep offering a Wake button that keeps failing.
…of tab switches Reported as 'the tab shows no banner' while the host was verifiably unreachable: the banner only started polling from selectSession, which RETURNS EARLY for the tab you are already on (so a page loaded with the remote tab active never polled), and a long-lived tab keeps running the JS it loaded — the feature was invisible to anyone who did not switch tabs after the deploy. The poller is now page-wide: one interval (created on init and on the first session switch), re-targeted whenever the active session changes, plus a visibilitychange wake-up. It no longer depends on any single selection path running. Also adds test/sse-dispatch-table.test.ts: a static guard that every [SSE_EVENTS.X, '_onFoo'] entry names an event constants.js defines AND a handler some module defines. Both halves fail silently (a typo'd constant is an undefined table key; a renamed handler just never runs), which is exactly how a new banner can never appear with no error anywhere.
… input cap Two findings from a final review pass over the wake-on-LAN feature. `_onRemoteHostWaking` / `_onRemoteHostWakeFailed` were defined in BOTH `panels-ui.js` (toasts) and `host-wake-ui.js` (banner). Both files mix into `CodemanApp.prototype` and `host-wake-ui.js` loads later, so the panels-ui copies were silently shadowed: the toast never fired, and a wake started for a BACKGROUND session (input on a non-active tab) produced no notification at all, since the banner handler only acts on the active session. The handlers now live only in `host-wake-ui.js`, show the toast unconditionally, and update the banner when the woken session is the active one. `appendBoundedPending` dropped only WHOLE chunks, so a single input value over the cap (one large paste is one `input` value, up to the 100 KB input schema) was kept in full: "bounded at 4 KB" held per chunk, not per session, and nothing was logged. The surviving chunk's head is now trimmed too, code-point aware so a multi-byte character is never split into a replacement char. Adds the guard that would have caught the first one: every SSE dispatch handler must be defined in exactly ONE frontend module. The existing test only asserts a handler EXISTS somewhere, which two modules both satisfy while one is shadowed.
Pressing Run on a remote case whose host was asleep failed with `could not verify tmux on remote host 192.168.50.137: …` — an ssh error that blames tmux for a machine that is merely suspended. The only wake paths were typed input on an established session and the banner's Wake button, so OPENING a session (the moment the user actually decides to use that host) had none. `RemoteWakeRegistry.ensureHostAwake()` reuses the existing probe/wake/readiness machinery for a host that has no session yet, and is wired into the two user-initiated create paths: `POST /api/quick-start` for a remote case (before the tmux prereq probe, which is what surfaced the misleading error) and `POST /api/sessions` with `attachRemoteSession`. A host without a wake target is not even probed, so its behavior and latency are byte-identical. The wake is blocking — the caller gets the session or an error — but bounded by REMOTE_WAKE_REQUEST_READY_TIMEOUT_MS (40 s) instead of 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 has to cover the whole request (40 s wake + 1.5 s probe + the tmux probe's own 15 s = 56.5 s worst case), which is why it is 40 s and not 45. A timeout now says the host did not come back, and an unreachable host without a wake target says so instead of pointing at tmux. The wiring is deliberately in the HTTP ROUTE, never in the shared session service: `cron-service.ts` builds sessions there with nobody waiting on the answer, and a wake on that path would power the host on for every schedule — the timer-driven re-wake invariant Ark0N#1 exists to prevent. Both halves are asserted (importers of `remote-wake`, and `ensureHostAwake` having exactly one caller file), so a future caller has to come through the guard test. A rejection from the wake IO is caught too: a broken target must fail the wake, not the route. `remote:hostWaking`/`remote:hostWakeFailed` now carry `forNewSession` for the session-less case, where "input is queued" would be untrue; the toast then reads "the session starts when it is back". Live wake numbers are unchanged (this reuses the measured ~12 s S3 path); the route behavior is covered by new tests in session-routes.test.ts with an injected registry, so no test opens a real socket or ssh.
One more wake path — asking before I push itI have one more commit for this branch ( The gapWaking was reachable from input and from the banner button, but not from opening a session — which is the moment you actually decide to use that host. i.e. an error that blames tmux for a machine that is merely asleep, with nothing in the log. What the commit does
Verification
Test sessions were deleted afterwards, and the remote tmux sessions with them (an attempt against a discovered, non-owned session detaches and leaves it alone).
No changeset, same as before. |
…ion too Found by driving the real UI: with a MAC configured in remote-hosts.json, removing it (here: to reach the "Configure WoL" dialog) changed nothing for a running session — _effectiveRemote short-circuited on the session's own snapshot whenever that snapshot HAD a target, so the resolver was only ever consulted in the one direction where the feature was missing. The documented "host config is authoritative" promise therefore failed in the direction a user can actually observe, and a wake target could live on invisibly after being deleted from the config. The resolver is now consulted on the TTL regardless, and wins for the wake fields in both directions. Also adds a route test for the browser's real input shape: one POST per keystroke, all buffered during a wake, replayed IN ORDER.
4a30f51 to
8dfc965
Compare
…ession refreshHostWakeBanner clears _hostWake before calling _hostWakeTick, so the clear branch's `if (this._hostWake)` guard skipped the repaint: once the banner had appeared for an unreachable remote session it stayed up on every chat (local ones included) until a reload, and the 30s ticker never cleared it either. Render unconditionally in that branch — _renderHostWakeBanner is idempotent with a null state. Reproduced in a real browser (Puppeteer, mobile viewport): state went null but banner.hidden stayed false. Regression test added in test/host-wake-banner.test.ts (red before, green after).
|
Updated the branch — it now carries three commits past the snapshot you have ( What changed since
|
|
Thanks for this, and for the live test against a real sleeping machine: the I ran typecheck, lint, format:check, check:frontend-syntax and check:public-assets (all clean) and the full suite: 387 files, 7324 tests, 12 skipped, exit 0. The invariant work is the part I want to call out as right: only an explicit user action can wake, and it is enforced by two source-tree wiring guards rather than a comment. I checked by hand that nothing in Nothing blocking. These are the things I would like fixed, and I am happy to do the first three at merge time if you would rather not round-trip. 1. Wake state is dropped on two of roughly eight cleanup paths ( 2. An oversized paste is head-trimmed and then executed as a fragment ( 3. The Wake button uses the 90 s budget your own comment rules out ( 4. An in-flight wake has no cancellation path and delays shutdown. 5. The banner promises queueing on a path that never queues ( Smaller things, and I will take these at merge unless you are touching the branch anyway:
One process note: you asked in the comment whether to push Fix 1 through 5 and I will merge. The rest I will fold in at merge time. |
Review follow-up on the wake-on-LAN PR (five findings, all of them about the state the feature keeps and the budgets it inherits): - Wake state is dropped by `WebServer.cleanupSession` instead of the two delete routes, so it now goes with the session on EVERY cleanup path (cron, admin, scheduled-run teardown, error paths) instead of surviving with up to 4 KB of the user's buffered keystrokes. `registerSessionRoutes` returns the registry so the server can own its lifetime without the wake-capable code living in `server.ts`; the wiring guard is updated to allow that and gains a second assertion that `server.ts` calls nothing but `drop`/`stop` on it. - `_effectiveRemote` returns before `_state`, so a LOCAL session no longer gets a wake-state entry — the input gate runs on every keystroke, so that entry used to be allocated for every session the user types in. - An input chunk larger than the 4 KB cap is dropped OUTRIGHT instead of being head-trimmed and then written as a fragment: one paste is one `input` value and was never typed character by character, so its tail is a partial command the user never sent. The drop is logged. - The manual wake button passes `REMOTE_WAKE_REQUEST_READY_TIMEOUT_MS` (40 s) like the create/attach paths, instead of inheriting the 90 s session default that the dashboard's reverse proxy cuts off at 60 s. - `RemoteWakeRegistry.stop()` aborts in-flight readiness polls (abortable sleep) and refuses new wakes, and `WebServer.stop()` calls it, so a restart during a wake no longer waits the poll out. - The banner/toast wording keys off a new `queuedInput` flag on the two SSE events, which is true only when the server actually holds bytes: browser keystrokes travel over the WebSocket, which never passes through the registry, so the wake BUTTON must not promise queued input. The failed-wake path also stops pattern-matching the error message (it re-asks the reachability route) and the WoL dialog says "admin-only" instead of "host not found" for a non-admin in multi-user mode.
- `host-wake-ui.js` joins the documented load order (12.2) and gets its `@dependency`/`@loadorder` tags; the frontend module count is 33, not 32. - `remote-wake` is not "(pure)" — the module uses `dgram`/`net`/`child_process`. - SSE counts: 160 constants, and the category is "Remote auto-reconnect / wake (5)"; the route table's per-file counts are refreshed (sessions 37, cases 34). - The CLAUDE.md wake rule now names the create/attach wake, the 40 s request budget, the whole-chunk paste drop, the registry's lifetime (drop on cleanup, stop on shutdown) and the deliberately non-wake-aware WebSocket keystroke path — that paragraph is what the next person reads. - Reverted the eight lines of unrelated Prettier markdown churn in `docs/architecture-invariants.md` (docs/ is not in the format glob, so it was an editor): only the new wake paragraph remains in the diff.
|
Pushed 1. Wake state lifetime.
2. Oversized paste. 3. Wake button budget. 4. Cancellation. 5. Wording. Both SSE events now carry Smaller ones taken: the failed-wake handler no longer pattern-matches the error string (it re-asks Left for you (I did not want to decide these unilaterally):
Verification.
|
… wake fields
Own review pass over the PR:
- `_flush` took the chunk out of the buffer only AFTER awaiting the write. Input
arriving during that await is enqueued (`waking` is still set, so it takes the
buffer path), and the 4 KB cap then drops the OLDEST chunk — which is the one
already on its way to the pane. The `shift()` that followed removed the NEXT
chunk instead, so the drop-oldest bookkeeping silently lost a chunk that was
never written, while the log line blamed the one that was. The chunk is now
removed before the await and re-inserted at the FRONT on a failed write, so the
order of the queue behind it is preserved. Regression test: a chunk enqueued
during the first write of a full buffer must still reach the pane (red against
the old order).
- `showCreateCaseModal()` reset the remote-host form fields but not the two new
wake inputs, so one host's MAC/command carried over into the next host that
form saved.
- The banner's pre-poll `wakeConfigured` labelled a command-only host as 'mac'.
Nothing reads the distinction, but the field is documented as which path is
configured, so it says the truth until the first poll corrects it.
- Stale `resolveRemote` comment ("only for sessions that have no usable target of
their own"): after the host config became authoritative in both directions it is
consulted on the TTL regardless.
|
Follow-up: I read the whole diff against 1. The flush could lose a chunk that was never written. 2. 3. The banner's pre-poll Verification unchanged in shape: |
2 participants
What this does
Adds Wake-on-LAN for remote (SSH) sessions, so a sleeping host can be woken instead of silently swallowing input.
A remote host is armed by either of two
RemoteHostfields inremote-hosts.json:wakeMac(new, the normal case) — a comma-separated MAC list. Codeman builds and sends the magic packet itself overdgram(UDP 9, broadcast), dependency-free.wakeCommand(kept) — a single executable path, run without a shell (spawn(command, [], { stdio: 'ignore' })), for waking via a router, another machine, or a script. It takes precedence overwakeMacwhen both are set.Two triggers, both gated on a real user action:
POST /api/sessions/:id/inputprobes the host (bare TCP connect, throttled to one probe per 30 s per session, wake-enabled hosts only) and, if it is down, wakes it, callsreattachRemote(), and flushes buffered input in order.GET /api/sessions/:id/reachability(probe only, never wakes) andPOST /api/sessions/:id/wake. With no wake target configured, the button opens the existing host-config dialog pre-filled for that host.Closes #433.
The two invariants
#1 — only user input or an explicit wake request may wake a host. The auto-reconnect watcher (COD-108),
handleRemoteSessionDroppedand boot recovery have no access to the registry. A wake there would re-wake the host seconds after every suspend, so it could never stay asleep — a worse bug than the one this fixes. This is asserted as a wiring guard (asrc/scan intest/remote-wake.test.ts), not a comment.GET …/reachabilityprobes and never wakes (regression test: probe a sleeping fake host → the wake spy stays empty).#2 — no
ServerAliveInterval. Keepalives would move bytes into an otherwise idle connection every interval, which is exactly what the byte-threshold idle detector on the sleeping host must not read as activity. The probe (~200 B / 30 s) sits far below it and cannot wake anyone by SYN. The reasoning paragraph lives indocs/architecture-invariants.md, where you asked for it, so the next person does not "fix" the stalled pane by adding keepalives.wakeCommandgatingThe config dialog writes through
PUT /api/remote-hosts/:id, which is already admin-only in multi-user mode (adminOnly,case-routes.ts:682) — same as the rest of the host config — so a non-admin cannot name an executable for the server to run. The schema accepts a single executable path only (no arguments, no$/backticks), and it is spawned withshell: false.Pending buffer
[RemoteWake] pending buffer cap reached for session … — oldest input dropped). Keeping the tail preserves what the user just typed; a silently unbounded buffer keyed on user input would be a memory leak.The
dgramfinding (in the code, not just the commit message)setBroadcast()on an unbound socket throwsEBADFon Linux, the followingsendfails withEACCES, and the function reports success — the magic packet never left the machine and it looked like "the host just did not come back". The order is load-bearing, so the broadcast flag is set inside thebindcallback, with a⚠️comment at the call site (src/remote-wake.ts). The socket is injectable so a test asserts thebind → setBroadcastorder (a real UDP broadcast in CI would be unwelcome). This is the bug the live test caught; unit tests with mocks would never have found it.Testing
test/remote-wake.test.ts(decision table, single-flight, buffer order + drop-oldest, wiring guard, socket order),test/routes/session-remote-wake.test.ts(route behavior),test/sse-dispatch-table.test.ts(frontend SSE dispatch guard).test/remote-hosts.test.ts(schema + rehydration),test/mocks/mock-session.ts.tsc --noEmit, eslint, prettier,check:frontend-syntax,check:public-assets— clean.npm test: 7303 passed, 15 skipped.POST /api/sessions/:id/wakereturned{woke:true, reachable:true}in 12 s including reattach, and the durable remote tmux session — and the agent conversation — survived the suspend. The banner was verified in a real browser (appears, Wake → "Waking…" → gone in 10.0 s, no console errors).test/quick-start.test.tsis red in my environment only: it binds127.0.0.1:3100, which a local Docker container already holds (EADDRINUSE). Unrelated to this change — the port comes fromTEST_PORT + 1off 3099, which is why grepping the file for3100finds nothing. Flagging it in case the suite is not fully green for you either.Review pass over the branch (commit
8dfc965d) found and fixed two things, both worth knowing since they are the silent-failure kind:_onRemoteHostWaking/_onRemoteHostWakeFailedwere defined in two frontend modules (panels-ui.jsfor the toast,host-wake-ui.jsfor the banner). Both mix intoCodemanApp.prototypeandhost-wake-ui.jsloads later, so the toast copy was silently shadowed — and a wake for a background session produced no notification at all. The handlers now live only inhost-wake-ui.js.appendBoundedPendingdropped only whole chunks, so a single input value over the cap (one large paste is oneinput, up to 100 KB by the input schema) was kept in full — "bounded at 4 KB" held per chunk, not per session. The surviving chunk's head is now trimmed, code-point aware.Both now have a guard: every SSE dispatch handler must be defined in exactly one module (the existing test only asserted one exists somewhere), and a test asserts a single oversized chunk is trimmed rather than kept.
No changeset, per your note.