# TECH.md — Remote Server: Error Handling & Abort

Linear: [APP-3987](https://linear.app/warpdotdev/issue/APP-3987)

## 1. Problem

The remote server client/server protocol needs robust error handling for malformed messages and a mechanism for the client to cancel in-progress server requests. Specifically:

1. **Malformed messages** — if the request ID is parseable, return an error for that request; otherwise drop silently
2. **Client-side timeout** — abort requests that don't receive a response within 2 minutes
3. **Abort notification** — a new message type allowing the client to cancel in-progress server requests
4. **Message too big** — already enforced at the sending layer by `write_message`
5. **Unexpected responses** — already handled (drop + log warning)
6. **Stream errors** — no special handling needed

## 2. Relevant Code

- `crates/remote_server/proto/remote_server.proto` — `ClientMessage`/`ServerMessage` envelopes, `ErrorCode` enum, `ErrorResponse`
- `crates/remote_server/src/protocol.rs` — `ProtocolError` enum, `read_message`/`write_message` with length-delimited framing, `MAX_MESSAGE_SIZE` (64 MB), `RequestId` newtype
- `crates/remote_server/src/server_model.rs` — `ServerModel` singleton with stdin reader loop, stdout writer task, `handle_message` dispatch
- `crates/remote_server/src/client.rs` — `RemoteServerClient` with `pending_requests: DashMap<RequestId, oneshot::Sender<ServerMessage>>`, background reader/writer tasks, `ClientError` enum
- `crates/remote_server/src/client_tests.rs` — existing tests using `tokio::io::duplex` for in-memory streams
- `crates/remote_server/src/protocol_tests.rs` — round-trip and edge-case protocol tests

## 3. Current State

The protocol layer handles basic I/O errors with a recoverable/fatal classification:
- `ProtocolError::Decode` is read-recoverable (payload bytes consumed, stream aligned), but the request ID is lost — no error response can be sent back.
- `ProtocolError::MessageTooLarge` is enforced on write via `write_message`. On read, it's fatal because the oversized payload isn't consumed.
- The server's stdin reader loop skips recoverable errors and breaks on fatal ones.

The client's `pending_requests` map uses `oneshot::Sender<ServerMessage>`. There's no timeout — requests wait indefinitely. There's no way for the client to cancel server-side work.

The server has no concept of in-progress request tracking — `handle_message` is synchronous today (only `Initialize` exists). No abort mechanism exists.

## 4. Proposed Changes

### 4.1. Proto: Add `Abort` notification

Add to `remote_server.proto`:

```protobuf
message Abort {
  string request_id_to_abort = 1;
}
```

Add `Abort abort = 3` to `ClientMessage.oneof message`.

Abort is a notification (fire-and-forget) — no server response expected. The `request_id` on the `ClientMessage` envelope is still set (for logging/tracing) but the client does not register a pending request for it.

### 4.2. Protocol layer: Extract request ID from raw bytes on decode failure

Update `protocol.rs`:

- Change `ProtocolError::Decode` to carry the extracted request ID: `Decode(prost::DecodeError, Option<RequestId>)`. The extraction is done inside `read_message` itself using a private `try_extract_request_id` helper, so callers get the `Option<RequestId>` directly without needing to handle raw bytes.
- The private `try_extract_request_id(buf: &[u8]) -> Option<String>` parses only protobuf field 1 (string) using manual wire-format parsing: checks for tag byte `0x0a` (field_number=1, wire_type=2), decodes the varint length, and extracts the UTF-8 string. Stops immediately after field 1, so corruption in later bytes does not affect extraction.

### 4.3. Server: Error response for malformed messages with parseable request ID

Update the stdin reader loop in `ServerModel::new`:

- On `ProtocolError::Decode` with raw buffer, call `try_extract_request_id`. If a request ID is found, send an `ErrorResponse { code: INVALID_REQUEST }` with that request ID through `response_tx`. If no request ID can be extracted, drop the message (log warning, as already done).

### 4.4. Client: Change oneshot type to `Result<ServerMessage, ClientError>`

Change `pending_requests` from `DashMap<RequestId, oneshot::Sender<ServerMessage>>` to `DashMap<RequestId, oneshot::Sender<Result<ServerMessage, ClientError>>>`.

- Reader task sends `Ok(msg)` for normal responses.
- On `ProtocolError::Decode` with raw buffer: try `try_extract_request_id`. If found and a pending request exists, send `Err(ClientError::Protocol(...))` through the oneshot. If not found, drop (already logged).
- `send_request` unwraps the double-Result: `rx.await.map_err(|_| ClientError::ResponseChannelClosed)??`.
- Move `ErrorResponse` handling into `send_request`: after receiving an `Ok(msg)`, check if the message is an `ErrorResponse` and convert to `Err(ClientError::ServerError)`. Callers like `initialize()` then only match on success variants + `_ => UnexpectedResponse`.

### 4.5. Client: 2-minute request timeout

Update `send_request` to wrap the oneshot `rx.await` with `tokio::time::timeout(Duration::from_secs(120), rx)`. On timeout:

- Remove the request from `pending_requests`
- Send an `Abort` message for the timed-out request ID (via `send_notification`)
- Return `ClientError::Timeout`

Add `ClientError::Timeout` to the `ClientError` enum.

### 4.6. Client: `send_notification` helper

Add a private `fn send_notification(&self, msg: ClientMessage)` that sends through `outbound_tx` without registering a pending request. Used by timeout-triggered abort.

### 4.7. Server: In-progress request tracking & abort handling

Add `in_progress: HashMap<RequestId, tokio::sync::oneshot::Sender<()>>` to `ServerModel`. Each entry holds a cancellation signal sender.

- When `handle_message` starts processing a request that may be long-running, it inserts a cancellation receiver. The background work checks the receiver for cancellation.
- For synchronous/fast requests like `Initialize`, no tracking needed.
- On receiving `Abort { request_id_to_abort }`: look up the request ID in `in_progress`, if found send the cancel signal and remove the entry. If not found, no-op.
- When a request completes (response sent), remove it from `in_progress`.

Abort has pure notification semantics — the server does not send any response for abort messages.

Note: today all handlers are synchronous, so abort is mostly a no-op. This is future-proofing for long-running requests (file tree, code review, etc.). The infrastructure should be wired up now.

## 5. End-to-End Flow

### Malformed message with parseable request ID (server-side)

1. Client sends a corrupted protobuf where field 1 (request_id) is intact but other fields are malformed
2. Server stdin reader calls `read_client_message` → `ProtocolError::Decode(err, raw_buf)`
3. Reader calls `try_extract_request_id(&raw_buf)` → `Some("abc-123")`
4. Reader sends `ErrorResponse { code: INVALID_REQUEST, request_id: "abc-123" }` through `response_tx`
5. Server stdout writer sends the error response to the client
6. Client reader task resolves the pending request for "abc-123" with the error

### Client request timeout → abort

1. Client calls `send_request` which registers a oneshot in `pending_requests` and sends the `ClientMessage`
2. Server receives the request and begins (potentially long-running) work
3. After 2 minutes with no response, `tokio::time::timeout` fires in `send_request`
4. Client removes the request from `pending_requests`
5. Client sends `ClientMessage { request_id: new_uuid, message: Abort { request_id_to_abort: original_id } }` via `send_notification`
6. Client returns `ClientError::Timeout` to the caller
7. Server receives the `Abort`, looks up `original_id` in `in_progress`, sends the cancel signal if found

## 6. Risks and Mitigations

- **`ProtocolError::Decode` variant change is a breaking API change** — all existing match arms on `ProtocolError::Decode(_)` must be updated to `ProtocolError::Decode(_, Some(...))` / `ProtocolError::Decode(_, None)`. This is internal-only and caught at compile time.
- **`try_extract_request_id` false negatives** — if corruption hits the request_id field bytes, extraction fails and we fall back to dropping the message. This is the correct behavior since we genuinely can't correlate the error.
- **Oneshot type change ripples** — changing from `oneshot::Sender<ServerMessage>` to `oneshot::Sender<Result<ServerMessage, ClientError>>` affects the reader task and `send_request`. The existing test mock servers need updating since they construct `ServerMessage` directly.

## 7. Testing and Validation

- **Protocol test**: Verify `try_extract_request_id` extracts the ID from corrupted payloads where field 1 is intact, and returns `None` when field 1 is also corrupt.
- **Server malformed message test**: Send a corrupted protobuf with a valid request_id prefix → assert server responds with `ErrorResponse { code: INVALID_REQUEST }`.

## 8. Follow-ups

- Client timeout and abort tests (mock server that never responds, `tokio::time::pause()`)
- Abort cancellation tests (verify server cancels in-progress work)
- Domain-specific error codes (e.g. `FILE_NOT_FOUND`) as the protocol grows
- Structured log delivery over the protocol as an alternative to stderr streaming