Rust: `create_session` deadlocks forever when `ClientOptions::session_fs` is set — the session's request consumer starts after the `session.create` RPC
- Lingua principale
- Java
- Stelle
- 10.5k
- Fork
- 1.5k
- Merge medio
- 1g 11h
- PR unite (30g)
- 128
Descrizione
## Summary
In the Rust SDK, `Client::create_session` hangs **indefinitely** for any client configured with `ClientOptions::with_session_fs(..)`. The CLI issues a `sessionFs.readFile` while `session.create` is still in flight, and the SDK never answers it, so both sides wait forever.
The Rust SDK pre-registers the session on the router before the RPC (correctly), but the only consumer of that session's request channel — and the only place the `SessionFsProvider` is installed — is `spawn_event_loop`, which runs **after** the create RPC returns. Registration alone only *queues* the inbound request; nothing answers it.
The Go and Node.js SDKs do not have this bug: both start the session's request handling *eagerly, before* the RPC. So this is a Rust-specific divergence from the behaviour the other SDKs already document and implement.
Version: `github-copilot-sdk` 1.0.13 (latest published). CLI 1.0.83 (the release pinned in the crate's own `cli-version.txt`), run as an external server (`copilot --server --port N`). Reproduced on Linux x86_64 across 8+ fresh server instances. **Still present on `main` as of today.**
## Wire trace
Captured with a logging TCP proxy between the SDK and the CLI server:
```
[7.271] C->S REQ id=3 session.create {… "sessionId": "2677a0ec-0f2f-4290-9a8c-b80b903470a3" …}
[7.691] S->C REQ id=1 sessionFs.readFile {"path": "…/.session-state/workspace.yaml",
"sessionId": "2677a0ec-0f2f-4290-9a8c-b80b903470a3"}
[67.27] -- connection closed (client-side 60s timeout; the SDK never responded) --
```
The session IDs match, so the request *is* routed to a registered session — it is queued and never drained, not misrouted.
## Root cause (`rust/src/session.rs`, `start_prepared_create`)
1. `~1175` — the session is registered before the RPC, with a comment showing the window is known and intentional:
> For non-cloud sessions we generate the id client-side … so the session can be registered BEFORE the RPC — the CLI may issue session-scoped requests (e.g. `sessionFs.writeFile` for workspace metadata) during `session.create` processing, before it has sent the response.
2. `rust/src/router.rs` — `register()` creates an `mpsc::unbounded_channel()`; the routing task does `sender.send(request)` for a registered session. The receiver is handed out in `SessionRegistration::channels`.
3. `~1394` — `call_with_inline_callback("session.create", …).await?`
4. `~1419` — `spawn_event_loop(…, session_fs_provider, channels, …)` — **the first and only consumer of that receiver**, and the only place `session_fs_provider` is installed (`~2968`: `session_fs_dispatch::dispatch`).
Steps 3 and 4 are in the wrong order for the window step 1 deliberately creates.
`start_prepared_resume` has the same ordering (`~1636`), so `resume_session` should be affected identically.
## Why the other SDKs are unaffected
Both start the consumer before the RPC:
- **Go** (`go/client.go` ~1093): *"Pre-register non-cloud sessions BEFORE issuing the RPC so any session-scoped requests the CLI emits during session.create processing (e.g. sessionFs.writeFile for workspace metadata) **can be routed to the correct handlers**."* `initializeSession()` is called there, and `newSession` *"starts processEvents eagerly, before the RPC confirms"* (~1447). (#2320 — the eagerly-started `processEvents` goroutine leaking on create failure — is further confirmation Go starts it early.)
- **Node.js** (`nodejs/src/client.ts` ~1627): same comment; `initializeSession(localSessionId)` → `setupSessionFs(s, config)` runs before `sendRequest("session.create", …)`.
Rust is the outlier: it performs the *registration* half of that pattern but not the *handler-installation* half.
## Things that are not workarounds
- `prepare_session(cfg)?.start()` — same code path (`start_prepared_create`).
- Draining the queue from application code — `Client::register_session` and `SessionChannels` are `pub(crate)`.
- Serving the callback from a second `Client` — `sessionFs.setProvider` is connection-level and exclusive (*"Another client is already the session filesystem provider"*).
- Upgrading — 1.0.13 is the latest published version.
- `with_base_directory` instead of `session_fs` does avoid the hang (no `sessionFs` RPC is ever issued), but that defeats the purpose for anyone using `session_fs` as their isolation boundary.
## Suggested fix (verified)
Start the event loop before the create RPC on the non-cloud path, matching Go/Node. The loop is purely reactive — no startup RPC, no dependency on the create response — and `capabilities` is already a shared `Arc>` written once the response arrives. On an early return the armed `PendingSessionRegistration` cancels `shutdown`, which is the loop's own exit condition, so failure cleanup is unchanged.
Concretely: hoist the `spawn_event_loop(..)` call into a one-shot closure and invoke it immediately after `PendingSessionRegistration` is armed when `local_session_id.is_some()`, taking the channels from the stash there; leave the cloud path (server-assigned ID) spawning after the response as it does today.
I applied exactly this to a local 1.0.13 checkout and re-ran the same scenario. `session.create` now completes in ~460 ms, having answered 12 provider calls mid-flight:
```
[4.375] C->S REQ id=3 session.create
[4.824] S->C REQ id=1 sessionFs.readFile -> [4.824] C->S RESP id=1 ok
[4.825] S->C REQ id=2 sessionFs.mkdir -> [4.825] C->S RESP id=2 ok
… mkdir ×3, writeFile ×2, rename, readFile ×2, readdirWithTypes …
[4.834] S->C RESP id=3 session.create (returns)
```
(The create then returns a normal application-level error about a custom agent missing from my own plugin directory — an unrelated problem on my side, and exactly what the `base_directory` variant reports too.)
Happy to open a PR with this change plus a regression test if that's useful.
## Reproduction
Any Rust client with `ClientMode::Empty` + `with_session_fs(..)` + a `SessionFsProvider`, against an external `copilot --server`. It does not require a real Copilot entitlement to observe: a mock JSON-RPC server that answers `connect` and `sessionFs.setProvider`, then issues a `sessionFs.readFile` on receiving `session.create` and waits, reproduces the hang deterministically — the SDK never sends a response.
Guida per i contributori
Apri la guida per i contributori
Direzione di ricerca
Start in rust/src/session.rs at start_prepared_create and compare its ordering with start_prepared_resume; read rust/src/router.rs to understand how registered session requests are queued and consumed. Move event-loop startup before the non-cloud create or resume RPC, then add a regression test using the described mock JSON-RPC server and verify sessionFs requests receive responses during the RPC.
Scritto dal modello di indicizzazione a partire dal testo della issue.
Valutazione
- Stack tecnologico
- rust
- Ambito
- api, backend
- Tipo di issue
- Bug
- Difficoltà
- 3/5
- Tempo stimato
- 1-2 giorni
- Stato di attività
- Attiva
- Chiarezza
- Specificata chiaramente
- Idoneità per principianti
- 76/100