CLAUDE.md@src/grove/client git:20260803.49581e6A

Immutable. This exact content is served forever at /api/v1/blob/57193f6fff1a8317.

# grove.client — transport-agnostic attach (local PTY / SSH)

> ↑ [root](../../../CLAUDE.md)

Attach a user to a workspace's tmux session, and talk to the daemon, over either a local connection or SSH — the same client code regardless of transport.

## Attach

- **Two `AttachSession` impls share one Protocol so the terminal bridge is transport-agnostic.** `LocalAttach` uses stdlib `pty.fork` + `asyncio.add_reader` for local backends: one PTY pair per attach, lifecycle on close is SIGHUP → waitpid → close. `SshAttach` uses `asyncssh.create_process` over the existing `SSHClientConnection` for remote backends, with binary I/O (`encoding=None`). A caller picks the impl by backend and never branches on transport again.
- **Once attached, tmux owns the UI — never reimplement a multiplexer.** Ctrl-b w / Ctrl-b s / copy mode / the status line are tmux's. The client's job ends at wiring stdin/stdout to the session.

## Transport

- **Three transports, one Protocol.** `LocalTransport` spawns a child daemon; `SshTransport` tunnels to a remote one; `UrlTransport` (`BackendConfig.daemon_url`) points at an already-running daemon — start/close are no-ops (its lifecycle is not ours) and interactive attach is refused (no PTY/SSH channel). `UrlTransport` is [grove.mcp](../mcp/CLAUDE.md)'s path. `_resolve_token` order: explicit `daemon_token` wins on every backend (that's what lets a URL backend reach another machine), then the same-host local mint, then `NeedsPairingError`.
- **`SshTransport` reuses ONE `asyncssh.SSHClientConnection` for both jobs.** The HTTP forward to the daemon (`forward_local_port`) AND the interactive attach (`SSHClientProcess`) ride the same TCP session — daemon RPC and terminal attach never open a second SSH session.
- The daemon binds loopback only; remote access is this SSH port-forward. The bind/auth-deferral rationale lives in [daemon](../daemon/CLAUDE.md).
- **No raw `httpx` exception may escape `GroveClient`; `_request` is the single seam that guarantees it.** This module's contract says "transport failures raise `TransportError`", and **httpx's timeout exceptions stringify to the EMPTY STRING** (httpcore maps a bare `TimeoutError()` through, whose message is `""`). Every layer above renders `str(exc)`, so an unguarded call turns a dropped steering message into `Error executing tool grove_send_workspace_message:` and *nothing else*, leaving an orchestrator no strategy but blind retry. The generalizable trap: **an exception's type being informative does not make its string informative**, and the string is what every consumer shows. Guard where the foreign exception is born, not per call site — `send_message` was the one verb reaching for the httpx client directly (it tolerates both 200 and 204), and that bypass is exactly where the bug surfaced, so new verbs go through `_request` even when their success shape is unusual.
- **The same contract has a SUCCESS-path hole, and it is the one a guard on failures cannot see.** `_request` translates every httpx exception, but an unguarded `resp.json()` on the 200 branch lets a proxy error page or a truncated body escape as `json.JSONDecodeError`, which is neither `ProtocolError` (the capability-degrade branches key on one and cannot see it) nor `TransportError` (so the module docstring is simply false for it). It renders as "Expecting value: line 1 column 1", which a caller reads as a Grove bug rather than a broken hop. **A "no foreign exception escapes" rule has to cover parsing the reply, not only making the call** — the transport failed either way; only the moment differs. `_unwrap` is where that guard belongs.
- **A timeout is not a statement about the outcome and must never be reported as one.** The daemon may be mid-operation and still commit it, so the message says "check state before retrying" rather than "failed". Observed, not theorized: a create that "failed" this way had already provisioned a full workspace, and the operator's retry produced a duplicate nobody knew about.
- **A client deadline shorter than the server's own bound on a single step turns a healthy slow operation into a failure.** `_DEFAULT_TIMEOUT_S` (30 s) is right for reads and steer calls — a daemon that has not answered a listing or a keystroke injection in 30 s is wedged, not busy — but create/resume/respawn take `_LIFECYCLE_TIMEOUT_S`, which is the SUM of the engine's own per-step bounds plus headroom: `InitScriptConfig.timeout_seconds` (300 s) + `container.up_timeout_seconds` (900 s, a cold devcontainer build pulls a base image and installs Features) + 60 s for `git worktree add` and the agent launch. Adding a client method: ask what the *engine's* bound on that operation is before accepting the default.
- **"The CLI works but MCP doesn't" is evidence about the transport, not about the payload.** `grove message` / `grove create` are in-process (`build()` → `WorkspaceManager`: no HTTP, no deadline) while MCP goes client → daemon, so the two paths share almost no failure surface. Reasoning about message *content* will not find a bug that lives in the deadline.

## Cross-platform

- **POSIX-only stdlib (`pty` / `fcntl` / `termios`) MUST be guarded with `if sys.platform != "win32":`.** A bare top-level import raises `ModuleNotFoundError` on Windows during *test collection*, failing every test that transitively imports the module — green on Linux/macOS, red across the board on Windows with no useful context. Guard the import, then raise `NotImplementedError("requires POSIX (use WSL on Windows)")` at the first runtime entry point. The module loads cleanly everywhere; the unsupported path fails loudly only when exercised. Degrade to a typed error, never an import failure (same rule as tmux-needs-WSL2). `tests/client/test_local_attach.py` carries `pytestmark = skipif(win32)` so the suite stays runnable on Windows.

## Session lessons

Add client-specific transport/attach lessons here. Cross-cutting wire-contract facts belong in [root](../../../CLAUDE.md); daemon HTTP/auth facts in [daemon](../daemon/CLAUDE.md).

- **`open_attach` takes an ARGV, not a session name.** A containerized workspace is entered by exec'ing into its container, not by naming a session on the daemon's host, so a bridge that composes `tmux attach -t <name>` itself holds a policy only the engine can answer. It runs what `AttachInstructionView.attach_argv()` returns, and neither `LocalAttach` nor `SshAttach` knows what a runtime is — which is also why neither class carries a test-only alternate constructor: "run this argv" was always what the tests wanted and is now the real signature. `SshAttach` still takes a joined STRING because ssh runs a remote shell rather than an argv; the transport does the `shlex.join`, at the one boundary where that is true.
- **A terminal HANDOFF is not a transport target at all — it belongs in the CLI.** `grove shell` enters a workspace's container with `os.execvp`: nothing is bridged, because the CLI process *becomes* the exec, exactly as `grove attach` becomes its attach argv — and `grove attach`, the oldest verb of this shape, has no counterpart here either. The discriminator for this package is whether something is bridged or owned: `AttachSession` bridges a PTY to xterm.js, `vscode.py` launches an external process that a remote surface could also trigger. A handoff does neither, and a class here for it would be an abstraction with one implementation and no stream to share.
- **A launch-and-forget editor attach (VS Code) is a transport target but NOT an `AttachSession`.** Don't force-fit a PTY-streaming Protocol onto a target with nothing bidirectional — the two `AttachSession` impls share their Protocol because both stream; an editor launch has no stream to share, so it stays outside that Protocol entirely. `client/` reads container facts through the narrowest duck type available (two attribute names), never engine internals — the same "patch the boundary, don't import the implementation" discipline the rest of this module follows.
- **A spawned child's lifetime is bound to its parent at SPAWN time, not at cleanup — cleanup is exactly what does not run on an abnormal exit.** `LocalTransport.start()` spawns the daemon with plain `Popen`; `close()` does the right thing, but a SIGKILLed/OOM-killed/terminal-closed parent skips `close()` entirely, leaving daemons orphaned at PPID 1 burning ~1.4% of a core each indefinitely. The fix is `PR_SET_PDEATHSIG`, and it does NOT live here — it is armed in the spawned process's OWN startup (`grove.daemon.cli._arm_parent_death_signal`, gated on `--print-port`, `LocalTransport`'s exact argv signature), not via `Popen(preexec_fn=...)` in this launcher. Reason: `preexec_fn` runs Python/library code between fork() and exec() **in the launcher's process**, and fork() only clones the calling thread — any lock another live thread holds at that instant (Textual's TUI driver thread genuinely runs alongside this codepath, since the TUI is one of `LocalTransport`'s callers) can wedge the child forever. fork()+exec() itself carries no such hazard regardless of thread count, so arming post-exec, in the child's own code, sidesteps it entirely — at the cost of a longer arm-window, closed by a `getppid()` re-check right after the `prctl` call. Linux-only; degrades to a no-op on macOS/Windows, same shape as `paths.exclusive_lock`'s `fcntl` guard. **`LocalAttach` (`pty.fork` + `os.execvp`) needs no equivalent** — a pty's master-fd close (guaranteed on ANY process exit, kernel-level, abnormal or not) already delivers SIGHUP to the session-leader child, so the linkage is inherent to the mechanism, unlike a plain `Popen` over pipes which has none. `vscode.py`'s `start_new_session=True` and `core/process.py`'s `spawn_detached` are the opposite case — deliberately detached launch-and-forget, not a leak.