feat(remote-host): E2E remote-host transport, mux codec, and client UI - #188
Conversation
Client + protocol side of Remote Host Support — the persistent, multiplexed, end-to-end-encrypted transport that reaches a host through a relay, plus the My Hosts UI. - protocol/crypto/noise: in-house Noise_NK_25519_AESGCM_SHA256 state machine over WebCrypto (+ @noble X25519), validated against official Noise test vectors; AEAD replay window (monotonic counter + sliding bitmask), forward secrecy per session, fail-closed nonce guard. - protocol/host-transport/mux: shared mux wire codec (frame enums/flags/QoS, 1 MiB cap, encode/decode) — the single source both client and host compile against. - clients/shared/host-transport/remote: persistent Noise-NK + mux transport behind IHostMessenger/IStreamClient — priority scheduler, per-session credits, 64 KiB chunking with fail-closed reassembly, full-attach resume (re-runs open{bearer}, never restores identity from a ticket), reconnect ready-boundary backoff. - clients/gui-app: My Hosts list + honest status DTO rendering (live-session override, presence-degraded, timestamped reachability provenance), remote transport selection by kind (default + tab scope), terminal dead-tile state split, direction-aware version-skew copy, enrollment PoP, remote workspace path picker. - clients/traycer-cli: host update-progress + rollback-aware installer, first- start health probe separating binary health from CS reachability. Versioned RPC compatibility preserved: no new method names — new capabilities fold onto existing versioned methods (host.status v1.1, workspace.prepareFolders v1.1); released-surface + two-sided-invariant guards extended to the stream registry. Pairs with the internal control-plane / relay / host-daemon branch. Signed-off-by: Hardik Shingala <hardik@traycer.ai>
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 44 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Repository UI (base), Organization UI (inherited) Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (16)
Summary by CodeRabbit
WalkthroughThis PR introduces Remote Host Support: a Noise-encrypted multiplexed transport for remote hosts, host discovery/version-policy APIs, a "My Hosts" settings UI, remote workspace folder picking, host compatibility/version-skew messaging, a "reaped" terminal lifecycle state, host directory refresh/persistence, per-panel host scoping, a new-terminal dialog with host badges, and a CLI host update pipeline with health checks and rollback. ChangesRemote Transport & Protocol
Host Management & GUI Integration
CLI & Release Tooling
Estimated code review effort: 5 (Critical) | ~180 minutes Sequence Diagram(s)sequenceDiagram
participant GUIClient as GUI Client (RemoteSession)
participant RelaySocket as RelaySocket
participant Relay as Relay Server
participant Host as Remote Host
GUIClient->>RelaySocket: mint attach grant + dial (?grant=jws)
RelaySocket->>Relay: WebSocket connect + attach
Relay-->>RelaySocket: attach_ack
RelaySocket-->>GUIClient: onAttachAck
GUIClient->>GUIClient: Noise handshake msg0
GUIClient->>Relay: encrypted msg0 (DATA frame)
Relay-->>Host: forward msg0
Host-->>Relay: msg1
Relay-->>GUIClient: forward msg1
GUIClient->>GUIClient: derive NoiseSession, complete handshake
GUIClient->>Host: encrypted mux OPEN frame
Host-->>GUIClient: encrypted mux OPEN_ACK
GUIClient->>Host: SUBSCRIBE / REQUEST frames
Host-->>GUIClient: STREAM_FRAME / RESPONSE frames
sequenceDiagram
participant User
participant MyHostsList
participant useUpdateHostVersionPolicy
participant AuthService
participant RunnerHost as IRunnerHost/Electron
User->>MyHostsList: toggle auto-update / apply now
MyHostsList->>useUpdateHostVersionPolicy: mutate(hostId, input)
useUpdateHostVersionPolicy->>AuthService: updateHostVersionPolicy(hostId, input)
AuthService->>RunnerHost: updateHostVersionPolicy(bearer, hostId, input)
RunnerHost-->>AuthService: UpdateHostVersionPolicyFetchResult
AuthService-->>useUpdateHostVersionPolicy: result / error
useUpdateHostVersionPolicy->>MyHostsList: invalidate registeredHosts query
sequenceDiagram
participant CLI as traycer host update
participant Marker as update-progress-marker
participant Installer
participant HealthProbe as probeHostHealth
participant Service
CLI->>Marker: write "updating" (targetVersion)
CLI->>Installer: installHost(version)
Installer-->>CLI: install result
CLI->>HealthProbe: probeHostHealth()
alt healthy
HealthProbe-->>CLI: healthy: true
CLI->>Marker: delete marker
else unhealthy
HealthProbe-->>CLI: healthy: false, detail
CLI->>Service: rollback to previousVersionedDir
CLI->>Marker: write "failed" (detail)
CLI-->>CLI: throw HOST_UPDATE_HEALTH_CHECK_FAILED
end
Possibly related issues
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
protocol/package.json declared @noble/curves for the Noise crypto module but bun.lock was never regenerated, breaking bun install --frozen-lockfile in CI. Signed-off-by: Hardik Shingala <hardik@traycer.ai>
session.ts imports Mutex from async-mutex to serialize decrypt against the shared replay window, but the dependency was never declared in package.json/bun.lock — breaking module resolution wherever bun.lock is frozen (CI). Signed-off-by: Hardik Shingala <hardik@traycer.ai>
There was a problem hiding this comment.
Actionable comments posted: 41
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
clients/gui-app/src/providers/__tests__/windows-bridge-provider.test.tsx (1)
145-211: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftConsider consolidating duplicated
IRunnerHostmock literals.This PR had to hand-edit the same 3 new fields (
relayBaseUrl,listRegisteredHosts,updateHostVersionPolicy) across at least 3 near-identical fullIRunnerHostmock literals in this file,menu-command-listener.test.tsx, andhost-tray-command-listener.mounted.test.tsx(plus similar patterns likely elsewhere, e.g.local-host-gate.test.tsx'smakeHost). The PR's own file list also introducesclients/shared/host-client/mock/mock-runner-host.tsas a canonical mock runner host implementation for this same interface.Every future
IRunnerHostsurface change will require this same multi-file, easy-to-miss edit. Consider extracting a shared base-mock factory (or reusing/wrapping the newmock-runner-host.tsif its shape fits renderer test needs) that these test files spread/override from.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clients/gui-app/src/providers/__tests__/windows-bridge-provider.test.tsx` around lines 145 - 211, The duplicated IRunnerHost test mocks are drifting and make interface updates brittle; consolidate them into a shared base mock/factory instead of repeating full literals in windows-bridge-provider.test.tsx and the other listener tests. Reuse the canonical mock in mock-runner-host.ts if it matches the renderer test shape, or create a shared helper that returns the common runner host defaults and let each test override only the fields it needs. Make sure the shared helper includes the newly added relayBaseUrl, listRegisteredHosts, and updateHostVersionPolicy members so future IRunnerHost changes only need one edit.clients/gui-app/src/lib/host/__tests__/durable-stream-transport.test.ts (1)
51-74: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winNo test exercises the new
buildHostStreamClient→nullthrow path.
mocks.buildHostStreamClient.mockReturnValue(fakeWs)inbuildParamsalways resolves to a non-null client, so the new "invalid public key" throw branch added toopenDurableStreamTransport(durable-stream-transport.ts lines 69-76) has no covering test in this suite.✅ Suggested test case
+it("throws when buildHostStreamClient returns null (invalid remote public key)", () => { + mocks.buildHostStreamClient.mockReturnValue(null); + const { params } = buildParams(() => undefined); + expect(() => openDurableStreamTransport(params)).toThrow( + /invalid public key/, + ); +});🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clients/gui-app/src/lib/host/__tests__/durable-stream-transport.test.ts` around lines 51 - 74, Add a test in durable-stream-transport.test.ts that covers the new null client path in openDurableStreamTransport by making buildHostStreamClient return null instead of fakeWs. Use the existing buildParams helper or a new variant to simulate the invalid public key case, then assert that the transport throws the expected error and does not proceed with reconnect/close behavior. Reference openDurableStreamTransport and buildHostStreamClient so the new branch is explicitly covered.clients/gui-app/src/stores/terminals/terminal-session-store.ts (1)
414-491: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFreshly dispatched resize is immediately purged as "stale" by the very next call.
On a reconnect,
onConnectionStatus's open-branch callsflushRequestedResize()(which, ifrequestedCols/Rows !== effectiveCols/Rows, mints a brand-new resizeclientActionId, records it viarecordPendingAction, and dispatches it) immediately followed byreplayPendingActionsAfterReconnect(). The replay function re-readsget().pendingActionsand treats every currently-pendingresizeentry as stale, including the oneflushRequestedResize()just added moments earlier — so the just-sent resize's pending-action record gets deleted right after being created, even though no ack has arrived yet.In practice this is masked because
onSnapshotcallsflushRequestedResize()again afterward (once the host's snapshot lands) and that later entry survives — but the design intent of tracking resize actions for ack/eviction bookkeeping is defeated for this first dispatch, and the ordering is fragile.🔧 Proposed fix: run replay before the fresh flush
if (status !== "open") return; - flushRequestedResize(); - replayPendingActionsAfterReconnect(); + replayPendingActionsAfterReconnect(); + flushRequestedResize();Also applies the same "prefer functional methods over for-loops" guideline to the
write-frame replay loop:♻️ Optional style fix
- for (const pending of Object.values(pendingActions)) { - if (pending.frame.kind !== "write") continue; - dispatchClientFrame(pending.frame); - } + Object.values(pendingActions) + .filter((pending) => pending.frame.kind === "write") + .forEach((pending) => dispatchClientFrame(pending.frame));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clients/gui-app/src/stores/terminals/terminal-session-store.ts` around lines 414 - 491, `replayPendingActionsAfterReconnect` is deleting the resize just added by `flushRequestedResize`, so the reconnect flow in `onConnectionStatus` should replay old pending actions before issuing the fresh resize. Update the reconnect open-branch ordering so `replayPendingActionsAfterReconnect()` runs first, then `flushRequestedResize()`, and keep `recordPendingAction`/`removePendingAction` behavior unchanged. While touching `replayPendingActionsAfterReconnect`, prefer a functional iteration style for the `write` replay path instead of the manual loop if needed.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@clients/desktop/src/electron-preload/auth-bridge.ts`:
- Around line 8-12: The auth bridge in electron-preload is importing plain-data
host types directly from the shared host-client modules, which violates the
preload-only dependency boundary. Update auth-bridge to use types from
src/ipc-contracts/ instead of `@traycer-clients/shared/host-client/`* by mirroring
or re-exporting HostListFetchResult, UpdateHostVersionPolicyFetchResult, and
UpdateHostVersionPolicyInput there, and keep those ipc-contracts definitions
aligned with the canonical shared module.
In
`@clients/gui-app/src/components/home/host-workspace-selector/__tests__/remote-workspace-path-picker-host.test.tsx`:
- Around line 18-22: Replace the object-shaped helper type MutateCall with an
interface definition in remote-workspace-path-picker-host.test.tsx, following
the TypeScript guideline to prefer interface for object shapes. Keep the same
readonly fields and callback signatures, and update any references that depend
on MutateCall so the test helpers and mutate call typing continue to work
unchanged.
- Around line 159-164: The success path test for the remote workspace path
picker triggers mounted state updates through the mutation callback, so the
`onSuccess` call should be wrapped in `act` just like the error/rejection cases.
Update the test in `remote-workspace-path-picker-host.test.tsx` around the
`mocks.mutateCalls[0].onSuccess` invocation so React flushes the dialog state
changes before asserting on `pending`.
In
`@clients/gui-app/src/components/home/host-workspace-selector/remote-workspace-path-picker-host.tsx`:
- Around line 136-141: The submit flow in remote-workspace-path-picker-host
should also block when openMutation.isPending is true, not just when the button
is disabled. Update the submit function and the Enter key handler path so both
check the pending mutation state before calling openMutation.mutate, preventing
duplicate remote open requests while a request is already in flight.
- Line 10: The path shortcut chips in RemoteWorkspacePathPickerHost are using
Badge with only onClick, so they are not treated as interactive controls. Update
the home/recent path rendering in remote-workspace-path-picker-host.tsx to use a
button-based component or an element with proper button semantics and keyboard
support, while keeping the same chip styling. Make sure the clickable path items
remain easy to find in the code by using the existing
RemoteWorkspacePathPickerHost rendering logic for the shortcut list.
In `@clients/gui-app/src/components/local-host-gate.tsx`:
- Around line 495-526: The host update gate in GateIncompatibleHost is deriving
behavior from the user-facing VersionSkewCopy.action string ("Update the app"),
which is fragile and can break when copy changes. Add a machine-readable
discriminator to VersionSkewCopy in describeVersionSkew/version-skew-copy.ts
(for example, a direction or role field) and have GateIncompatibleHost compute
hostIsOutdated from that typed field instead of comparing display text, then
keep the refresh/force-action gating based on that new symbol.
In `@clients/gui-app/src/components/settings/panels/host-settings-panel.tsx`:
- Around line 58-65: The Host settings panel copy is inconsistent because the
description in HostSettingsPanel promises “this machine’s local service” while
the body copy says local host management is unavailable in this shell. Update
the description text in the HostSettingsPanel component so it only describes the
hosts list that is actually available in this view, and keep the local-service
limitation solely in the explanatory message below. Be sure to adjust the copy
where the Host panel title/description is defined, not the MyHostsList content.
In `@clients/gui-app/src/components/settings/panels/my-hosts-list.tsx`:
- Around line 364-373: The “Update…” popover trigger in MyHostsList is missing
the same pending-state guard used by the auto-update Switch and the “Apply now”
action. Update the Button inside PopoverTrigger in my-hosts-list.tsx to disable
when the relevant mutation is pending, using the same mutation.isPending check
and host-scoped logic already used around the surrounding controls so the
trigger cannot start a conflicting write while one is in flight.
In `@clients/gui-app/src/components/settings/panels/my-hosts-model.ts`:
- Around line 190-205: The version regex in isValidHostVersion and
HOST_VERSION_PATTERN is duplicated from the server-side DESIRED_VERSION_PATTERN,
so it can drift silently. Update the client to consume the shared source of
truth from `@traycer/protocol` (or another shared module used by both the GUI app
and authn-v3) instead of hardcoding the regex locally, and keep the existing
trim-and-test behavior in isValidHostVersion.
In `@clients/gui-app/src/hooks/auth/use-update-host-version-mutation.ts`:
- Around line 54-85: Capture a stable mutation context in
useUpdateHostVersionPolicy by adding onMutate to snapshot the current
binding/auth context and hostId before the request starts, then use that context
in mutationFn/onSuccess instead of the live closure values. Update the query
invalidation in onSuccess to target the registeredHosts key from the captured
binding, so a host or binding swap during the in-flight mutation cannot
invalidate the wrong cache entry. Keep the existing useMutation,
authMutationKeys.updateHostVersionPolicy, and authQueryKeys.registeredHosts
wiring, but ensure the captured context is the source of truth for the mutation
lifecycle.
In `@clients/gui-app/src/hooks/host/use-host-client-for.ts`:
- Around line 109-118: Starting the remote transport session inside useMemo in
useHostClientFor can open a socket during render before commit. Move
built.remoteTransport?.session.start() out of the useMemo block and into the
existing useEffect that already cleans up remoteTransport.session.close(), so
the start/close lifecycle is paired after commit and only runs for committed
bindings.
In `@clients/gui-app/src/hooks/host/use-host-stream-client-for.ts`:
- Around line 132-162: The buildHostStreamClient path is still falling through
for remote targets that are not valid RemoteHostDirectoryEntry values, which
allows the fallback WsStreamClient route to connect when it should not. Update
buildHostStreamClient to fail closed by returning null for any
params.target.kind === "remote" that does not satisfy
isRemoteHostDirectoryEntry, and only create the remote transport when the target
is a complete remote entry with a websocketUrl.
- Around line 214-280: `use-host-stream-client-for` is invoking
`buildHostStreamClient` inside `useMemo`, but that builder starts
`remoteTransport.session` for remote targets and makes render-time memoization
impure. Move the remote session startup/teardown into a `useEffect` with cleanup
(or refactor `buildHostStreamClient` to be fully lazy/pure) while keeping the
existing memoized `binding`/`memoizedTarget` construction intact. Use the
`buildHostStreamClient`, `useMemo`, and `PLACEHOLDER_REMOTE_STATUS` paths to
locate the affected logic and ensure remote sessions are created only in
effect-driven side effects, not during render.
In `@clients/gui-app/src/lib/host/host-messenger.ts`:
- Around line 243-249: The remote transport key construction in
host-messenger.ts is including volatile host status, which causes session
identity changes on availability updates. Update the transport key builder
around the logic that joins entry.hostId, entry.websocketUrl, entry.version,
entry.status, and entry.publicKey so that status is excluded, keeping
createRemoteHostTransport keyed only by stable fields and preserving the active
transport for the same host.
- Around line 58-85: The remote-branch handling in host-messenger should not
fall through to the local WsRpcClient path when params.target.kind is "remote"
but isRemoteHostDirectoryEntry(params.target) fails or websocketUrl is null.
Update the create/return flow around createRemoteHostTransport and the final
return so malformed remote targets immediately return null, and only construct
WsRpcClient for non-remote targets.
In `@clients/gui-app/src/lib/host/one-shot-stream-transport.ts`:
- Line 2: Add a focused test that directly covers the buildHostStreamClient(...)
=== null branch in one-shot-stream-transport behavior, since the existing
settings-panel and worktree-delete tests only stub the transport and never
trigger the invalid-public-key throw. Create or extend a test around the
relevant host stream client setup to assert the throw path from
buildHostStreamClient and verify the error is surfaced when the client cannot be
built.
In `@clients/gui-app/src/lib/host/version-skew-copy.ts`:
- Around line 44-58: The fallback in version-skew-copy.ts is not fully driven by
guidance, so ambiguous guidance (both upgrade flags set or neither set) can
silently return the wrong update direction. Update the logic in the version-skew
copy function to explicitly handle the comparison/default path and the
ambiguous-guidance case, using the existing guidance checks around
hostShouldUpgrade and clientShouldUpgrade, rather than always returning the
host-update copy. If ambiguous guidance should never happen, assert or log it in
this function instead of defaulting silently.
In `@clients/gui-app/src/lib/host/viewer-reachability-store.ts`:
- Around line 15-26: The viewer reachability store currently only exposes
getViewerReachabilityCheck(), so checksByHostId is never populated and the
viewer-specific connection-issue path in deriveHostPresence cannot be reached.
Add a writer such as recordViewerReachabilityCheck to update the checksByHostId
map with ViewerReachabilityCheck entries, and wire that function into the
tab-open probe and the “Check now” action so those flows persist the latest
result instead of always returning null.
In `@clients/gui-app/src/stores/terminals/terminal-session-store.ts`:
- Around line 161-192: The FIFO trim logic in appendPendingAction currently uses
a manual for-loop to rebuild the pendingActions map. Refactor the eviction path
to use a functional transform with Object.fromEntries and map (or equivalent
functional array methods) when constructing trimmed, while keeping the same
keys.slice(...) behavior, MAX_PENDING_ACTIONS cap, and evicted flag semantics.
In `@clients/shared/host-client/host-version-policy-fetcher.ts`:
- Around line 1-147: Add a dedicated test file for
updateHostVersionPolicyViaHttp, mirroring the coverage style used for
fetchRegisteredHostsViaHttp in remote-fetcher.test.ts. Exercise the main
branches of host-version-policy-fetcher.ts: ok parsing, not-found, invalid,
unauthorized, and network-error, and assert the returned discriminated
kind/result for each case. Use the updateHostVersionPolicyViaHttp and
hostPatchUrl behavior as the entry point, and mock
fetch/response.json/hostVersionPolicyResponseSchema as needed to keep the tests
focused on this helper.
In `@clients/shared/host-transport/remote/create-remote-transport.ts`:
- Around line 104-116: The bearer token read in deriveBearerToken currently
swallows getBearerToken failures and returns null, which hides transport-level
diagnostics. Update deriveBearerToken in create-remote-transport.ts to catch the
exception, emit a non-secret error log at this boundary (no token contents), and
then return null as before. Use the existing bearer() / source.getBearerToken()
flow to locate the change and keep the behavior of AttachGrantProvider unchanged
aside from the added logging.
- Around line 84-95: Replace the monkey-patch in createRemoteTransport by using
an explicit teardown callback on RemoteSessionOptions instead of reassigning
session.close. Add an injected onClose (or similarly named) hook to the
RemoteSession construction path, store it in RemoteSession, and invoke it from
RemoteSession.close() so unregister() runs through the class’s own teardown
flow. Update the create-remote-transport logic and any related
RemoteSession/RemoteSessionOptions symbols to keep the lifecycle contract
explicit and type-safe.
In `@clients/shared/host-transport/remote/logical-stream.ts`:
- Around line 93-100: The terminal closed-state handling in LogicalStream is
incomplete because notifyStatus("closed", ...) does not mark the stream
disposed, allowing LogicalStream.close() to send a duplicate CLOSE after
host-side cleanup. Update the LogicalStream status transition path that handles
"closed" to also set disposed = true, and verify the close() guard still
short-circuits once the stream has been terminally closed. Use the LogicalStream
class, its notifyStatus/transition flow, and close() to locate the fix.
In `@clients/shared/host-transport/remote/noise-channel.ts`:
- Around line 111-130: The host public key decoding in decodeHostPublicKey is
still using heuristic auto-detection, which leaves the encoding ambiguous.
Update the GET /hosts contract so the published DTO carries an explicit encoding
tag, then adjust decodeHostPublicKey to switch on that field instead of sniffing
the string contents. Keep the length validation and InvalidHostPublicKeyError
behavior intact, and use the existing decodeHostPublicKey and KEY_LEN symbols to
locate the change.
In `@clients/shared/host-transport/remote/relay-socket.ts`:
- Around line 63-91: The relay attach flow is still passing the grant through
the URL query string instead of WebSocket subprotocols. Update RelaySocket and
RelaySocketOptions to pass the attach grant via IStreamWebSocketFactory.create
using a subprotocol like grant.<jws>, and widen the factory interface so it
accepts the subprotocol list. Keep the existing RelaySocket wiring and timeout
logic, but remove the withGrantQuery-based URL construction from the constructor
and use the new subprotocol-aware creation path.
In `@clients/shared/host-transport/remote/remote-session.ts`:
- Around line 876-897: The reconnect path in handleConnectionLost is discarding
the specific cause, which makes teardown/reconnect diagnostics too generic. Keep
the passed-in cause in RemoteSession.handleConnectionLost and use it when
calling teardownConnection instead of hardcoding "connection-lost"; also remove
the void cause line so the transport boundary preserves meaningful reasons like
write-failed, socket-closed, or malformed-openAck. Ensure any logging in
teardownConnection/remote-session lifecycle uses the provided cause so
production logs can distinguish the failure source.
In `@clients/traycer-cli/src/commands/host-update.ts`:
- Around line 122-151: The failure-marker write in host update error handling
can throw and replace the original install error; make the write in
host-update’s catch block best-effort so `installHost` failures still propagate.
In `hostUpdate` around the `writeUpdateProgressMarker` call, catch and
ignore/log marker-write errors, then always rethrow the original `cause` after
logging with `ctx.runtime.logger.error`. Apply the same non-throwing treatment
anywhere else the failed-state marker is written so `installHost`,
`writeUpdateProgressMarker`, and `deleteUpdateProgressMarker` behave
consistently.
- Around line 200-251: The rollback path in host-update handling can throw
before the failure state is recorded, so wrap the entire rollback branch in a
fail-closed try/catch around the `rollbackToVersionedDir` and
`createServiceInstallLifecycle` calls in `host-update.ts`. Ensure any error
during rollback still calls `writeUpdateProgressMarker` with state "failed" and
then rethrows a `cliError` using
`CLI_ERROR_CODES.HOST_UPDATE_HEALTH_CHECK_FAILED` rather than letting the
exception escape as `E_UNEXPECTED`. Keep the existing health-check failure
details from `probe.detail`, `result.record.version`, and `previous.version` in
the final error path.
In `@clients/traycer-cli/src/installer/__tests__/install.test.ts`:
- Around line 1-263: Add a regression test for the legacy-migration crash window
that is missing from this suite. In `install.test.ts`, mirror the existing
forward-swap crash test by exercising `migrateLegacyLayoutIfNeeded` from
`install.ts`: simulate the legacy plain-directory install, perform only the
rename/move step, and assert `readActiveVersionedDir` still resolves to the old
install until the pointer flip runs. Use the existing symbols
`migrateLegacyLayoutIfNeeded`, `readActiveVersionedDir`, and `hostInstallDir` to
pin that a crash in the middle never leaves the install unresolved or lossy.
In `@clients/traycer-cli/src/installer/install.ts`:
- Around line 774-789: The symlink/target-resolution flow is duplicated between
readActiveVersionedDir and migrateLegacyLayoutIfNeeded, so refactor the shared
lstat/isSymbolicLink/readlink/resolveSymlinkTarget logic into a helper like
resolveInstallPointerTarget(target) that returns a unified result shape for
missing, symlink, or plain dir. Update readActiveVersionedDir to use that helper
and return the resolved path or null, and make migrateLegacyLayoutIfNeeded
consume the same helper so both paths stay in sync and avoid future drift.
- Around line 578-699: The crash-safe pointer invariant is broken by the legacy
migration step because migrateLegacyLayoutIfNeeded() moves the old plain
directory out of hostInstallDir before flipHostInstallPointer() atomically
repoints it, leaving a crash window where hostInstallDir is missing. Update
atomicSwap() so the legacy layout migration preserves the old pointer until the
new versioned dir is ready, or otherwise performs migration through the same
atomic pointer-flip flow used by flipHostInstallPointer(), keeping
hostInstallDir always resolving to either the old or new install.
- Around line 718-746: Windows pointer flips in flipHostInstallPointer still
fail when hostInstallDir(environment) already exists as a junction on win32
because rename(tmpLinkPath, target) does not replace it. Update the flip path to
handle the Windows-specific case by removing/unlinking the existing target
before recreating it (or using an equivalent junction replace flow), while
keeping the current behavior for non-Windows platforms and preserving the
existing CLI_ERROR_CODES.HOST_INSTALL_FAILED error handling.
- Around line 791-843: Make legacy-layout migration crash-recoverable in
migrateLegacyLayoutIfNeeded by ensuring a partially completed
rename(opts.target, migratedDir) can be rediscovered on the next install before
sweepStaleVersionedDirs runs. Use a deterministic migrated dir name or persist a
small marker alongside the move so flipHostInstallPointer can resume from the
existing migratedDir instead of treating hostInstallDir as empty. Update the
migration path and the subsequent recovery/lookup logic to recognize and
preserve the migrated directory for rollback.
In `@clients/traycer-cli/src/installer/uninstall.ts`:
- Around line 116-131: The symlink-target resolution logic in
resolveInstallDirTarget duplicates the existing helper flow in install.ts
(resolveSymlinkTarget/readActiveVersionedDir), so refactor to use one shared
utility instead of reimplementing lstat/isSymbolicLink/readlink/path resolution
here. Export the common helper from install.ts or move it into a shared
pointer-resolve utility, then update uninstall.ts to call that shared function
so both paths stay consistent.
In `@protocol/package.json`:
- Line 121: Add async-mutex as a direct dependency in protocol/package.json
because protocol/src/crypto/noise/session.ts imports it at runtime and published
installs will otherwise break. Update the package manifest in the protocol
package so the Session-related noise crypto code can resolve async-mutex without
relying on transitive dependencies.
In `@protocol/src/crypto/noise/bytes.ts`:
- Around line 56-69: `hexToBytes` is allowing partial parses because
`Number.parseInt(..., 16)` can accept malformed pairs like "1g" and return a
valid byte; update the strict parsing logic in `hexToBytes` to validate each
2-character slice as exactly hex before conversion, and keep throwing on any
non-hex input so invalid data cannot decode silently.
In `@protocol/src/crypto/noise/replay-window.ts`:
- Around line 56-70: `ReplayWindow.commit` currently mutates state without
enforcing the same acceptance rules as `check`, so a stale, negative, or
out-of-window counter can corrupt the replay window. Update `commit` to validate
the incoming counter against the existing `check` predicate before touching
`highest` or `bitmask`, and fail closed by rejecting invalid commits rather than
relying on caller discipline. Keep the window update logic in `commit` unchanged
for valid counters, but ensure the method cannot advance state for counters that
`check` would not accept.
In `@protocol/src/host-transport/mux.ts`:
- Around line 366-406: decodeMuxFrame currently trusts the decrypted payload
length and can parse frames larger than the local 1 MiB cap, so add an early
size check before any header parsing or slicing. In decodeMuxFrame, reject any
Uint8Array whose length exceeds the same MAX_MUX_FRAME_BYTES limit used by the
sender path, and throw a MuxFrameDecodeError with a clear oversized-frame
message before reading version, type, or json/binary sections. Keep the rest of
the parsing logic unchanged so remote-session.ts callers still receive the same
MuxFrame shape for valid inputs.
- Line 377: Reject unknown frame type bytes in the mux decoder instead of
force-casting them. In mux.ts, update the decode path around the `type`
assignment in the frame parsing logic to validate `bytes[1]` against the known
`MuxFrameType`/`MuxFrameTypeValue` members and throw `MuxFrameDecodeError` for
unrecognized values. Make sure the check happens before constructing or
returning the `MuxFrame` so downstream dispatch never receives an invalid
`type`.
In `@protocol/src/host/host-status.ts`:
- Around line 163-173: Tighten the `busySessionCount` field in
`hostStatusDtoSchema` so it matches the fail-closed contract used elsewhere.
Update the `z.object` definition in `host-status.ts` to validate
`busySessionCount` as a nonnegative integer, aligning it with the `host.status`
v1.1 schema in `contracts.ts`, and keep `deriveUpdateAffordance` from ever
receiving invalid counts.
---
Outside diff comments:
In `@clients/gui-app/src/lib/host/__tests__/durable-stream-transport.test.ts`:
- Around line 51-74: Add a test in durable-stream-transport.test.ts that covers
the new null client path in openDurableStreamTransport by making
buildHostStreamClient return null instead of fakeWs. Use the existing
buildParams helper or a new variant to simulate the invalid public key case,
then assert that the transport throws the expected error and does not proceed
with reconnect/close behavior. Reference openDurableStreamTransport and
buildHostStreamClient so the new branch is explicitly covered.
In `@clients/gui-app/src/providers/__tests__/windows-bridge-provider.test.tsx`:
- Around line 145-211: The duplicated IRunnerHost test mocks are drifting and
make interface updates brittle; consolidate them into a shared base mock/factory
instead of repeating full literals in windows-bridge-provider.test.tsx and the
other listener tests. Reuse the canonical mock in mock-runner-host.ts if it
matches the renderer test shape, or create a shared helper that returns the
common runner host defaults and let each test override only the fields it needs.
Make sure the shared helper includes the newly added relayBaseUrl,
listRegisteredHosts, and updateHostVersionPolicy members so future IRunnerHost
changes only need one edit.
In `@clients/gui-app/src/stores/terminals/terminal-session-store.ts`:
- Around line 414-491: `replayPendingActionsAfterReconnect` is deleting the
resize just added by `flushRequestedResize`, so the reconnect flow in
`onConnectionStatus` should replay old pending actions before issuing the fresh
resize. Update the reconnect open-branch ordering so
`replayPendingActionsAfterReconnect()` runs first, then
`flushRequestedResize()`, and keep `recordPendingAction`/`removePendingAction`
behavior unchanged. While touching `replayPendingActionsAfterReconnect`, prefer
a functional iteration style for the `write` replay path instead of the manual
loop if needed.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 22398573-9a3c-40a5-9142-dc718ec4130e
📒 Files selected for processing (163)
clients/desktop/src/config.tsclients/desktop/src/electron-main/ipc/__tests__/runner-ipc.test.tsclients/desktop/src/electron-main/ipc/auth-ipc.tsclients/desktop/src/electron-main/ipc/ipc-parsers.tsclients/desktop/src/electron-preload/auth-bridge.tsclients/desktop/src/electron-preload/preload-bridge.tsclients/desktop/src/ipc-contracts/ipc-channels.tsclients/desktop/src/renderer-shell/__tests__/desktop-runner-host.test.tsclients/desktop/src/renderer-shell/desktop-runner-host.tsclients/gui-app/__tests__/traycer-app.test.tsxclients/gui-app/src/components/__tests__/epics-list.test.tsxclients/gui-app/src/components/__tests__/local-host-gate.test.tsxclients/gui-app/src/components/auth/__tests__/user-menu.test.tsxclients/gui-app/src/components/auth/auth-landing-page.tsxclients/gui-app/src/components/epic-canvas/renderers/__tests__/terminal-connection-overlay-state.test.tsclients/gui-app/src/components/epic-canvas/renderers/terminal-connection-overlay-state.tsclients/gui-app/src/components/epic-canvas/renderers/terminal-connection-overlay.tsxclients/gui-app/src/components/epic-canvas/renderers/terminal-tile.tsxclients/gui-app/src/components/epic-canvas/renderers/tui-agent-tile.tsxclients/gui-app/src/components/epic-canvas/snapshots/snapshot-error-banner.tsxclients/gui-app/src/components/home/host-workspace-selector/__tests__/remote-workspace-path-picker-host.test.tsxclients/gui-app/src/components/home/host-workspace-selector/remote-workspace-path-picker-host.tsxclients/gui-app/src/components/layout/__tests__/desktop-dialog-host.test.tsxclients/gui-app/src/components/layout/__tests__/host-tray-command-listener.mounted.test.tsxclients/gui-app/src/components/layout/__tests__/menu-command-listener.test.tsxclients/gui-app/src/components/layout/__tests__/sign-in-button.test.tsxclients/gui-app/src/components/layout/app-shell.tsxclients/gui-app/src/components/layout/header/host-picker.tsxclients/gui-app/src/components/local-host-gate.tsxclients/gui-app/src/components/onboarding/onboarding-page.tsxclients/gui-app/src/components/settings/panels/__tests__/my-hosts-model.test.tsclients/gui-app/src/components/settings/panels/host-settings-panel.tsxclients/gui-app/src/components/settings/panels/my-hosts-list.tsxclients/gui-app/src/components/settings/panels/my-hosts-model.tsclients/gui-app/src/components/settings/panels/use-worktree-delete-run.tsclients/gui-app/src/hooks/agent/use-host-reachability.tsclients/gui-app/src/hooks/auth/use-registered-hosts-query.tsclients/gui-app/src/hooks/auth/use-update-host-version-mutation.tsclients/gui-app/src/hooks/git/use-git-list-changed-files-subscription.tsclients/gui-app/src/hooks/host/__tests__/use-host-client-for.test.tsxclients/gui-app/src/hooks/host/__tests__/use-host-query.test.tsxclients/gui-app/src/hooks/host/__tests__/use-host-stream-client-for.test.tsxclients/gui-app/src/hooks/host/use-host-client-for.tsclients/gui-app/src/hooks/host/use-host-stream-client-for.tsclients/gui-app/src/hooks/workspace/use-workspace-folder-actions.tsclients/gui-app/src/lib/app-version.tsclients/gui-app/src/lib/auth/auth-service.tsclients/gui-app/src/lib/host/__tests__/durable-stream-transport.test.tsclients/gui-app/src/lib/host/__tests__/remote-workspace-path-picker.test.tsclients/gui-app/src/lib/host/__tests__/stream-wake-reconnect.test.tsclients/gui-app/src/lib/host/__tests__/version-skew-copy.test.tsclients/gui-app/src/lib/host/durable-stream-transport.tsclients/gui-app/src/lib/host/host-messenger.tsclients/gui-app/src/lib/host/one-shot-stream-transport.tsclients/gui-app/src/lib/host/owned-durable-stream-client.tsclients/gui-app/src/lib/host/remote-workspace-path-picker.tsclients/gui-app/src/lib/host/stream-runtime-context.tsclients/gui-app/src/lib/host/stream-runtime.tsxclients/gui-app/src/lib/host/stream-wake-reconnect.tsclients/gui-app/src/lib/host/use-close-ws-stream-client-on-replace.tsclients/gui-app/src/lib/host/use-durable-stream-transport.tsclients/gui-app/src/lib/host/use-worktree-delete-stream-transport.tsclients/gui-app/src/lib/host/version-skew-copy.tsclients/gui-app/src/lib/host/viewer-reachability-store.tsclients/gui-app/src/lib/query-keys/auth-mutation-keys.tsclients/gui-app/src/lib/query-keys/auth-query-keys.tsclients/gui-app/src/lib/registries/terminal-session-registry.tsclients/gui-app/src/providers/__tests__/epic-access-coordinator.test.tsxclients/gui-app/src/providers/__tests__/host-compatibility-provider.test.tsxclients/gui-app/src/providers/__tests__/windows-bridge-provider.test.tsxclients/gui-app/src/providers/host-runtime-provider.tsxclients/gui-app/src/stores/epics/canvas/__tests__/host-binding.test.tsclients/gui-app/src/stores/epics/open-epic/__tests__/store.test.tsclients/gui-app/src/stores/epics/open-epic/store.tsclients/gui-app/src/stores/terminals/__tests__/terminal-session-store.test.tsclients/gui-app/src/stores/terminals/terminal-session-store.tsclients/shared/host-client/REMOTE-TRANSPORT.mdclients/shared/host-client/__tests__/remote-fetcher.test.tsclients/shared/host-client/host-version-policy-fetcher.tsclients/shared/host-client/mock/mock-runner-host.tsclients/shared/host-client/remote-fetcher.tsclients/shared/host-transport/__tests__/ws-rpc-client.test.tsclients/shared/host-transport/chat-stream-client.tsclients/shared/host-transport/epic-stream-client.tsclients/shared/host-transport/host-stream-client.tsclients/shared/host-transport/i-stream-client.tsclients/shared/host-transport/migration-stream-client.tsclients/shared/host-transport/notifications-stream-client.tsclients/shared/host-transport/remote/__tests__/chunker.test.tsclients/shared/host-transport/remote/__tests__/grant-client.test.tsclients/shared/host-transport/remote/__tests__/logical-stream.test.tsclients/shared/host-transport/remote/__tests__/mux-frame.test.tsclients/shared/host-transport/remote/__tests__/noise-channel.test.tsclients/shared/host-transport/remote/__tests__/scheduler.test.tsclients/shared/host-transport/remote/active-remote-sessions.tsclients/shared/host-transport/remote/chunker.tsclients/shared/host-transport/remote/config.tsclients/shared/host-transport/remote/create-remote-transport.tsclients/shared/host-transport/remote/grant-client.tsclients/shared/host-transport/remote/index.tsclients/shared/host-transport/remote/logical-stream.tsclients/shared/host-transport/remote/noise-channel.tsclients/shared/host-transport/remote/relay-socket.tsclients/shared/host-transport/remote/remote-host-messenger.tsclients/shared/host-transport/remote/remote-session.tsclients/shared/host-transport/remote/remote-stream-client.tsclients/shared/host-transport/remote/scheduler.tsclients/shared/host-transport/speech-stream-client.tsclients/shared/host-transport/terminal-stream-client.tsclients/shared/host-transport/worktree-delete-stream-client.tsclients/shared/host-transport/ws-rpc-client.tsclients/shared/host-transport/ws-stream-client.tsclients/shared/platform/runner-host.tsclients/traycer-cli/src/commands/__tests__/cli-entrypoint-registration.test.tsclients/traycer-cli/src/commands/__tests__/host-update.test.tsclients/traycer-cli/src/commands/host-update.tsclients/traycer-cli/src/host/__tests__/update-progress-marker.test.tsclients/traycer-cli/src/host/update-progress-marker.tsclients/traycer-cli/src/index.tsclients/traycer-cli/src/installer/__tests__/install.test.tsclients/traycer-cli/src/installer/index.tsclients/traycer-cli/src/installer/install.tsclients/traycer-cli/src/installer/uninstall.tsclients/traycer-cli/src/runner/errors.tsclients/traycer-cli/src/service/__tests__/health-probe.test.tsclients/traycer-cli/src/service/health-probe.tsclients/traycer-cli/src/store/__tests__/paths.test.tsclients/traycer-cli/src/store/paths.tsprotocol/package.jsonprotocol/scripts/snapshot-released-stream-method-names.tsprotocol/scripts/snapshot-stream-support-matrix.tsprotocol/scripts/snapshot-support-matrix.tsprotocol/src/crypto/noise/__tests__/cipher-state.test.tsprotocol/src/crypto/noise/__tests__/concurrency.test.tsprotocol/src/crypto/noise/__tests__/interop.test.tsprotocol/src/crypto/noise/__tests__/noise-nk-vectors.test.tsprotocol/src/crypto/noise/__tests__/replay-window.test.tsprotocol/src/crypto/noise/bytes.tsprotocol/src/crypto/noise/cipher-state.tsprotocol/src/crypto/noise/constants.tsprotocol/src/crypto/noise/errors.tsprotocol/src/crypto/noise/handshake-state.tsprotocol/src/crypto/noise/index.tsprotocol/src/crypto/noise/primitives.tsprotocol/src/crypto/noise/replay-window.tsprotocol/src/crypto/noise/session.tsprotocol/src/crypto/noise/symmetric-state.tsprotocol/src/crypto/noise/types.tsprotocol/src/framework/rpc-manifest.tsprotocol/src/host-transport/mux.tsprotocol/src/host/RELEASE-INVARIANT.mdprotocol/src/host/__tests__/__fixtures__/released-stream-method-names.tsprotocol/src/host/__tests__/__fixtures__/stream-support-matrix.tsprotocol/src/host/__tests__/__fixtures__/support-matrix.tsprotocol/src/host/__tests__/released-stream-surface-compat.test.tsprotocol/src/host/__tests__/two-sided-release-invariant.test.tsprotocol/src/host/__tests__/two-sided-stream-release-invariant.test.tsprotocol/src/host/host-status.tsprotocol/src/host/index.tsprotocol/src/host/registry.tsprotocol/src/host/status/contracts.tsprotocol/src/host/workspace/contracts.tsprotocol/src/host/workspace/unary-schemas.ts
grant-client.ts hand-defined its own zod schema and imported zod directly, the only place in clients/shared (a code-only lib with no dependency list of its own) to import a third-party package outside what its consumers already hoist. clients/shared has an existing zod@3/zod@4 version conflict elsewhere in the tree that keeps bun from hoisting a single zod to the workspace root, so this broke module resolution for whichever app bundled it (gui-app, desktop) without itself declaring zod. Every other CS wire-response schema (host-status.ts, git-schemas.ts, provider-schemas.ts, worktree-schemas.ts, ...) already lives in @traycer/protocol, which owns zod directly. Move the attach-grant response schema there (protocol/src/host/attach-grant.ts) and have grant-client.ts import it, matching remote-fetcher.ts's existing pattern for the sibling /api/v3/hosts endpoint. Also fixes a merge-induced compile break: a gui-app test fixture's codex sessionAnchor was missing the coveredUntilMessageId field development's fake-context-coverage work added to every session-anchor variant. Signed-off-by: Hardik Shingala <hardik@traycer.ai>
Address valid CodeRabbit review comments across protocol, clients/shared, gui-app, desktop, and traycer-cli. Security/correctness: - fail closed on incomplete remote directory rows (host-messenger and buildHostStreamClient no longer fall through to plain WS without Noise) - cap frame size in decodeMuxFrame; validate frame-type byte - strict hex decode in noise/bytes; guard replay-window commit() - win32: remove existing junction reparse point before rename in flipHostInstallPointer (MoveFileEx cannot replace a junction) Bugs: - LogicalStream marks itself disposed on close (no spurious CLOSE frame) - drop volatile status from remote transport cache key - StrictMode-safe eager start via explicit autoStart param - reconnect resize ordering in terminal-session-store - host-version mutation captures host in onMutate; a11y PathChip button; Enter-key submit guard; version-skew direction flag - guard marker writes / rollback in host-update; propagate teardown cause; guard scheduler onWriteError; log swallowed bearer read Maintainability/tests: - share HOST_VERSION_PATTERN via @traycer/protocol/host/version - shared symlink-pointer resolver in installer; ipc-contract re-exports - new tests (host-version-policy-fetcher, one-shot-stream throw path, shared runner-host factory) and copy/nitpick fixes Deferred as tracked follow-ups: duplicate host row on enrollment retry, legacy-migration crash-safety, viewer-reachability writer (S2). Signed-off-by: Hardik Shingala <hardik@traycer.ai>
CodeRabbit review — resolution summaryAddressed in 02d9673. Every inline thread has been resolved; rationale is on each thread. Full audit lives in the Traycer epic artifact. Tally: 37 valid (fixed) · 3 false-positive · 4 arch-decision — plus all 3 outside-diff comments fixed. Fixed highlights (valid):
False positives (not changed, resolved with reasoning):
Arch-decision / deferred (tracked follow-up):
|
… gaps
Findings from a post-implementation soundness review of the remote-host
architecture, closed as one client-side fix set plus a follow-up hardening
pass on the two gaps the review's own re-verification found.
Session collapse (one E2E session per host, not per consumer):
- get-or-create cache in active-remote-sessions keyed on
(hostId, userId, hostPublicKey, relayAttachUrl); ref-counted, torn down
immediately (synchronously) on last release, never kept warm
- acquire moved from useMemo into useEffect bodies across the render-path
hooks so React StrictMode's double-invoke can never orphan a refcount
- canonical remote-aware owner identity (transport-key.ts) adopted by the
app-wide HostStreamProvider, chat/terminal durable registries, and epic
session mounting, so a same-host public-key rotation (re-enrollment /
corruption recovery) forces those owners to release the stale session and
acquire a fresh one instead of looping a doomed re-handshake forever
- two upstream notification gaps fixed so that rotation is actually visible
to the owners above: HostClient's sameHostTransport gate and
use-host-directory-entry's reference-stability cache both compared on
public key now, not just hostId/kind/url/version/status
host_attached stale-Noise resume:
- a detached-then-reattached host leg now routes through the existing full
reconnect path (fresh NoiseChannel + open{bearer}) instead of a silent
scheduler resume against Noise state the host already discarded
Chunk reassembly:
- shared, parametrized conformance spec exercised by both the client and
host chunk reassemblers, so the two copies can't drift silently
Silent-drift guards:
- host RPC bootstrap manifests derived from the registry instead of a
hand-synced fixture
- host-status DTOs parsed with .strict() end to end, backed by one golden
fixture shared with authn-v3's own serializer test
- property test pinning that no mux routing field is externalized outside
the Noise AEAD without being fed as associated data
Signed-off-by: Hardik Shingala <hardik@traycer.ai>
…e-host-support # Conflicts: # clients/gui-app/src/lib/host/use-durable-stream-transport.ts
…e-host-support # Conflicts: # clients/gui-app/src/lib/registries/terminal-session-registry.ts # clients/gui-app/src/stores/terminals/__tests__/terminal-session-store.test.ts # clients/gui-app/src/stores/terminals/terminal-session-store.ts
…, not concrete WsStreamClient Matches TerminalStreamClient/ChatStreamClient's existing pattern so a remote transport can substitute for the local WsStreamClient (T14). The merge of origin/main's new resources.subscribe support typed it against the concrete WsStreamClient, which doesn't satisfy the IHostStreamClient the remote-host provider tree hands out. Signed-off-by: Hardik Shingala <hardik@traycer.ai>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
clients/gui-app/src/hooks/host/use-host-client-for.ts (1)
54-57: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDo not expose the previous client while the new binding effect catches up.
Because
bindingis updated in a passive effect, a render aftertarget,requestContext, oruserIdchanges can still return the prior host client until Line 196/200/232 runs. Key the binding and returnnullunless it matches the current inputs to avoid one-render stale host/auth usage.Proposed direction
interface HostClientBinding { readonly client: HostClient<HostRpcRegistry>; readonly remoteTransport: RemoteHostTransport< HostRpcRegistry, HostStreamRpcRegistry > | null; + readonly hostId: string; + readonly websocketUrl: string; + readonly requestContext: RequestContext; + readonly userId: string; }- setBinding({ client, remoteTransport: built.remoteTransport }); + setBinding({ + client, + remoteTransport: built.remoteTransport, + hostId: target.hostId, + websocketUrl: target.websocketUrl, + requestContext, + userId, + });Then gate the return value against the current
target,requestContext, anduserIdbefore returningbinding.client.Also applies to: 194-237
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@clients/gui-app/src/hooks/host/use-host-client-for.ts` around lines 54 - 57, The host client hook is returning a stale client before the passive effect updates the binding, so key the binding to the current inputs and avoid exposing the previous client during a target, requestContext, or userId change. Update use-host-client-for’s binding logic and the return path around the client selection so it only returns binding.client when the binding still matches the current target, requestContext, and userId, otherwise return null until the effect catches up.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@clients/gui-app/src/hooks/host/use-host-client-for.ts`:
- Around line 65-73: `buildTransientHostClient` currently only guards against a
missing `websocketUrl`, which still allows remote `HostDirectoryEntry` rows to
be opened with a plain `WsRpcClient`. Update the helper to fail closed by
checking the target’s locality before constructing the client, and return `null`
for non-local targets so remote rows cannot bypass the Noise session lifecycle
managed by `useHostClientFor`. Keep the existing null handling and apply the new
guard in the same branch where `buildTransientHostClient` creates the transient
client.
---
Outside diff comments:
In `@clients/gui-app/src/hooks/host/use-host-client-for.ts`:
- Around line 54-57: The host client hook is returning a stale client before the
passive effect updates the binding, so key the binding to the current inputs and
avoid exposing the previous client during a target, requestContext, or userId
change. Update use-host-client-for’s binding logic and the return path around
the client selection so it only returns binding.client when the binding still
matches the current target, requestContext, and userId, otherwise return null
until the effect catches up.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI (base), Organization UI (inherited)
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8ce190fb-e9f3-4e34-9b88-82fb6a5d5df6
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
clients/desktop/src/electron-main/ipc/__tests__/runner-ipc.test.tsclients/gui-app/src/components/layout/__tests__/sign-in-button.test.tsxclients/gui-app/src/hooks/host/use-host-client-for.tsclients/gui-app/src/lib/registries/terminal-session-registry.tsclients/gui-app/src/stores/epics/canvas/__tests__/host-binding.test.tsclients/gui-app/src/stores/terminals/__tests__/terminal-session-store.test.tsclients/gui-app/src/stores/terminals/terminal-session-store.tsclients/shared/host-transport/resources-stream-client.tsclients/shared/host-transport/terminal-stream-client.tsprotocol/src/host/registry.ts
💤 Files with no reviewable changes (4)
- clients/shared/host-transport/resources-stream-client.ts
- clients/shared/host-transport/terminal-stream-client.ts
- clients/gui-app/src/stores/terminals/tests/terminal-session-store.test.ts
- clients/gui-app/src/stores/terminals/terminal-session-store.ts
…fresh Reshape RemoteHostFetcher to return a discriminated outcome (hosts / signed-out / failed) so HostDirectoryService.refresh() can retain the last-known remote entries on a transient fetch failure instead of clearing them and unbinding an active remote selection. Signed-out remains a legitimate clear. refresh() is now single-flight so upcoming interval + open-time triggers coalesce. (T20, audit P4) Signed-off-by: Hardik Shingala <hardik@traycer.ai>
…ign-in/picker-open HostDirectoryService now owns a 15s refresh poll (paused while the tab is hidden, immediate refresh on visibility return) alongside the existing startup fetch, so remote hosts added/removed after launch reach every picker within one tick instead of staying stale for the session. HostRuntime.start() also triggers a directory refresh on every request-context transition (sign-in/sign-out/user switch), so a fresh sign-in populates remotes immediately rather than waiting for the next poll. A shared useRefreshHostDirectoryOnOpen hook fires an opportunistic refresh at the moment a user actually looks at a host list: the global HostPicker dialog, the HostOnlySelect dropdown, WorktreePickerHostSection, and MobileHostGate's cardinality evaluation. Cheap thanks to T20's single-flight refresh(). (T21, audit P1-P3) Signed-off-by: Hardik Shingala <hardik@traycer.ai>
…snapshots, label host lifecycle section Settings > Agents (agent-selection-guide) and Settings > General > File Edit Snapshots each get a panel-local host selector using the same transient useHostClientFor + scoped HostRuntimeContext pattern already used by Settings > Worktrees/Providers - selecting a host here never rebinds the app-wide active host. The snapshots row's destructive clear confirmation now names the target host. Settings > Host wraps its local-only CLI lifecycle rows (install, update, restart, register/deregister, rename) in a clearly labeled 'This machine' section, distinguishing them from the cross-device My Hosts list above - copy/structure only, no functional change. Extracted settings-host-select.tsx's non-component helpers into settings-host-labels.ts so the file exports only the SettingsHostSelect component (react-refresh/only-export-components). Also widens hostDirectoryEntryEquals in use-host-directory-entry.ts to exported (pure visibility change, no behavior change) since a concurrent in-flight ticket's new hook reuses it. (T23, audit G4-G6) Signed-off-by: Hardik Shingala <hardik@traycer.ai>
…e active host NotificationsSessionProvider now binds to useReactiveLocalHostEntry() (new hook - a reactive, non-app-wide-rebinding read of HostDirectoryService.getLocalEntry(), reusing use-host-directory-entry's field-equality cache) paired with the existing transient useHostStreamClientFor, rather than useReactiveActiveHostId() + the app-wide useWsStreamClient(). Selecting a remote host anywhere else in the app no longer moves or drops the user's notification stream. Teardown/reopen is now keyed on the resolved stream client's object identity rather than a hostId string, so it correctly follows a local host respawn (same hostId, new endpoint - a new client object) while staying inert across an active-host switch elsewhere (same local client, no reconnect). No local host (browser/mobile shells) leaves the stream simply unopened. (T25, audit G8) Signed-off-by: Hardik Shingala <hardik@traycer.ai>
…dge cross-host open-existing rows The command palette's 'Create new terminal' offered folder rows scoped to the active host only, with no way to pick a different host (audit G2). Extracted the sidebar '+' popover's host+folder selection into a shared NewTerminalPickerBody (host section, folder list, launch action) so the logic lives in exactly one place; the palette row now opens NewTerminalDialogHost, a new per-tab dialog mounted alongside NewConversationModalHost that reuses the same body and launches through openTileIntoTargetGroup, so the created terminal's persisted hostId reflects the explicitly picked host. Open-existing chat rows (audit G3) now carry a real host badge (CommandItem.hostBadge, rendered by SubpageView) whenever the chat's own hostId differs from the active host; no badge in the common single-host case. Terminal rows stay unbadged since terminal.list is only ever issued against the active host's client today - there is no cross-host terminal-listing plumbing to badge against (flagged as a follow-up, not invented here). TUI-agent rows carry a real hostId too and could be badged the same way chats are; left unbadged as out of this ticket's scope, also flagged as a follow-up. Artifacts verified host-agnostic, untouched. hostBadge is an optional string field rather than string-or-null - a deliberate, documented exception: making it required would force a no-op null value onto ~20 unrelated CommandItem literals across sources this ticket doesn't touch. (T22, audit G2-G3) Signed-off-by: Hardik Shingala <hardik@traycer.ai>
…restarts Persist HostDirectoryService's explicit host selection so creation surfaces (composer, new-conversation modal, TUI-agent launch) reopen against the previously chosen host instead of always snapping back to the local machine. Restoration is evaluated once during start(), gated so the local-host auto-promotion can't lock in a default binding before the remembered host gets its one shot, and gives up in favor of today's default-entry behavior the moment the initial refresh resolves if that host is offline or absent - startup is never blocked waiting for it. A shell that ends up fully unbound after that first attempt (no local host, remote list not yet loaded) gets exactly one more restore try, consumed by the next refresh that actually delivers remote entries - this is what lets it pair with the prior ticket's sign-in-triggered refresh for web/mobile shells. Explicitly clearing the selection erases the remembered host rather than persisting a "cleared" marker, so a future launch still falls back to the local default instead of staying unbound forever. Signed-off-by: Hardik Shingala <hardik@traycer.ai>
…ener badges, settings panels, and terminal creation Settings ▸ File edit snapshots and ▸ Agent instructions could silently fall back to reading/writing through the active host's client once a picked non-active host either vanished from the directory or was still mid-connect - a destructive action or a device-scoped editor would then target the wrong machine with no visible sign anything was off. Both sections now share a useSettingsHostScope hook that distinguishes no override / still connecting / vanished / resolved, and render an explicit disabled "host unavailable" state instead of ever falling back silently. Also fixes the stale "this device" copy in the guide's revert-confirm dialog, the host-picker's position/label jump between loading and loaded states, the inconsistent empty-host-list fallback, missing directory-refresh-on-open wiring on SettingsHostSelect, the "This machine" heading mismatched against the sibling "My Hosts" heading, and consolidates four duplicated host-option-label helpers into settingsHostOptionLabel. Elsewhere in the same review pass: HostDirectoryService could burn its one-shot post-startup restore on a near-miss delivery instead of staying armed for the batch that actually contains the remembered host, and its poll interval wasn't rearmed after a visibility-triggered refresh, doubling the next tick. The Chats opener's cross-host badge could false-positive while the active host id was still unresolved at boot, and blank directory labels rendered an empty badge instead of falling back to the raw hostId. MobileHostGate refreshed the directory a second time on every signed-in mount even though HostDirectoryService had just refreshed it during start(). The palette's new-terminal dialog had no close button and had lost its epic-specific "no directories" empty-state copy in the extraction to the shared worktree folder list. Signed-off-by: Hardik Shingala <hardik@traycer.ai>
… runtime semantics The /stream release-invariant guards modeled compatibility on the unary /rpc whole-manifest gate, where a method name present on only one peer is handshake-fatal for the whole connection. /stream does not work that way: each subscribe runs checkStreamMethodCompatibility per method, and a host-missing method degrades that one feature quietly via the client capability cache. A new stream method name (resources.subscribe, merged after the host-v1.0.0 baseline) is therefore additive, not a break. - released-stream-surface-compat: assert baselined names are a subset of today's registry (no removals) instead of exact-set equality; additions are allowed. - two-sided-stream-release-invariant: replace whole-manifest checkStreamCompatibility with per-method checkStreamMethodCompatibility over each baseline's own methods, in both bridge directions. - add a degrade-contract test: names absent from a baseline do not affect the per-method result of its baselined methods. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Tanveer Gill <tanveer@traycer.ai>
|
Pushed Why the old assertions were wrong: both tests treated the frozen What actually matters, and what the rewritten tests now check:
Fixtures and the snapshot generation scripts are untouched — the frozen baseline stays an honest record of Verified: the 2 target test files pass (4/4), full This is orthogonal to the branch's current merge-conflict state against |
Four conflicts, plus three defects the conflict markers did not cover.
Resolved conflicts:
- terminals-subpage.ts - keep both: our 4-arg openerExistingLeaf (the
cross-host badge slot) and main's `currentCwd` on the title.
- epic-session-provider.tsx / its test - keep both: independent
declarations added either side of an empty base hunk.
- notifications-session-provider.tsx - take MAIN. #906 replaced the
notifications-room awareness reader with host-selected activity planes
and moved it out of the cloud-only branch; our side contributed only the
local-host pin, so this is main's structure with `localStreamClient`.
Found by tsc, in hunks git merged cleanly:
- notifications-session-provider.tsx:471 called `wsStreamClient`, which
this branch renamed to `localStreamClient`. It is not a variable in that
file at all - only a property name - so it would have thrown at runtime.
- epic-surface-isolation.test.tsx gained TWO `getActiveHost` keys, one per
side. JS silently keeps the last, so the fake would have answered `null`
instead of the host entry and quietly weakened the test. Kept main's
real entry, which agrees with `resolveHostById` in the same object.
- AgentActivityStreamClient arrived typed to the concrete WsStreamClient;
every sibling here takes IHostStreamClient so a remote host can supply
its own transport, and the class only calls `.subscribe()`. Widened.
Also: adding the activity disposer made the reopen effect test a four-term
boolean twice, tripping the complexity ceiling (18 > 16). Extracted
`anyStreamOpen`; the two call sites stay separate reads because the second
must observe the `tearDown()` between them.
Verified: compile green (5 projects), lint clean, providers 192/192.
Full suite showed 9 failures, all load-induced - every one passes in
isolation (desktop 2/2, cli 41/41, gui-app 233/233) and none of their files
are touched by this merge. Machine load was 8-10 under parallel worktrees.
Signed-off-by: Hardik Shingala <hardik@traycer.ai>
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
|
@coderabbitai approve |
✅ Action performedComments resolved and changes approved. |
…rface The remote-host transport (#188) seams every stream wrapper on IStreamClient so remote sessions can stand in for the local WebSocket client. The two managed-command wrappers landed in parallel and still demanded the concrete WsStreamClient, which no longer typechecks against the interface the gui-app hooks now hand out. Migrate them exactly like their chat/terminal siblings. Signed-off-by: Amiteshwar Randhawa <amiteshwar04@gmail.com>
MobileRunnerHost predated the remote-host transport (#188) and the Devices & Sessions surface (#757), so it implemented 27 of IRunnerHost's 37 members and the package did not compile. Nothing here reimplements a request. This shell owns its requests in-process, so - exactly as MockRunnerHost does, and as the interface's own doc comments prescribe for browser/dev shells - each member delegates to the same shared `*ViaHttp` helper the desktop's Electron main process calls: - listRegisteredHosts -> fetchRegisteredHostsViaHttp - listUserSessions -> listUserSessionsViaHttp (real abort: owning the request in-process, the caller's signal reaches fetch rather than only settling the caller) - revokeUserSession, revokeAllSessions -> revoke*ViaHttp, with the retained step-up credential attached and dropped on a step-up-required verdict, matching auth-ipc.ts - mintHostCredential -> mintHostCredentialViaHttp on the caller's own bearer; the mint is not step-up gated - requestStepUpChallenge, verifyStepUpChallenge -> step-up helpers; only expiry metadata leaves the boundary, the bearer stays inside - updateHostVersionPolicy -> updateHostVersionPolicyViaHttp; it addresses any registered host by id, not a local one The retained step-up credential and its self-nulling accessor mirror the desktop closure and MockRunnerHost so the three cannot drift. getLastKnownLocalHostId returns null: a phone never runs a host, so there is no pid metadata to read. relayBaseUrl is baked like the desktop's, with the same TRAYCER_DEV_RELAY_BASE_URL dev override. Also updates four members that drifted: requestHostRespawn now resolves `declined` (a phone cannot restart a host) instead of void, the token refresh passes clientKind: null (neither cli nor desktop), notifications gain onForegroundDisplay as a no-op (a phone has no second focused window), and the web entry's RemoteHostFetcher returns the discriminated outcome. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Devansh Kukreja <devansh@traycer.ai>
…hen the shell has no native dialog `kind: "remote"` no longer describes where a host is - it describes how it is reached. Since #188 a remote host means the relay path: an authn-minted attach grant, then a Noise-NK handshake against the host's registry-published public key. `createRemoteHostTransport` never dials the entry's `websocketUrl`. The mobile dev scaffolding still baked its host as `remote` with a 127.0.0.1 address and no public key, so `isRemoteHostDirectoryEntry` rejected it, `remoteTransportKey` returned null, and every request failed before it left the client: HostRpcError: Remote host '<id>' does not expose a valid remote transport The baked entry is `local` now, which is what it has always been: a 127.0.0.1 host the browser dials directly on its own `websocketUrl` - the meaning `host-directory.ts` gives that kind. That relabel exposed a latent gap in the folder-add path. It picked the native OS dialog from the host kind alone, but sharing the client machine is necessary and not sufficient: a browser/phone shell installs a no-op `pickFolders` and reports `canPickNatively: false`. A local host seen from such a shell resolved an empty pick and silently added nothing. The choice now consults the shell capability as well - which is what the call site's own comment and `IWorkspaceFoldersHost` already prescribed, and what kept the gap unreachable only while the dev host was mislabelled `remote`. Scope note: this makes the browser/simulator loop exercise the direct local socket. The relay path a real phone takes still needs its own dev setup and is not covered here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Devansh Kukreja <devansh@traycer.ai>
MobileRunnerHost predated the remote-host transport (#188) and the Devices & Sessions surface (#757), so it implemented 27 of IRunnerHost's 37 members and the package did not compile. Nothing here reimplements a request. This shell owns its requests in-process, so - exactly as MockRunnerHost does, and as the interface's own doc comments prescribe for browser/dev shells - each member delegates to the same shared `*ViaHttp` helper the desktop's Electron main process calls: - listRegisteredHosts -> fetchRegisteredHostsViaHttp - listUserSessions -> listUserSessionsViaHttp (real abort: owning the request in-process, the caller's signal reaches fetch rather than only settling the caller) - revokeUserSession, revokeAllSessions -> revoke*ViaHttp, with the retained step-up credential attached and dropped on a step-up-required verdict, matching auth-ipc.ts - mintHostCredential -> mintHostCredentialViaHttp on the caller's own bearer; the mint is not step-up gated - requestStepUpChallenge, verifyStepUpChallenge -> step-up helpers; only expiry metadata leaves the boundary, the bearer stays inside - updateHostVersionPolicy -> updateHostVersionPolicyViaHttp; it addresses any registered host by id, not a local one The retained step-up credential and its self-nulling accessor mirror the desktop closure and MockRunnerHost so the three cannot drift. getLastKnownLocalHostId returns null: a phone never runs a host, so there is no pid metadata to read. relayBaseUrl is baked like the desktop's, with the same TRAYCER_DEV_RELAY_BASE_URL dev override. Also updates four members that drifted: requestHostRespawn now resolves `declined` (a phone cannot restart a host) instead of void, the token refresh passes clientKind: null (neither cli nor desktop), notifications gain onForegroundDisplay as a no-op (a phone has no second focused window), and the web entry's RemoteHostFetcher returns the discriminated outcome. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Devansh Kukreja <devansh@traycer.ai>
…hen the shell has no native dialog `kind: "remote"` no longer describes where a host is - it describes how it is reached. Since #188 a remote host means the relay path: an authn-minted attach grant, then a Noise-NK handshake against the host's registry-published public key. `createRemoteHostTransport` never dials the entry's `websocketUrl`. The mobile dev scaffolding still baked its host as `remote` with a 127.0.0.1 address and no public key, so `isRemoteHostDirectoryEntry` rejected it, `remoteTransportKey` returned null, and every request failed before it left the client: HostRpcError: Remote host '<id>' does not expose a valid remote transport The baked entry is `local` now, which is what it has always been: a 127.0.0.1 host the browser dials directly on its own `websocketUrl` - the meaning `host-directory.ts` gives that kind. That relabel exposed a latent gap in the folder-add path. It picked the native OS dialog from the host kind alone, but sharing the client machine is necessary and not sufficient: a browser/phone shell installs a no-op `pickFolders` and reports `canPickNatively: false`. A local host seen from such a shell resolved an empty pick and silently added nothing. The choice now consults the shell capability as well - which is what the call site's own comment and `IWorkspaceFoldersHost` already prescribed, and what kept the gap unreachable only while the dev host was mislabelled `remote`. Scope note: this makes the browser/simulator loop exercise the direct local socket. The relay path a real phone takes still needs its own dev setup and is not covered here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Devansh Kukreja <devansh@traycer.ai>
What
Client + protocol side of Remote Host Support — the persistent, multiplexed, end-to-end-encrypted transport that lets the Traycer app reach a host through a relay, plus the My Hosts UI. Pairs with the internal control-plane / relay / host-daemon branch (the cloud + host-daemon side lives outside this repo).
Highlights
protocol/crypto/noise): in-houseNoise_NK_25519_AESGCM_SHA256over WebCrypto (+@nobleX25519), validated against official Noise test vectors; AEAD replay window (monotonic counter + sliding bitmask), forward secrecy per session, fail-closed nonce guard.protocol/host-transport/mux): the single wire-contract source both client and host compile against (frame enums/flags/QoS, 1 MiB cap, encode/decode).clients/shared/host-transport/remote): persistent Noise-NK + mux behindIHostMessenger/IStreamClient— priority scheduler, per-session credits, 64 KiB chunking with fail-closed reassembly, full-attach resume (re-runsopen{bearer}, never restores identity from a ticket), reconnect ready-boundary backoff.clients/gui-app): honest status-DTO rendering (live-session override, presence-degraded, timestamped reachability provenance), remote transport selection bykind(default + tab scope), terminal dead-tile state split, direction-aware version-skew copy, enrollment proof-of-possession, remote workspace path picker.clients/traycer-cli): host update-progress + rollback-aware installer, first-start health probe separating binary health from CS reachability.Compatibility
No new RPC method names — new capabilities fold onto existing versioned methods (
host.statusv1.1,workspace.prepareFoldersv1.1). Released-surface + two-sided-release-invariant guards extended to the stream registry.Verification
bun run compilegreen across the workspace.--max-warnings 0clean.