github / github/copilot-sdk

Code-generate inbound (server→client) RPC dispatch for the Rust SDK

Aperta
#1,764 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
enhancement
Lingua principale
Java
Stelle
10.5k
Fork
1.5k
Merge medio
1g 11h
PR unite (30g)
127

Descrizione

## Background

The SDK protocol has two directions of RPC:

- **Outbound** (client→server): the SDK calls into the runtime.
- **Inbound** (server→client): the runtime calls *back* into the SDK — e.g. `clientSession.*` (`canvas`, `sessionFs`) and the new `clientGlobal.*` (`llmInference`) callbacks added in #1689.

For **inbound** dispatch, the four SDKs split into two camps:

- **C#, Go, Python, TypeScript** code-generate the inbound handler scaffolding (handler interfaces + the dispatch/registration glue) from `api.schema.json`. Adding the `clientGlobal` bucket in #1689 required teaching all four `scripts/codegen/*.ts` generators about it.
- **Rust** generates only the inbound **wire types** (`generated/api_types.rs`) and **method-name constants** (`rpc_methods::*` in `generated/rpc.rs`). The per-group **dispatch + handler trait** is hand-written, consistently, across all three inbound groups:
- `canvas.*` → `rust/src/canvas_dispatch.rs`
- `sessionFs.*` → `rust/src/session_fs_dispatch.rs`
- `llmInference.*` → `rust/src/copilot_request_handler.rs` (`dispatch`)

Each follows the identical shape: prefix-check → match `rpc_methods::*` constant → deserialize generated params → call the hand-written handler trait → serialize the result.
- **Java** has no codegen pipeline at all (entire RPC surface hand-authored), so it's out of scope here.

## Proposal

Bring Rust to parity with the other generated SDKs by code-generating the inbound **handler trait + dispatch helper** per group into `rust/src/generated/`, so future inbound callbacks (new `clientSession` / `clientGlobal` groups) come for free, as they already do in C#/Go/Python/TS.

## Why this is its own PR, not part of #1689

This was investigated during #1689 and deliberately deferred:

1. **Do all three groups, not a one-off.** Rust's hand-written-dispatch convention is currently uniform across `canvas`, `sessionFs`, and `llmInference`. Generating only `llmInference` (the group #1689 touches) would trade Rust's *internal* consistency for cross-SDK symmetry and leave a confusing one-off. The clean change converts all three groups together — a cross-cutting refactor of shipped, well-tested dispatchers and their **public** trait surfaces (`CanvasHandler`, `SessionFsProvider`, `CopilotRequestHandler`), out of scope for a callbacks PR.

2. **Generate only the low-level wire trait; keep ergonomic traits + streaming bridges hand-written.** The public `CopilotRequestHandler` (`send_request` + `open_websocket`) does **not** mirror the wire methods (`httpRequestStart` + `httpRequestChunk`); ~1200 lines bridge frame-oriented streaming ↔ that 2-method seam with stateful exchange correlation. Codegen should emit only the thin wire-level trait + dispatch (mirroring what TS generates), with the ergonomic traits and the llmInference bridge staying hand-written. Net hand-written savings are small (≈ the dispatch `match` + method constants); the value is future-proofing and symmetry, not LOC.

3. **Validation.** The streaming handler is heavily covered by Rust e2e (cancel / error / session-id) record-replay tests that should gate the refactor in CI.

## Suggested scope of work

- [ ] Add inbound handler-trait + dispatch emission to `scripts/codegen/rust.ts` (read `clientSession` + `clientGlobal`), emitting into `rust/src/generated/`.
- [ ] Migrate `canvas_dispatch.rs`, `session_fs_dispatch.rs`, and `copilot_request_handler.rs` to implement/use the generated trait + dispatch instead of their hand-rolled `match` + local method-name constants.
- [ ] Keep ergonomic public traits (`CanvasHandler`, `SessionFsProvider`, `CopilotRequestHandler`) and the llmInference streaming bridge hand-written.
- [ ] Verify `cargo build` + clippy + the full Rust e2e suite stay green.

## References

- PR #1689 (LLM inference callbacks) — where the `clientGlobal` bucket and the C#/Go/Python/TS generator changes landed.
- Generators with inbound support: `scripts/codegen/{csharp,go,python,typescript}.ts` (`emitClientGlobalApiRegistration` / `emitClientSessionApiRegistration`).
- Rust generator (outbound only today): `scripts/codegen/rust.ts`.

Guida per i contributori

Apri la guida per i contributori

Direzione di ricerca

Start with scripts/codegen/rust.ts and compare inbound emission in scripts/codegen/{csharp,go,python,typescript}.ts. Then inspect rust/src/canvas_dispatch.rs, rust/src/session_fs_dispatch.rs, and rust/src/copilot_request_handler.rs, preserving the ergonomic traits and streaming bridge. Done means generated inbound traits and dispatch replace the hand-written matches while cargo build, clippy, and the full Rust e2e suite remain green.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Valutazione

Stack tecnologico
rust, typescript
Ambito
api, backend-api-design, tooling
Tipo di issue
Refactoring
Difficoltà
4/5
Tempo stimato
3-5 giorni
Stato di attività
Tranquilla
Chiarezza
Specificata chiaramente
Idoneità per principianti
55/100

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.