docs: remove obsolete daemon session connection architecture specs
This commit is contained in:
@@ -1,101 +0,0 @@
|
|||||||
# Daemon Session and Connection Plan Review
|
|
||||||
|
|
||||||
Date: 2026-02-20
|
|
||||||
Plan reviewed: `docs/daemon-session-connection-architecture-spec.md`
|
|
||||||
Change set reviewed: current uncommitted workspace changes
|
|
||||||
|
|
||||||
## Verdict
|
|
||||||
|
|
||||||
Partial alignment.
|
|
||||||
|
|
||||||
The local changes implement major parts of the plan (runtime-owned host connectivity, disposed terminal state in `DaemonClient`, connect timeout path, relay multi-socket handling, and first-open UX gating). However, there are still blocking gaps against non-negotiable invariants and migration requirements.
|
|
||||||
|
|
||||||
## Findings
|
|
||||||
|
|
||||||
### 1. `clientSessionKey` is still optional, with silent fallback identity generation (Blocking)
|
|
||||||
|
|
||||||
Plan requirements impacted:
|
|
||||||
- Invariant 1: daemon session keyed by `clientSessionKey`
|
|
||||||
- Invariant 9: no silent fallback identities
|
|
||||||
- Migration steps 1-2: introduce/persist and require session key on direct + relay
|
|
||||||
|
|
||||||
Evidence:
|
|
||||||
- `packages/server/src/client/daemon-client.ts:148` keeps `clientSessionKey` optional.
|
|
||||||
- `packages/server/src/client/daemon-client.ts:373`-`packages/server/src/client/daemon-client.ts:377` silently generates `clt_${safeRandomId()}` when missing.
|
|
||||||
- `packages/app/src/hooks/use-daemon-client.ts:23`-`packages/app/src/hooks/use-daemon-client.ts:35` creates `DaemonClient` without providing `clientSessionKey`.
|
|
||||||
- `packages/app/src/utils/test-daemon-connection.ts:51`-`packages/app/src/utils/test-daemon-connection.ts:63` builds probe clients/URLs without `clientSessionKey`.
|
|
||||||
|
|
||||||
Impact:
|
|
||||||
- Session continuity is not uniformly guaranteed for all app->daemon paths.
|
|
||||||
- Identity fallback remains implicit instead of failing fast.
|
|
||||||
|
|
||||||
Coding-standards impact:
|
|
||||||
- Violates “prefer explicit error over fallback” for identity-critical behavior.
|
|
||||||
|
|
||||||
### 2. Host runtime async generation guards are incomplete for probe-side effects (High)
|
|
||||||
|
|
||||||
Plan requirements impacted:
|
|
||||||
- HostRuntime machine rule: all async side effects generation-guarded; stale async completions must not patch current state.
|
|
||||||
|
|
||||||
Evidence:
|
|
||||||
- `packages/app/src/runtime/host-runtime.ts:506`-`packages/app/src/runtime/host-runtime.ts:611` (`runProbeCycleNow`) performs async probes and then mutates snapshot/switches connection, but has no request-generation guard equivalent to `switchRequestVersion`.
|
|
||||||
- Generation checks exist only in switch flow (`packages/app/src/runtime/host-runtime.ts:627`-`packages/app/src/runtime/host-runtime.ts:764`).
|
|
||||||
|
|
||||||
Impact:
|
|
||||||
- Overlapping probe cycles can apply stale probe snapshots or stale switching decisions.
|
|
||||||
|
|
||||||
### 3. Directory sync machine is only partially modeled (Medium)
|
|
||||||
|
|
||||||
Plan requirements impacted:
|
|
||||||
- Section 7.3 machine shape (`initial_loading`, `revalidating`, `error_before_first_success`, `error_after_ready`).
|
|
||||||
|
|
||||||
Evidence:
|
|
||||||
- Runtime exposes `"idle" | "loading" | "ready" | "error"` only (`packages/app/src/runtime/host-runtime.ts:25`-`packages/app/src/runtime/host-runtime.ts:29`).
|
|
||||||
- “Revalidating” and both error phases are inferred indirectly via `hasEverLoadedAgentDirectory` rather than explicit machine states (`packages/app/src/runtime/host-runtime.ts:41`, `packages/app/src/runtime/host-runtime.ts:480`-`packages/app/src/runtime/host-runtime.ts:493`).
|
|
||||||
|
|
||||||
Impact:
|
|
||||||
- Current behavior can work, but the plan’s explicit state model and transition clarity are not fully realized.
|
|
||||||
|
|
||||||
### 4. React still triggers connection lifecycle operations outside explicit user intent (Medium)
|
|
||||||
|
|
||||||
Plan requirements impacted:
|
|
||||||
- React integration contract: React should not coordinate connection lifecycle except explicit user intent.
|
|
||||||
|
|
||||||
Evidence:
|
|
||||||
- `packages/app/src/components/multi-daemon-session-host.tsx:36`-`packages/app/src/components/multi-daemon-session-host.tsx:44` calls `runtime.ensureConnectedAll()` and `runtime.runProbeCycleNow()` on app foreground transitions.
|
|
||||||
|
|
||||||
Impact:
|
|
||||||
- Lifecycle policy remains partly driven by React effect orchestration.
|
|
||||||
|
|
||||||
### 5. Loading/refresh policy remains distributed across multiple hooks (Low)
|
|
||||||
|
|
||||||
Plan + coding-standards impact:
|
|
||||||
- Plan favors machine-driven, centralized state decisions.
|
|
||||||
- Coding standards call out “distributed decisions and conditional accretion”.
|
|
||||||
|
|
||||||
Evidence:
|
|
||||||
- Similar refresh/sync logic appears in:
|
|
||||||
- `packages/app/src/hooks/use-sidebar-agents-list.ts`
|
|
||||||
- `packages/app/src/hooks/use-all-agents-list.ts`
|
|
||||||
- `packages/app/src/hooks/use-aggregated-agents.ts:45`-`packages/app/src/hooks/use-aggregated-agents.ts:81`
|
|
||||||
- `useAggregatedAgents.refreshAll` writes store directly without runtime sync state markers (`packages/app/src/hooks/use-aggregated-agents.ts:70`-`packages/app/src/hooks/use-aggregated-agents.ts:76`).
|
|
||||||
|
|
||||||
Impact:
|
|
||||||
- Greater risk of state drift and inconsistent loading semantics between surfaces.
|
|
||||||
|
|
||||||
## What is aligned well
|
|
||||||
|
|
||||||
- `DaemonClient` now has terminal `disposed` state and no-op `ensureConnected` in disposed state (`packages/server/src/client/daemon-client.ts`).
|
|
||||||
- Connect timeout path is explicit (`packages/server/src/client/daemon-client.ts:473`-`packages/server/src/client/daemon-client.ts:484`).
|
|
||||||
- Direct websocket URLs now support `clientSessionKey` (`packages/server/src/shared/daemon-endpoints.ts:64`-`packages/server/src/shared/daemon-endpoints.ts:77`).
|
|
||||||
- Relay and server now use session-style external keys and allow multi-socket client presence (`packages/server/src/server/relay-transport.ts:325`, `packages/server/src/server/websocket-server.ts`, `packages/relay/src/cloudflare-adapter.ts`).
|
|
||||||
- App-side host runtime is now the primary connectivity source for context/session wiring (`packages/app/src/contexts/daemon-connections-context.tsx`, `packages/app/src/components/multi-daemon-session-host.tsx`).
|
|
||||||
- Agent first-open vs revalidation UX behavior is substantially closer to plan (`packages/app/src/hooks/use-agent-screen-state-machine.ts`, `packages/app/src/screens/agent/agent-ready-screen.tsx`).
|
|
||||||
|
|
||||||
## Recommended next steps
|
|
||||||
|
|
||||||
1. Make `clientSessionKey` mandatory for app/runtime `DaemonClient` creation paths and throw explicit errors when missing.
|
|
||||||
2. Add probe-cycle generation tokens so stale probe results cannot update snapshots or trigger switches.
|
|
||||||
3. Promote directory sync to an explicit machine (or equivalent discriminated state) matching planned transition semantics.
|
|
||||||
4. Consolidate agent-directory refresh policy into one runtime-owned path and remove duplicate hook-level logic.
|
|
||||||
5. Add structured transition logging fields from section 12 (`serverId`, `clientSessionKey`/hash, `from/to/event`, `connection path`, `generation`, typed reason).
|
|
||||||
@@ -1,300 +0,0 @@
|
|||||||
# Daemon Session and Connection Architecture Specification
|
|
||||||
|
|
||||||
Status: Draft for implementation
|
|
||||||
Owner: App + Server + Relay
|
|
||||||
Last updated: 2026-02-20
|
|
||||||
|
|
||||||
## 1. Purpose
|
|
||||||
|
|
||||||
This spec defines the connection and session model across:
|
|
||||||
|
|
||||||
- Client app
|
|
||||||
- Relay
|
|
||||||
- Daemon server
|
|
||||||
|
|
||||||
The goal is to remove drift, make invalid states impossible, and move lifecycle decisions into explicit state machines outside React.
|
|
||||||
|
|
||||||
## 2. Scope
|
|
||||||
|
|
||||||
In scope:
|
|
||||||
|
|
||||||
- Session identity and lifetime
|
|
||||||
- Connection identity and lifetime
|
|
||||||
- Daemon client/transport ownership rules
|
|
||||||
- Host runtime manager behavior
|
|
||||||
- Relay responsibilities and protocol shape
|
|
||||||
- React integration boundaries
|
|
||||||
- UX loading contracts tied to machine state
|
|
||||||
- Required tests
|
|
||||||
|
|
||||||
Out of scope:
|
|
||||||
|
|
||||||
- Agent business logic (tool calls, model behavior)
|
|
||||||
- New UI visual design
|
|
||||||
|
|
||||||
## 3. Terminology
|
|
||||||
|
|
||||||
- `serverId`: stable daemon identity.
|
|
||||||
- `clientSessionKey`: stable client identity for one app install/profile. Persists across app restarts.
|
|
||||||
- `session`: daemon-side logical session associated with one `clientSessionKey`.
|
|
||||||
- `connection`: one physical socket path (direct or relay).
|
|
||||||
- `transport`: adapter that owns exactly one physical connection.
|
|
||||||
- `DaemonClient`: connection actor that owns one transport.
|
|
||||||
- `HostRuntimeManager`: per-host orchestrator that owns active and probe daemon clients.
|
|
||||||
- `probe client`: ephemeral daemon client used for latency/health probing only.
|
|
||||||
|
|
||||||
## 4. Non-Negotiable Invariants
|
|
||||||
|
|
||||||
1. Daemon session is keyed by `clientSessionKey`, not by transport.
|
|
||||||
2. Session survives disconnects and client restarts.
|
|
||||||
3. Client may open and close many connections over time for the same session.
|
|
||||||
4. `DaemonClient` owns exactly one transport; transport owns exactly one connection.
|
|
||||||
5. No separate ad-hoc `isConnected` state outside `DaemonClient` connection state.
|
|
||||||
6. Host runtime is the only source of truth for per-host connectivity in the app.
|
|
||||||
7. React does not coordinate connection lifecycle; React only renders machine snapshots and dispatches intents.
|
|
||||||
8. Probe clients never mutate active connection/session state.
|
|
||||||
9. No silent fallback identities. Missing required identity inputs are explicit errors.
|
|
||||||
|
|
||||||
## 5. Identity Model
|
|
||||||
|
|
||||||
## 5.1 Session Identity
|
|
||||||
|
|
||||||
- Required on every app->daemon connection (direct or relay): `clientSessionKey`.
|
|
||||||
- Daemon canonical external key:
|
|
||||||
- `externalSessionKey = session:${clientSessionKey}`
|
|
||||||
- If a socket reconnects with same key, daemon reattaches to existing session.
|
|
||||||
- If socket disconnects, daemon keeps session alive for reconnect grace period.
|
|
||||||
|
|
||||||
## 5.2 Connection Identity
|
|
||||||
|
|
||||||
- Connection identity is per-socket and ephemeral.
|
|
||||||
- Connection identity is **not** the session identity.
|
|
||||||
- A transport instance never needs a separate protocol-level `connectionId` to identify itself on disconnect; instance ownership is local and explicit.
|
|
||||||
|
|
||||||
## 6. Component Responsibilities
|
|
||||||
|
|
||||||
## 6.1 DaemonClient
|
|
||||||
|
|
||||||
- Owns one transport instance.
|
|
||||||
- Exposes one connection machine.
|
|
||||||
- Supports clean terminal disposal.
|
|
||||||
- Must not be resurrected after disposal.
|
|
||||||
- Emits typed connection state changes.
|
|
||||||
|
|
||||||
## 6.2 Transport
|
|
||||||
|
|
||||||
- Owns one connection (WebSocket or equivalent).
|
|
||||||
- No multiplexing of multiple peer connections in one transport instance.
|
|
||||||
- Must expose `open`, `message`, `error`, `close`.
|
|
||||||
- Must enforce connect timeout and report typed failure reason.
|
|
||||||
|
|
||||||
## 6.3 HostRuntimeManager (per host)
|
|
||||||
|
|
||||||
- Owns per-host source of truth snapshot.
|
|
||||||
- Owns active `DaemonClient`.
|
|
||||||
- May create many probe clients in parallel; they are ephemeral and isolated.
|
|
||||||
- Can switch active client cleanly by:
|
|
||||||
1. creating next client,
|
|
||||||
2. subscribing,
|
|
||||||
3. promoting snapshot generation,
|
|
||||||
4. disposing previous client.
|
|
||||||
- Serializes transitions so stale async completions cannot patch current state.
|
|
||||||
|
|
||||||
## 6.4 Relay
|
|
||||||
|
|
||||||
- Relay routes bytes; it does not own logical session policy.
|
|
||||||
- Control plane reports connected client sessions.
|
|
||||||
- Data plane maps one daemon data peer per client session key.
|
|
||||||
- Relay may have many client sockets for same `clientSessionKey` concurrently.
|
|
||||||
|
|
||||||
## 6.5 Daemon Server
|
|
||||||
|
|
||||||
- Owns canonical session map keyed by `externalSessionKey`.
|
|
||||||
- Reattaches sockets to existing session on reconnect.
|
|
||||||
- Session continuity must work identically for direct and relay.
|
|
||||||
|
|
||||||
## 7. State Machines
|
|
||||||
|
|
||||||
## 7.1 DaemonClient Machine
|
|
||||||
|
|
||||||
States:
|
|
||||||
|
|
||||||
- `idle`
|
|
||||||
- `connecting`
|
|
||||||
- `connected`
|
|
||||||
- `disconnected`
|
|
||||||
- `disposed` (terminal)
|
|
||||||
|
|
||||||
Events:
|
|
||||||
|
|
||||||
- `CONNECT_REQUEST`
|
|
||||||
- `TRANSPORT_OPEN`
|
|
||||||
- `TRANSPORT_CLOSE(reason)`
|
|
||||||
- `TRANSPORT_ERROR(reason)`
|
|
||||||
- `CONNECT_TIMEOUT`
|
|
||||||
- `RECONNECT_TIMER`
|
|
||||||
- `DISPOSE`
|
|
||||||
|
|
||||||
Rules:
|
|
||||||
|
|
||||||
- `disposed` is terminal. All events except idempotent `DISPOSE` are ignored.
|
|
||||||
- `ensureConnected` is no-op in `disposed`.
|
|
||||||
- `connecting` has bounded timeout.
|
|
||||||
- Any disconnect clears in-flight waiters tied to that connection.
|
|
||||||
|
|
||||||
## 7.2 HostRuntimeManager Machine (per host)
|
|
||||||
|
|
||||||
States:
|
|
||||||
|
|
||||||
- `booting`
|
|
||||||
- `no_connections`
|
|
||||||
- `selecting_active_connection`
|
|
||||||
- `connecting_active`
|
|
||||||
- `online`
|
|
||||||
- `degraded` (active exists but not connected)
|
|
||||||
- `error`
|
|
||||||
- `stopped`
|
|
||||||
|
|
||||||
Context:
|
|
||||||
|
|
||||||
- `activeClientRef`
|
|
||||||
- `activeConnectionRef`
|
|
||||||
- `generation`
|
|
||||||
- `probeResults`
|
|
||||||
- `lastError`
|
|
||||||
|
|
||||||
Rules:
|
|
||||||
|
|
||||||
- All async side effects are generation-guarded.
|
|
||||||
- Snapshot only changes through machine transitions.
|
|
||||||
- Probe transitions cannot directly mutate active client state.
|
|
||||||
- Active switch is atomic with generation increment.
|
|
||||||
|
|
||||||
## 7.3 Agent Directory Sync Machine (per host session mirror)
|
|
||||||
|
|
||||||
States:
|
|
||||||
|
|
||||||
- `idle`
|
|
||||||
- `initial_loading`
|
|
||||||
- `ready`
|
|
||||||
- `revalidating`
|
|
||||||
- `error_before_first_success`
|
|
||||||
- `error_after_ready` (non-blocking)
|
|
||||||
|
|
||||||
Rules:
|
|
||||||
|
|
||||||
- After first success, errors are non-blocking.
|
|
||||||
- Sidebar skeleton/loading is tied to this machine, not ad-hoc React flags.
|
|
||||||
|
|
||||||
## 8. Relay Contract
|
|
||||||
|
|
||||||
## 8.1 Required Behavior
|
|
||||||
|
|
||||||
- Client connects with `clientSessionKey`.
|
|
||||||
- Relay control reports client session presence changes.
|
|
||||||
- Daemon establishes peer data connection per `clientSessionKey`.
|
|
||||||
- Multiple client sockets with same `clientSessionKey` are allowed; relay does not enforce single client socket.
|
|
||||||
|
|
||||||
## 8.2 Prohibited Behavior
|
|
||||||
|
|
||||||
- Enforcing one client socket per `clientId` when that `clientId` is used as session identity.
|
|
||||||
- Treating replacement of one socket as session replacement.
|
|
||||||
|
|
||||||
## 8.3 Protocol Notes
|
|
||||||
|
|
||||||
- Keep protocol focused on session keys and control events.
|
|
||||||
- Do not introduce protocol `connectionId` for disconnect ownership.
|
|
||||||
- Internal debug correlation IDs are allowed in logs, not required on the wire.
|
|
||||||
|
|
||||||
## 9. React Integration Contract
|
|
||||||
|
|
||||||
1. React reads runtime snapshots via external store subscription.
|
|
||||||
2. React never mirrors connection state into local `useState`/`useRef`.
|
|
||||||
3. React never calls connection lifecycle APIs except explicit user intent events.
|
|
||||||
4. Complex transition logic stays in machines/reducers, not effects.
|
|
||||||
|
|
||||||
## 10. UX Contract
|
|
||||||
|
|
||||||
## 10.1 Agent First Open
|
|
||||||
|
|
||||||
- If agent history has never been loaded for that agent in this session:
|
|
||||||
- show full overlay spinner (centered, no text).
|
|
||||||
- do not show "refreshing history" toast.
|
|
||||||
- do not show empty "start chatting..." placeholder simultaneously.
|
|
||||||
|
|
||||||
## 10.2 Already Loaded Agent Revalidation
|
|
||||||
|
|
||||||
- Show existing toast for refresh/revalidation.
|
|
||||||
- Do not block with overlay.
|
|
||||||
|
|
||||||
## 10.3 Optimistic Agent Creation
|
|
||||||
|
|
||||||
- No loading overlay during optimistic create flow.
|
|
||||||
|
|
||||||
## 10.4 Errors
|
|
||||||
|
|
||||||
- Non-blocking indicator for revalidation failures after first successful load.
|
|
||||||
|
|
||||||
## 11. Retry and Timeout Policy
|
|
||||||
|
|
||||||
- Client connect timeout: bounded and explicit.
|
|
||||||
- Relay handshake timeout: bounded and explicit.
|
|
||||||
- Backoff retry for recoverable disconnects.
|
|
||||||
- Terminal `disposed` state disables retries.
|
|
||||||
- Retry reasons are typed and machine-readable.
|
|
||||||
|
|
||||||
## 12. Observability Requirements
|
|
||||||
|
|
||||||
Every connection/session event must log:
|
|
||||||
|
|
||||||
- `serverId`
|
|
||||||
- `clientSessionKey` (or hashed form)
|
|
||||||
- machine state transition (`from`, `to`, `event`)
|
|
||||||
- connection path (`direct`/`relay`)
|
|
||||||
- generation id (host runtime)
|
|
||||||
- typed reason/error code
|
|
||||||
|
|
||||||
## 13. Test Requirements
|
|
||||||
|
|
||||||
## 13.1 Unit
|
|
||||||
|
|
||||||
- `DaemonClient` transitions including terminal `disposed`.
|
|
||||||
- Host runtime generation guards prevent stale async writes.
|
|
||||||
- Agent directory sync machine transition coverage.
|
|
||||||
|
|
||||||
## 13.2 Integration
|
|
||||||
|
|
||||||
- Session continuity across disconnect/reconnect for direct.
|
|
||||||
- Session continuity across disconnect/reconnect for relay.
|
|
||||||
- Session continuity when switching active path direct <-> relay.
|
|
||||||
- Probe clients running in parallel do not affect active client/session.
|
|
||||||
|
|
||||||
## 13.3 E2E
|
|
||||||
|
|
||||||
- App can remain connected while switching network paths without losing agents.
|
|
||||||
- Sidebar and connection status converge correctly (no stuck connecting drift).
|
|
||||||
- First-open agent overlay contract and toast gating.
|
|
||||||
|
|
||||||
## 14. Migration Plan
|
|
||||||
|
|
||||||
1. Introduce and persist `clientSessionKey` on client.
|
|
||||||
2. Require session key for both direct and relay socket attach paths.
|
|
||||||
3. Update daemon session attach to use unified `externalSessionKey`.
|
|
||||||
4. Split relay daemon manager into:
|
|
||||||
- control manager
|
|
||||||
- per-peer connection instances
|
|
||||||
5. Remove single-client-socket relay enforcement for same session key.
|
|
||||||
6. Add `disposed` terminal state to `DaemonClient`.
|
|
||||||
7. Move remaining connection lifecycle logic from React effects into machines.
|
|
||||||
8. Add/expand tests before deleting legacy paths.
|
|
||||||
9. Delete legacy dual-state paths and fallback identity behavior.
|
|
||||||
|
|
||||||
## 15. Acceptance Criteria
|
|
||||||
|
|
||||||
- One canonical source of truth per host in runtime manager snapshot.
|
|
||||||
- No mirrored connection flags in React.
|
|
||||||
- No stuck `connecting` without timeout path.
|
|
||||||
- Session survives app restart and transport switch.
|
|
||||||
- Client can create/use many connections over time for same session.
|
|
||||||
- First-open loading UX follows overlay/toast contract exactly.
|
|
||||||
|
|
||||||
Reference in New Issue
Block a user