github / github/copilot-sdk

Add is_alive liveness-probe API across SDKs (decouple liveness from typed ping deserialization)

Đang mở
#1,354 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
enhancement
Ngôn ngữ chính
Java
Star
10.5k
Fork
1.5k
Merge trung bình
1 ngày 11 giờ
Pull request đã merge (30 ngày)
128

Mô tả

## Summary

The Copilot CLI host (`github/github-app`) had to work around a deserialization failure in our SDK's typed `ping()` API on the session-resume / warm-CLI-pool / liveness-probe paths. See [github/github-app#5461](https://github.com/github/github-app/pull/5461) for the workaround.

Their fix introduces a `ping_cli_compat(&client)` helper that drops to the raw `client.call("ping", json!({}))` so the result body is never deserialized — they only care whether the JSON-RPC round trip succeeded.

We should give them (and everyone doing liveness checks) a first-class primitive instead of forcing them to bypass our typed API.

## Root cause

Across all five SDKs, `ping()` deserializes the response into a typed body:

- Rust: `Client::ping(&self) -> Result` ([`rust/src/lib.rs:1676`](https://github.com/github/copilot-sdk/blob/main/rust/src/lib.rs#L1676))
- Node: `client.ping()` returning `{message, timestamp, protocolVersion?}` ([`nodejs/src/client.ts:1032`](https://github.com/github/copilot-sdk/blob/main/nodejs/src/client.ts#L1032))
- Go: `client.Ping(ctx, msg) (*PingResponse, error)` ([`go/client.go:1317`](https://github.com/github/copilot-sdk/blob/main/go/client.go#L1317))
- .NET: `client.PingAsync(...) : Task` ([`dotnet/src/Client.cs:874`](https://github.com/github/copilot-sdk/blob/main/dotnet/src/Client.cs#L874))
- Python: equivalent typed return

The authoritative `PingResult` schema (`@github/copilot/schemas/api.schema.json`) requires `message`, `timestamp` (date-time string), and `protocolVersion` (integer > 0). The Rust hand-written `PingResponse` softens this with `#[serde(default)]` and `Option`, but that only helps for *missing* fields — wrong types (e.g. `null` timestamp, integer-shape drift) still fail. Same brittleness exists across the other SDKs.

`ping` is used by hosts as a liveness check (warm-pool reuse, resumed-session aliveness, retrier "is the cached client still alive" probes). These paths straddle CLI version boundaries — a resumed older CLI process can answer with a slightly different `ping` body shape, and the entire liveness check fails even though the RPC itself succeeded.

This is exactly the wrong failure mode for a health check: the consumer asked "is the CLI reachable?" and we answered "no" because of a body-shape mismatch.

## Proposed fix

Add a dedicated liveness-probe API to every SDK `Client`:

- **Sends** the `ping` JSON-RPC call.
- **Returns success based solely on JSON-RPC success** — never deserializes the result body.
- **Composable with caller-supplied timeouts** — no baked-in timeout, so each host (startup probe, warm-pool, resume probe, background keepalive) sets its own budget.
- **`ping()` and generated `rpc.ping()` stay strict and schema-faithful.** Callers who actually want the typed data keep getting it; schema drift continues to surface there as a real error.

Per-language names:

| SDK | API |
|--------|--------------------------------------------|
| Rust | `Client::is_alive(&self) -> bool` |
| Node | `client.isAlive(): Promise` |
| Python | `client.is_alive() -> bool` |
| Go | `client.IsAlive(ctx context.Context) bool` |
| .NET | `client.IsAliveAsync(ct) : Task` |

After this lands, the `github/github-app` workaround helper goes away and call sites become e.g. `existing.is_alive().await`.

## Rejected alternatives

- **Loosen `ping()` itself.** Considered and rejected. `ping()` is a typed schema-backed API; if it silently swallows malformed bodies, the contract becomes ambiguous (did the caller want liveness, or the ping data?). It also masks real CLI/schema drift in the API most likely to catch it.
- **Loosen the generated `rpc.ping(PingRequest)`.** Same reasoning, more so — generated APIs must remain schema-faithful. `is_alive` is the explicit escape hatch.
- **"Fix it only in the CLI."** The CLI should still honor the schema, and we should investigate any drift. But liveness checks fundamentally straddle version boundaries, so the SDK needs a body-agnostic primitive regardless.

## Acceptance criteria

- [ ] New `is_alive`/`IsAlive`/`isAlive` method on the Rust, Node, Python, Go, and .NET `Client` types.
- [ ] Method calls the `ping` JSON-RPC method and returns success purely on RPC success, ignoring the response body.
- [ ] Existing `ping()` / `rpc.ping()` typed APIs unchanged.
- [ ] Docs in each SDK clearly distinguish "use `is_alive` for liveness/warm-pool/resume checks" from "use `ping()` when you want the typed response."
- [ ] Tests cover the case where the CLI returns a `ping` body that fails strict deserialization but is otherwise a valid JSON-RPC success — `is_alive` returns `true`, `ping()` returns an error.
- [ ] Once shipped, the workaround in [github/github-app#5461](https://github.com/github/github-app/pull/5461) is removed.

## Design review

Reviewed and agreed with GPT-5.5; consensus on:
- Separate `is_alive` API (not loosening `ping`).
- Keep generated `rpc.ping` strict.
- No baked-in timeout; caller composes timeouts.
- Name `is_alive` clearly communicates intent ("RPC round trip succeeded") and beats `ping_raw` / `ping_check`.

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Hướng nghiên cứu

Bắt đầu với các điểm vào ping của Client được nêu trong rust/src/lib.rs:1676, nodejs/src/client.ts:1032, go/client.go:1317, dotnet/src/Client.cs:874 và client Python tương ứng, sau đó kiểm tra @github/copilot/schemas/api.schema.json. Chạy các bài kiểm thử SDK và bổ sung coverage cho một phản hồi JSON-RPC hợp lệ nhưng body của ping không thể được deserialization nghiêm ngặt. Được xem là hoàn tất khi mỗi Client có một phương thức liveness phù hợp với ngôn ngữ, các API ping đã typed hiện có không thay đổi và mỗi SDK đều tài liệu hóa sự phân biệt này.

Do mô hình lập chỉ mục viết ra từ nội dung của issue.

Đánh giá

Công nghệ
go, node.js, python, rust
Lĩnh vực
api, backend-api-design, documentation
Loại issue
Tính năng
Độ khó
5/5
Thời gian dự kiến
Hơn một tuần
Mức độ hoạt động
Ít trao đổi
Độ rõ ràng
Đặc tả rõ ràng
Mức phù hợp với người mới
48/100

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.