Skip to content

feat(bridge): opt-in remote daemon attach for cross-machine sessions - #53

Open
doug-w wants to merge 1 commit into
puritysb:masterfrom
doug-w:feat/remote-daemon-attach
Open

feat(bridge): opt-in remote daemon attach for cross-machine sessions#53
doug-w wants to merge 1 commit into
puritysb:masterfrom
doug-w:feat/remote-daemon-attach

Conversation

@doug-w

@doug-w doug-w commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Summary

Opt-in remote daemon attach: let a session bridge (agentdeck claude / codex / opencode) attach to a daemon on another machine and be driven from that machine's single Stream Deck — for running agents on several boxes (often over SSH) with one deck on a main node. Default behavior is byte-for-byte unchanged (local-only).

Closes #24.

Opt-in contract (review blocker 1)

--remote-daemon / AGENTDECK_REMOTE_DAEMON=1 is the opt-in switch and gates both remote paths. --daemon-host <host[:port]> (env AGENTDECK_DAEMON_HOST, alias AGENTDECK_REMOTE_DAEMON_HOST) only names the endpoint and requires the switch — the recommended explicit form is --remote-daemon --daemon-host mainnode.lan. A host hint without the switch is inert and the CLI warns pre-PTY (post-PTY stdout is swallowed); the warning condition shares one derivation helper (deriveRemoteAttachOpts) with the resolver so the two cannot drift. An unreachable or incapable named host returns null with no mDNS fallthrough. With no opt-in, nothing ever leaves the machine.

Reverse control — same-socket ONLY

The worker's outbound push socket (DaemonWsClient → daemon) is bidirectional. When the daemon focuses a remote session it sends command frames down the socket the worker already opened — the daemon never dials back. A NAT'd / SSH-only worker needs only a single outbound connection and no inbound reachability.

  • Frames: session_focus_down / session_unfocus_down (daemon → worker), session_command_down (daemon → worker, wraps a PluginCommand), session_event_up (worker → daemon, relayed BridgeEvent).
  • Inbound commands run through the same applyPluginCommand handler as the local WS server — local and remote control never diverge.
  • Dial-back was removed entirely (review blocker 5): the per-session callback token, WsServer extra-accepted-token surface, and the focus relay's remote dial path are gone. The machine token is the only accepted WS credential again. A remote session whose push socket dropped is unreachable until its worker reconnects.

Remote classification (review blocker 2)

session_push_register carries an explicit remoteAttach: true flag — sent when the user opted in AND the resolved daemon advertises the sameSocketControl capability. The daemon trusts the flag over the socket's source IP, so the documented ssh -L 9120:localhost:9120 topology (worker arrives on loopback) registers, lists, and focuses correctly. The non-local-IP heuristic remains as back-compat for older workers.

Sender-identity guards (review blocker 3)

Registration, state updates, session_event_up ingestion, and close-teardown are keyed to the session's registered sender socket: a stale socket cannot delete or clobber a newer registration. A reconnecting worker's new socket takes over focus atomically (SessionFocusRelay.migrateSender — old sender unfocused best-effort, session_focus_down re-sent down the new socket). Workers ignore down-frames whose sessionId is not their own.

Capability negotiation (review blocker 7)

The Node daemon's /health advertises sameSocketControl: true. The explicit-host path refuses (null + one-shot PTY-visible warning) a daemon lacking it; the mDNS confirm step filters such candidates and propagates the flag on those it keeps. The Swift (macOS App Store) daemon does not implement the push-channel control frames and is therefore never selected as a remote hub; docs are scoped to the Node CLI daemon.

Also fixed

  • Pairing token no longer logged (redacted connect line: host:port + token yes/no) — blocker 4.
  • Self-port guard is host-aware (isLoopbackHost(host) && port === core.port), so a remote daemon on the same numeric port is no longer rejected — blocker 6.

Trust boundary (explicit, per review discussion)

/health is unauthenticated and serves the pairing token — any LAN peer can self-serve credentials, so remoteAttach is spoofable by a paired-tier client, and takeover-by-re-register is inherently allowed (legitimate reconnects require it). This matches the pre-existing "paired = trusted" model (a paired client can already send commands). The sender-identity guards target stale-close races, not hostile LAN peers.

Final-round lifecycle fixes (2026-07-31 review)

The three gaps from the final lifecycle pass are fixed (details in the point-by-point comment):

  1. Stale cached target: DaemonWsClient now replaces its cached target with the provider result on every resolution, null included — a vanished or downgraded daemon stops being dialed until a valid target resolves again. Plus a client-side capability guard under remote intent (defense against resolver drift).
  2. Loopback capability gate: under --remote-daemon, the local-first branch also requires sameSocketControl — an incapable daemon behind ssh -L now gets the one-shot refusal (no host hint) or falls through to the explicitly named host (with --daemon-host), never a silent plain registration. A capable local daemon still wins over --daemon-host (pre-existing precedence, unchanged).
  3. Sender-owned aggregator cache: when a remote registration exists, session_push_state is accepted only from the registered sender socket for the whole update — shared push-state cache included, so a stale pre-reconnect socket can't poison the deduped local row. Pure-local pushes are untouched.

REVERSE_RELAYED_EVENTS was also removed — both ends of the push socket now import the one canonical RELAYED_EVENTS set.

Tests

Extracted the push-channel handling into bridge/src/session-push-channel.ts (narrow injectable deps) so the real registration/state/event path is integration-tested: ssh -L loopback registration → listable → focusable; overlapping old/new sockets with focus migration; session_event_up ownership rejection; unconditional ack. Plus: gate-flip regressions (host hint without switch never probes or browses), capability refusal + mDNS filter/propagation tests, worker foreign-sessionId guard over real sockets, stale-socket guards in the registry, and the same-socket E2E. The final-round fixes add 9 regressions: stale-target lifecycle over real sockets (null + incapable-via-real-resolver + client guard), loopback capability enforcement (refusal, hostHint fallthrough, ssh -L tunnel shape, remote: false untouched, warn dedup), and the sender-ownership overlap proof (old socket can change neither the remote registry nor the effective push-state cache; stranger rejected; pure-local path preserved). Full suite green apart from known Windows-host-only failures in files this branch doesn't touch.

Verification

Validated end-to-end on the real two-machine rig (Windows 11 main node + macOS worker, both on the remediated build): the full T1–T8 matrix in the verification comment — baseline local-only unchanged, inert host hint + pre-PTY warning, explicit remote attach with full deck reverse control, token redaction, ssh -L loopback registration, worker-restart focus migration, capability refusal of a pre-remediation daemon, and mDNS auto-discovery. The final-round fixes change no transport behavior (frames, wire format, and registration untouched), so per the reviewer's stated bar they are covered by the focused regressions + CI rather than a rig re-run.

🤖 Generated with Claude Code

@puritysb puritysb left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same-socket design and test coverage look solid, but the documented opt-in invariant is not currently enforced. startSession() always passes hostHint to resolveDaemonTarget(), and that function processes an explicit host before checking opts.remote. As a result, --daemon-host / AGENTDECK_DAEMON_HOST alone enables a remote connection even when --remote-daemon is absent. This conflicts with the issue decision and PR claim that all remote behavior is gated behind the explicit opt-in switch. Please either gate explicit-host resolution on remote === true and add a regression test, or deliberately change the policy/docs/issue contract so --daemon-host itself is the opt-in. Given the security boundary, this should be unambiguous and tested.

doug-w pushed a commit to doug-w/AgentDeck that referenced this pull request Jul 12, 2026
Address PR puritysb#53 review: the documented opt-in invariant was correct in
behavior but not self-evident or regression-tested. `resolveDaemonTarget`
processed an explicit host before the `remote` switch, which read as
"host alone silently enables remote."

Clarify the contract in code and docs: the two remote paths each require
a distinct, user-typed opt-in and neither runs on ambient state —
an explicit `--daemon-host` *is itself* the opt-in (a named node, never a
silent auto-attach), while mDNS auto-discovery is gated solely behind the
`--remote-daemon` switch. With neither and no local daemon, resolution
returns null (local-only, unchanged).

- Inject findLocal/probeHealth/discover deps into resolveDaemonTarget so
  the precedence + opt-in gate are unit-testable without the network.
- Add `resolveDaemonTarget precedence + opt-in gate` regression tests: local
  wins and skips remote; explicit host opts in without touching mDNS;
  unreachable named host returns null (no LAN fallthrough); mDNS runs ONLY
  when remote===true; no opt-in never leaves the machine.
- Document the security-boundary gate in docs/daemon.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FeDdptNxWmPfGK4Vph6VUk
@doug-w

doug-w commented Jul 12, 2026

Copy link
Copy Markdown
Contributor Author

Addressed in dbef6b4. Made the opt-in gate explicit, self-documenting, and regression-tested rather than changing runtime behavior — because the shipping docs already treat an explicit host as its own opt-in (README lists --daemon-host mainnode.lan as the standalone recommended invocation; docs/daemon.md says --remote-daemon and/or --daemon-host). The mismatch was only with the PR-body/issue-§4 phrasing "gated behind --remote-daemon".

The contract is now stated unambiguously — two distinct, user-typed opt-ins, neither firing on ambient state:

  • Explicit host (--daemon-host / AGENTDECK_DAEMON_HOST) is itself the opt-in — the user named a specific node, so it's never a silent auto-attach. An unreachable named host returns null instead of falling through to mDNS.
  • mDNS auto-discovery is the only path that could reach "whatever hub is on the LAN", so it's gated solely behind --remote-daemon (remote === true) and can't run on any other signal.
  • With neither opt-in and no local daemon, resolveDaemonTarget returns null → local-only, byte-for-byte unchanged.

resolveDaemonTarget now takes injectable findLocal/probeHealth/discover deps so the precedence + gate are unit-tested without the network. New resolveDaemonTarget precedence + opt-in gate block asserts: local wins and skips both remote paths; explicit host opts in without touching mDNS; unreachable named host → null with no LAN fallthrough; mDNS runs ONLY when remote === true; no opt-in never leaves the machine. Gate documented in docs/daemon.md. Full suite green (1726 passed).

If you'd rather go the other way — require --remote-daemon even alongside --daemon-host — say so and I'll flip the gate + update the README examples; but that would break the currently-recommended standalone --daemon-host invocation, so I kept the explicit-host-is-opt-in reading.

@puritysb puritysb left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for clarifying the intended gate and for adding the regression tests. I re-reviewed the full PR at dbef6b4, including the real-socket tests. The same-socket direction remains sound, but there are several merge-blocking issues that are not covered by the current suite:

  1. The agreed opt-in contract is still not enforced. In issue #24 I explicitly agreed to keep --remote-daemon as the opt-in switch and --daemon-host as the explicit endpoint, and later reiterated the --remote-daemon + local-default invariant. resolveDaemonTarget() still processes hostHint when remote !== true. The standalone --daemon-host README example is new in this PR, so it cannot be used as the pre-existing contract. Please require remote === true for both remote paths and document the explicit form as --remote-daemon --daemon-host mainnode.lan; explicit-host failure should still return null without mDNS fallback.

  2. The documented SSH local-forward topology is not registered as remote. session_push_register only calls registerRemoteSession when !isLocalConnection(senderIp). With ssh -L 9120:localhost:9120 mainnode and --daemon-host 127.0.0.1, the main-node daemon sees the push socket as loopback, skips remote registration, and the worker is not in that machine's sessions.json, so it never becomes listable/focusable. The E2E test uses a raw WsServer harness and does not exercise this daemon-side classification. Please add an integration test that goes through the real registration handler for the SSH-forward/loopback case.

  3. A stale socket can delete a newer registration. Re-registering the same sessionId replaces sender, but every socket installs an unconditional close handler that removes by sessionId alone and calls handleSenderClosed(sessionId). If the new socket registers before the old one finishes closing, the old close removes/unfocuses the new connection. Removal and focus teardown need sender identity checks, plus an overlapping old/new socket regression test. The same ownership check should be applied to session_event_up, and down-frames should be ignored by a worker when msg.sessionId !== this.sessionId.

  4. The daemon pairing token is written to debug logs. DaemonWsClient logs the complete URL after appending ?token=.... Please log only the redacted host/port.

  5. Dial-back is described as opt-in but is enabled for every session. startSession() unconditionally creates and registers an extra accepted callback token, including the default local-only path. In addition, the remote registry is removed when the push socket closes, so the advertised no-live-sender fallback is effectively unavailable after disconnect. Please either remove this fallback for now or put it behind an explicit, tested opt-in and make its lifecycle reachable.

  6. Remote targets are rejected by port number alone. resolved.port !== core.port drops a valid daemon on another host whenever it happens to use the same numeric port as the worker session. The self-port guard must also require a loopback/local host.

This does not require implementing the feature in the Swift daemon if the intended main node is the Node CLI daemon. However, the docs currently say only "daemon", and discovery accepts any /health response with mode === 'daemon'; a worker can therefore attach to the App Store Swift daemon, register successfully, and then silently lack the new same-socket control frames. Please scope the docs to the Node CLI daemon and preferably require an advertised /health capability such as sameSocketControl so unsupported daemons are not selected.

Verification on the PR branch: TypeScript --noEmit passed and the full Vitest suite passed (100 files, 1726 tests). GitHub currently has no check runs for this head. The branch also conflicts with current master in bridge/src/daemon-server.ts and bridge/src/ws-server.ts; please rebase carefully because both files have gained recent steering/topology behavior.

@puritysb puritysb added changes-requested Review feedback must be addressed before merge enhancement New feature or request platform:bridge Node bridge, daemon, hooks, and CLI labels Jul 18, 2026 — with ChatGPT Codex Connector

Copy link
Copy Markdown
Owner

Moving this PR back to Draft to match its actual state. The same-socket design and two-machine validation remain the right foundation, but the current head is 133 master commits behind, conflicts in bridge/src/daemon-server.ts and bridge/src/ws-server.ts, has no GitHub checks, and still has unresolved merge blockers:

  1. enforce the agreed --remote-daemon opt-in contract;
  2. register SSH local-forward/loopback workers correctly;
  3. guard registration/event/close handling by sender identity;
  4. redact the daemon pairing token from logs;
  5. remove or explicitly gate and make dial-back lifecycle-reachable;
  6. make the self-port guard host-aware;
  7. negotiate/document daemon same-socket capability, including Swift-daemon behavior.

Please rebase on current master, add the requested integration regressions, and rerun the real Windows-main/Mac-worker scenario before marking ready for review again. Issue #24 remains open and assigned to you.

@puritysb
puritysb marked this pull request as draft July 18, 2026 06:13
@puritysb

Copy link
Copy Markdown
Owner

Friendly ping — no new commits on feat/remote-daemon-attach since 2026-07-12, and the 2026-07-18 draft review still stands. The branch is now even further behind master and conflicts in bridge/src/daemon-server.ts and bridge/src/ws-server.ts.

The seven blockers from that review remain open:

  1. enforce the agreed --remote-daemon opt-in contract;
  2. register SSH local-forward/loopback workers correctly;
  3. guard registration/event/close handling by sender identity;
  4. redact the daemon pairing token from logs;
  5. remove — or explicitly gate and make dial-back lifecycle-reachable;
  6. make the self-port guard host-aware;
  7. negotiate/document daemon same-socket capability, including Swift-daemon behavior.

If you can rebase onto current master, add the requested integration regressions, and rerun the real Windows-main/Mac-worker scenario, I'll re-review. Otherwise I'll close this PR as stale by 2026-08-01 to keep the PR queue accurate. Issue #24 stays open and assigned to you so the work isn't lost, and you're welcome to resubmit whenever it's ready.

doug-w pushed a commit to doug-w/AgentDeck that referenced this pull request Jul 28, 2026
Address PR puritysb#53 review: the documented opt-in invariant was correct in
behavior but not self-evident or regression-tested. `resolveDaemonTarget`
processed an explicit host before the `remote` switch, which read as
"host alone silently enables remote."

Clarify the contract in code and docs: the two remote paths each require
a distinct, user-typed opt-in and neither runs on ambient state —
an explicit `--daemon-host` *is itself* the opt-in (a named node, never a
silent auto-attach), while mDNS auto-discovery is gated solely behind the
`--remote-daemon` switch. With neither and no local daemon, resolution
returns null (local-only, unchanged).

- Inject findLocal/probeHealth/discover deps into resolveDaemonTarget so
  the precedence + opt-in gate are unit-testable without the network.
- Add `resolveDaemonTarget precedence + opt-in gate` regression tests: local
  wins and skips remote; explicit host opts in without touching mDNS;
  unreachable named host returns null (no LAN fallthrough); mDNS runs ONLY
  when remote===true; no opt-in never leaves the machine.
- Document the security-boundary gate in docs/daemon.md.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FeDdptNxWmPfGK4Vph6VUk
@doug-w
doug-w force-pushed the feat/remote-daemon-attach branch from dbef6b4 to 552d71e Compare July 28, 2026 06:07
@puritysb

Copy link
Copy Markdown
Owner

Thanks for the rebase — the head is on today's master and merges clean now, and I have updated the branch onto the fixed base so the inherited red checks are no longer in the way (master's tip had drifted on two gates; both are resolved and master's CI is green).

I re-checked the seven blockers from the 2026-07-18 review against this head. At least two are unchanged, and bridge/src/remote-sessions.ts and bridge/src/mdns-discover.ts are byte-identical to the pre-review head — which is where the registry-level guards would live.

(3) Sender identity — still unguarded.

session_push_register in daemon-server.ts takes sessionId straight from the frame and calls registerRemoteSession, which overwrites host, port, callbackToken and sender for that id with no check that the frame arrived on the socket that owns it. Any worker that can reach and authenticate to the daemon can therefore claim another machine's session id and redirect its reverse-control path to itself.

The close handler has the mirror-image bug:

sender.once('close', () => {
  removeRemoteSession(sessionId);
  focusRelay.handleSenderClosed(sessionId);
  ...
});

This removes the entry unconditionally. If worker B re-registers the same id on a new socket and worker A's older socket then closes, A's close evicts B's live registration and drops focus for a session that is still attached — a reconnect race, not an attack, so it will show up in ordinary use.

This repo already has the rule written down, from a bug with the same shape on the device side (CLAUDE.md — "Pending device work keys on the device, not on its socket … must be keyed by the device and have its transport re-resolved at use time"). The fix is the same shape here: record the owning socket on the entry, and make both the re-register path and the close path no-op unless getRemoteSession(id)?.sender === sender.

(4) Token redaction — the token is still logged.

bridge/src/daemon-ws-client.ts:216:

const url = isLoopbackHost(host)
  ? `ws://127.0.0.1:${port}`
  : `ws://${formatHostForUrl(host)}:${port}${token ? `?token=${encodeURIComponent(token)}` : ''}`;
debug(TAG, `Connecting to daemon at ${url}`);

The daemon pairing token — the load-bearing one, not the ephemeral callback token — lands verbatim in the session bridge's debug log. Log host, port, and whether a token was present; never the URL.

I have not re-audited 1, 2, 5, 6 and 7 in this pass. Rather than have me guess at their state, please walk all seven point by point the way you did on #62 — that write-up made the difference between a reviewable PR and one that stalls. Issue #24 remains open and assigned to you either way.

@doug-w

doug-w commented Jul 28, 2026

Copy link
Copy Markdown
Contributor Author

All seven blockers from the 2026-07-18 review are addressed in 3079e2f, on a branch freshly rebased onto current master (the daemon-server.ts / ws-server.ts conflicts are resolved). Blocker-by-blocker:

  1. Opt-in contract enforced as agreed. resolveDaemonTarget now hard-gates BOTH remote paths on remote === true; a host hint without --remote-daemon is inert (regression: probe and mDNS are never called). The CLI warns pre-PTY via a shared derivation helper so the warning condition and the resolver cannot drift, and no false warning fires when the opt-in arrives via AGENTDECK_REMOTE_DAEMON=1. Docs now show --remote-daemon --daemon-host mainnode.lan as the explicit form (README, docs/daemon.md, CLAUDE.md, all four --help texts). Explicit-host failure still returns null with no mDNS fallthrough.

  2. SSH local-forward workers register correctly — via the real handler, integration-tested. Registration now carries an explicit remoteAttach: true intent flag (sent when opted-in AND the daemon advertises the capability), which the daemon trusts over the socket's source IP; the IP heuristic stays only as back-compat for older workers. The push handlers were extracted to bridge/src/session-push-channel.ts precisely so the real registration path is testable: session-push-channel.test.ts drives loopback-IP + flag through it and asserts the session is registered, listable via listRemoteEnriched, and focusable; remote-reverse-e2e.test.ts additionally proves the flag rides a real loopback socket.

  3. Sender-identity guards + overlap regression. removeRemoteSession / handleSenderClosed / updateRemoteSessionState / session_event_up are all keyed to the registered sender socket. On re-register of a focused session, migrateSender moves focus to the new socket (old sender unfocused best-effort, session_focus_down re-sent) so the old socket's late close is a no-op. Workers ignore down-frames with a foreign sessionId (tested over real sockets). The overlap regression asserts: registration survives, focus migrates, commands route down the new socket, and only the new socket's close tears down.

  4. Token redacted. The connect log now prints ws://<host>:<port> (token: yes/no) — the URL itself is never logged.

  5. Dial-back removed (the "remove" option). Callback token generation, WsServer.addAcceptedToken/removeAcceptedToken/acceptedTokens, the focus relay's setTargetResolver/FocusTarget.token/remote dial path, and the callbackToken registry field are all deleted — the machine token is again the only accepted WS credential, and default local-only sessions no longer register any extra token. Same-socket is the only reverse path; a remote session with no live push socket is unreachable until its worker reconnects (by design, now documented).

  6. Self-port guard is host-aware: rejects only isLoopbackHost(host) && port === core.port, so a remote daemon on the same numeric port resolves correctly (unit-tested).

  7. Capability negotiated + docs scoped. Node daemon /health advertises sameSocketControl: true. The explicit-host path refuses a daemon lacking it (null + one-shot warning — logError-based since plain logging is invisible in PTY mode, deduped per host:port because the resolver re-runs on every reconnect backoff); the mDNS confirm filters incapable candidates and propagates the flag on kept ones. Docs state the main node must run the Node CLI daemon. The Swift daemon still accepts a plain local push registration exactly as before — it just can never be selected as a remote hub.

One explicit note on the trust boundary, so it's on the record: /health is unauthenticated and serves the pairing token, so remoteAttach is spoofable by any paired-tier LAN client, and takeover-by-re-register is inherently permitted (legitimate reconnects require it). That matches the repo's pre-existing "paired = trusted" model — the new guards in (3) target stale-close races, not hostile LAN peers.

Accepted side effect worth naming: with --remote-daemon set on a machine that also runs its own local daemon, the local session registers with the flag too — the sessions list dedups it against the local registry, and focus uses same-socket instead of the loopback dial (functionally equivalent; covered by a dedicated test).

Suite: full bridge run green (1491 passed, 1 pre-existing Windows parallel flake in project-name.test.ts that passes solo); docs:check green. Remaining before I flip Draft → Ready: re-run the real Windows-main / Mac-worker scenario including the ssh -L topology, with both machines on the remediated build (a pre-remediation daemon lacks the capability flag and is now correctly refused).

puritysb commented Jul 29, 2026

Copy link
Copy Markdown
Owner

I integrated the latest master—including the npm BLE runtime fix in #87, session --weight from #62, and Linux systemd support from #54—into head dd1d2f6f09286e0f2b7c6776e7a032057c607114.

The merge conflicted in cli.ts, daemon-ws-client.ts, and index.ts; I resolved those conflicts while preserving both feature sets. During integration I also found a cross-feature regression where remote session registration dropped the sorting weight introduced by #62, and fixed the related propagation gaps:

  • preserve normalized weight in the remote registration frame;
  • carry weight through the daemon remote-session registry and list;
  • carry permissionMode, which was missing from remote push state;
  • preserve the existing weight on legacy re-registration and normalize hostile/out-of-range values;
  • add regression coverage for these cases.

Verification:

  • pnpm verify-version passed;
  • pnpm build passed;
  • pnpm docs:check passed;
  • ESLint on changed files: 0 errors;
  • full suite: 131 files / 2,165 tests passed;
  • both GitHub checks on the new head passed.

The seven original code blockers remain resolved and the branch is now integrated with current master. The remaining condition is the author's stated real-machine validation with the remediated build installed on both sides: Windows main node ↔ Mac worker, covering both the explicit-host and ssh -L topologies. This PR should remain Draft with changes-requested until that evidence is posted and the author marks it Ready for review.

@doug-w

doug-w commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

Yep, I'm going to do the manual test pass this evening after work and paste into the PR what was done before taking this out of draft.

@doug-w
doug-w force-pushed the feat/remote-daemon-attach branch from dd1d2f6 to 004dbf5 Compare July 31, 2026 23:24
@doug-w

doug-w commented Jul 31, 2026

Copy link
Copy Markdown
Contributor Author

Two-machine verification — all passed ✅

Setup: Windows 11 main node (daemon + Stream Deck, daemon start -f -d) + macOS worker, both on feat/remote-daemon-attach @ dd1d2f6f, rebuilt (shared + bridge). Verified GET /health on the main node reports "sameSocketControl": true before starting.

# Case Command (worker) Result
T1 Baseline local-only unchanged agentdeck claude (no flags) ✅ Session runs locally, never appears on the main-node deck, no outbound daemon connection in debug log
T2 Host hint without opt-in is inert + warns (blocker 1) agentdeck claude --daemon-host <WIN-IP> ✅ Pre-PTY warning printed (--daemon-host … ignored: remote attach requires --remote-daemon), session stays local. Same via AGENTDECK_DAEMON_HOST env; AGENTDECK_REMOTE_DAEMON=1 + host attaches with no warning
T3 Explicit remote attach + reverse control agentdeck claude --remote-daemon --daemon-host <WIN-IP> ✅ Session listed on the main node and rendered on the Stream Deck; live state repaints; focus from deck; prompt-option buttons appear and selections land in the Mac PTY; interrupt + Shift+Tab mode switch driven from the deck
T4 Pairing token redacted from logs (blocker 4) T3 run with -d, grep debug log ✅ Zero token= occurrences; connect line logs (token: yes) only
T5 ssh -L loopback topology registers as remote (blocker 2) ssh -L 9120:localhost:9120 <win> + agentdeck claude --remote-daemon ✅ Worker registers via the forwarded loopback socket (explicit remoteAttach flag, no IP heuristic), fully listable/focusable/steerable from the deck
T6 Worker restart while focused (blocker 3) Ctrl+C + immediate restart of the T3 session ✅ Re-registration survives the old socket's close (sender-identity guards), focus migrates to the new socket, no ghost/duplicate entry; killing the worker for good clears the key within seconds
T7 Capability negotiation refuses incapable daemon (blocker 7) Main node on pre-remediation master daemon; worker --remote-daemon --daemon-host <WIN-IP> ✅ One-time "daemon does not support remote attach" warning, session stays local-only, warning not repeated across reconnect backoff
T8 mDNS auto-discovery path agentdeck claude --remote-daemon (no host, no tunnel) ✅ Worker discovers the main-node daemon via mDNS, confirms sameSocketControl on /health, attaches; full T3-level control verified

Covers blockers 1–4 and 7 end-to-end on real hardware; blockers 5 (dial-back removal) and 6 (host-aware self-port guard) are structural and covered by the unit/integration suites (session-push-channel.test.ts, remote-reverse-e2e.test.ts, daemon-target.test.ts, focus-relay-samesocket.test.ts).

@doug-w
doug-w marked this pull request as ready for review July 31, 2026 23:27

Copy link
Copy Markdown
Owner

Thanks — this is the manual evidence I was waiting for. The T1–T8 matrix is clear, specific, and covers the real Windows-main / macOS-worker path we agreed was load-bearing in #24. I also re-checked the current squashed head 004dbf58: it merges cleanly, both GitHub checks are green, and the seven-item remediation is materially present. In particular, the opt-in gate, ssh-L classification, close/event sender ownership, token redaction, dial-back removal, host-aware self-port guard, and direct-host capability refusal are no longer the old implementation.

This is the same kind of point-by-point, honestly scoped evidence that made #62 reviewable — thank you.

I did one final lifecycle pass before clearing changes-requested. There are three narrower gaps left, so please keep this Draft for one more round:

1. A failed re-resolution keeps using the previous remote target

In DaemonWsClient.doConnect(), this.target is only updated when the provider returns non-null:

private async doConnect(): Promise<void> {
if (this.closed) return;
if (this.targetProvider) {
let resolved: DaemonTarget | null;
try {
resolved = await this.targetProvider();
} catch (err) {
debug(TAG, `targetProvider threw: ${err instanceof Error ? err.message : String(err)}`);
resolved = null;
}
if (this.closed) return;
if (resolved != null) {
const prev = this.target ? `${this.target.host}:${this.target.port}` : 'null';
const next = `${resolved.host}:${resolved.port}`;
if (prev !== next) debug(TAG, `Daemon target resolved ${prev}${next}`);
this.target = resolved;
}
}
if (!this.target) {
this.scheduleReconnect();
return;
}

After one successful remote attachment, if the daemon disappears or is replaced by an incapable daemon, resolveDaemonTarget() returns null, but the client retains the old target and immediately dials it again. The cached target still carries the old token and sameSocketControl: true, so a capability downgrade/replacement can bypass the refusal that T7 verifies on first connect.

Please clear/update the cached target on every provider result, including null, and add a reconnect regression: capable target → provider returns null/incapable → no new socket is opened until a valid target is resolved again.

2. Capability-less daemons behind ssh -L are still accepted by the local-first branch

resolveDaemonTarget() returns any loopback-discovered daemon before applying the remote capability check:

// 1. Local first — identical to the pre-remote behavior. Never gated: a
// loopback daemon (including an ssh -L forward of a remote one) is safe
// to attach to; whether the session *registers as remote* is decided by
// the capability-aware `remoteAttach` flag, not here.
const local = await findLocal();
if (local) {
return {
host: '127.0.0.1', port: local.port, httpPort: local.httpPort,
sameSocketControl: local.sameSocketControl,
};
}

That is correct for the ordinary local-default path, but an ssh local-forward also appears as loopback. With --remote-daemon --daemon-host 127.0.0.1 (or the documented tunnel topology), a pre-remediation Node daemon or Swift daemon behind the tunnel yields sameSocketControl: false; the worker still connects and sends a plain registration. It can receive an ack while remaining unlistable/unfocusable, instead of producing the one-shot refusal demonstrated by T7. T7 used the direct Windows IP, so it does not exercise this branch.

Please distinguish an explicit opted-in loopback tunnel from an ordinary local-default attachment, or otherwise enforce sameSocketControl for this remote-intent case. Add a regression for ssh -L + incapable daemon that expects null/warning and no attachment.

3. The shared push-state cache is still not sender-owned

The remote registry update is guarded, but the same frame first writes unconditionally to updatePushState():

if (msg.type === 'session_push_state') {
const { sessionId, state, modelName, effortLevel, permissionMode } = msg as any;
if (sessionId && state) {
// Local aggregator path — deliberately NOT sender-guarded.
updatePushState(sessionId, state, modelName, effortLevel, permissionMode);
// Remote registry path — sender-guarded so a stale socket can't
// clobber state pushed by the live one.
updateRemoteSessionState(sessionId, state, modelName, effortLevel, permissionMode, sender);
// Trigger sessions list broadcast so clients get fresh state
core.maybeBroadcastSessionsList();
}
return true; // consumed

This matters in the accepted local + --remote-daemon side effect: that session is registered in both paths, the sessions list dedups to the local row, and a stale pre-reconnect socket can still overwrite the shared state cache after the new sender has taken ownership. The remote registry rejects it, but the displayed local row can use the poisoned cache until a newer push or TTL expiry. The current test explicitly locks in the unguarded behavior.

Please make the aggregator write sender-aware whenever a remote registration exists, and change the overlap regression to prove that an old socket cannot change either the remote registry or the effective push-state cache.

One small non-blocking cleanup while this area is open: REVERSE_RELAYED_EVENTS is a manual mirror of RELAYED_EVENTS. Exporting/importing one canonical set would avoid adding another drift-prone wire-contract mirror.

A full two-machine rerun should not be necessary unless the transport behavior changes; focused unit/real-socket regressions for the three cases above plus green CI are sufficient. After that, update the PR body's now-stale “remaining verification” paragraph, mark it Ready, and I’ll do the final pass promptly.

@doug-w
doug-w marked this pull request as draft August 1, 2026 04:04
@doug-w
doug-w force-pushed the feat/remote-daemon-attach branch from 004dbf5 to 3664433 Compare August 1, 2026 04:06
@doug-w

doug-w commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Rebased onto current master (head 36644335) — rebase only, no code change (one mechanical import-conflict resolution in daemon-server.ts: union of master's subagent-timeline imports and this branch's push-channel imports). The remediation for the three final-round gaps lands as a separate amended push so that force-push compare shows only the fix delta.

@doug-w
doug-w force-pushed the feat/remote-daemon-attach branch from 3664433 to 06efcc6 Compare August 1, 2026 04:14
@doug-w

doug-w commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

All three final-round gaps are addressed in 06efcc61 (on top of today's master — the prior push was the rebase alone, so the force-push compare from there shows only this remediation). Point by point:

1. Failed re-resolution no longer keeps the previous remote target

DaemonWsClient.doConnect() now assigns the provider result to the cached target on every resolution — null included (a provider throw coerces to null). A null result clears the cache and the existing if (!this.target) hold prevents any dial until a valid target resolves again; recovery is automatic on a later backoff. Two supporting changes:

  • Belt-and-braces client guard: under remote intent the client itself refuses a non-null target lacking sameSocketControl (dead code in production now that the resolver enforces it everywhere — see gap 2 — but it makes your literal "null/incapable" property directly testable and defends against future resolver drift).
  • The reconnect base delay is constructor-injectable so the real-socket regressions don't sit through 2s+ backoffs.

Regressions (daemon-ws-client.test.ts, real WS server on an ephemeral port, TCP-connection counter as the primary assertion): (a) capable → provider flips to null → provider keeps being consulted but no new socket opens against the still-listening old endpoint → provider recovers → reconnects; (b) the capability-downgrade case composed through the real resolveDaemonTarget as the provider (findLocal flips capable → incapable ⇒ resolver refuses ⇒ no redial, then upgrade ⇒ reattach); (c) guard unit: non-null incapable target under remote intent ⇒ never dialed.

2. Loopback/tunnel daemons now face the same capability gate

resolveDaemonTarget's local-first branch enforces sameSocketControl whenever remote === true. An incapable loopback daemon (your ssh -L + pre-remediation/Swift shape):

  • with --daemon-host ⇒ falls through to the explicit-host branch, which probes the named host and applies its existing one-shot refusal — for --daemon-host 127.0.0.1 that probes the same tunneled daemon and refuses with the T7 warning. (This also fixes a shape the strict alternative would have broken: an incapable Swift App Store daemon open locally + --daemon-host <remote-hub> now reaches the hub instead of dead-ending with advice the user already followed.)
  • without a host hint ⇒ one-shot warning + null, no mDNS fallthrough — a deliberately-built tunnel fails loudly rather than silently roaming to another LAN daemon.

Without the switch the local path is byte-for-byte unchanged (plain local attach, no warning). One asymmetry worth naming explicitly: a capable local daemon still wins over an explicit --daemon-host (pre-existing precedence, unchanged and still regression-locked), so whether the named host is consulted depends on the local daemon's capability. Regressions: incapable-loopback-no-hint refusal (probe/mDNS never called), hint fallthrough to a capable named host, the ssh -L + --daemon-host 127.0.0.1 + incapable refusal, remote: false untouched, and warn dedup on the exported warnRemoteOnce. The old test that locked in "incapable local returned under remote:true" is replaced by these.

3. The push-state aggregator cache is sender-owned whenever a remote registration exists

session_push_state handling now resolves the remote registration first; if one exists and the frame arrived on a different socket than the registered sender, the whole update is dropped — aggregator cache, remote registry, and the sessions-list broadcast (a rejected write is a non-change). Sessions with no remote registration keep the plain unguarded local path. The overlap regression now proves your exact narrative: register(old) → register(new) → push_state(new) seeds known state, push_state(old) changes neither getRemoteSession().state nor getPushState(), and the live sender still updates both. Additional cases: a never-registered stranger socket is fully rejected, and a pure-local push (no remote registration) still updates the cache — so the guard cannot silently kill ordinary multi-session display.

One residual window, considered and left as-is: after the registered socket closes and the registry entry is pruned, a still-open zombie older socket's push finds no registration and takes the unguarded local path (visible up to the cache's 30s TTL in the dual-registration case). Guarding that would require keying ownership beyond the registration lifetime; it matches the letter of "whenever a remote registration exists" and the same paired-tier trust boundary as before.

Non-blocking cleanup

REVERSE_RELAYED_EVENTS is gone — RELAYED_EVENTS is exported from session-focus-relay.ts and imported by DaemonWsClient.forwardEvent, so both ends of the push socket share one canonical set (no import cycle; the set stays in the bridge package since it has no cross-language consumer).

Docs updated to match: docs/daemon.md resolution-precedence + capability + sender-guard bullets, docs/protocol.md session_push_state row. Suite: full run green apart from the known Windows-host-only failures in files this branch doesn't touch (linux-service/python-ble path separators etc., identical on master); all 61 remote-attach tests plus the 9 new regressions pass. No transport behavior changed — frames, wire format, and registration are untouched — so per your stated bar I have not re-run the two-machine scenario.

@doug-w
doug-w marked this pull request as ready for review August 1, 2026 04:16
--remote-daemon gates both remote paths (explicit --daemon-host and
mDNS discovery); workers register with an explicit remoteAttach flag
negotiated against the daemon's /health sameSocketControl capability,
so ssh -L loopback topologies work without IP heuristics. Reverse
control rides the worker's own push socket (session_focus_down /
session_command_down / session_event_up) -- dial-back is removed
entirely. Sender-identity guards plus focus migration protect
overlapping reconnects; the pairing token is redacted from logs; the
self-port guard is host-aware.

Verified end-to-end on a Windows main node + macOS worker across
local-only, explicit-host, mDNS, and ssh -L topologies.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@doug-w
doug-w force-pushed the feat/remote-daemon-attach branch from 06efcc6 to cc7b2fe Compare August 1, 2026 04:27
@doug-w

doug-w commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

(Head note: the remediation described above is now at cc7b2fe5 ? a cosmetic re-amend of 06efcc61 from a post-hoc adversarial code-review pass: one stale doc comment on ResolveDaemonDeps.warn updated to the new key space, and the warnRemoteOnce dedup test made order-independent by filtering to its own messages. No behavior change; CI green on the new head.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changes-requested Review feedback must be addressed before merge enhancement New feature or request platform:bridge Node bridge, daemon, hooks, and CLI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Attach remote sessions to a central daemon and control them from one Stream Deck (mDNS + explicit host, opt-in)

2 participants