github / github/copilot-sdk

Rust: `create_session` deadlocks forever when `ClientOptions::session_fs` is set — the session's request consumer starts after the `session.create` RPC

Open
#2,624 0 comments 1 reaction 0 assignees View on GitHub
Dominant language
Java
Stars
10.5k
Forks
1.5k
Avg merge
1d 11h
Merged PRs (30d)
128

Description

## 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.

Contributor guide

Open the contributing guide

Research direction

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.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
api, backend
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.