Skip to content

Commit 14faf4a

Browse files
tclemCopilot
andcommitted
Address PR #1164 review feedback
emitted from the loop correlate to a session in traces. Matches the pattern documented in the rust-coding-skill. - README.md / embeddedcli.rs: correct the embedded-CLI documentation to match what build.rs and embeddedcli.rs actually do — archives come from the github/copilot-cli GitHub Releases, integrity is SHA-256 against SHA256SUMS.txt, and the runtime cache path is ~/.cache/copilot-sdk-{version}/copilot. - test/scenarios/sessions/streaming/verify.sh: drop a duplicate '# Go: build' comment. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 8b60330 commit 14faf4a

4 files changed

Lines changed: 30 additions & 26 deletions

File tree

rust/README.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -499,9 +499,9 @@ COPILOT_CLI_VERSION=1.0.15 cargo build
499499

500500
### How it works
501501

502-
1. **Build time:** The SDK's `build.rs` detects `COPILOT_CLI_VERSION`, downloads the platform-appropriate binary from npm (`@github/copilot-{platform}`), verifies the tarball's SHA-512 integrity hash against npm's registry metadata, compresses with zstd, and embeds via `include_bytes!()`. No extra steps or tools needed — just the env var.
502+
1. **Build time:** The SDK's `build.rs` detects `COPILOT_CLI_VERSION`, downloads the platform-appropriate archive from the [`github/copilot-cli` GitHub Releases](https://github.com/github/copilot-cli/releases) (`copilot-{platform}.tar.gz` on macOS/Linux, `.zip` on Windows), verifies the archive's SHA-256 against the release's `SHA256SUMS.txt`, extracts the `copilot` binary, compresses it with zstd, and embeds via `include_bytes!()`. No extra steps or tools needed — just the env var.
503503

504-
2. **Runtime:** On the first call to `copilot::resolve::copilot_binary()`, the embedded binary is lazily extracted to `~/.cache/copilot-sdk/copilot_{version}`, SHA-256 verified, and cached. Subsequent calls return the cached path.
504+
2. **Runtime:** On the first call to `copilot::resolve::copilot_binary()`, the embedded binary is lazily extracted to `~/.cache/copilot-sdk-{version}/copilot` (or `copilot.exe` on Windows), SHA-256 verified, and cached. Subsequent calls return the cached path.
505505

506506
3. **Dev builds:** Without the env var, `build.rs` does nothing. The binary is resolved from PATH as usual — zero friction.
507507

rust/src/embeddedcli.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,8 +34,9 @@ pub fn bundled_version() -> Option<&'static str> {
3434
/// Returns the path to the installed CLI binary, lazily extracting on first call.
3535
///
3636
/// When the SDK was built with `COPILOT_CLI_VERSION` set, this extracts the
37-
/// embedded binary to `~/.cache/copilot-sdk/copilot_{version}`, verifies the
38-
/// SHA-256 hash, and returns the path. Subsequent calls return the cached result.
37+
/// embedded binary to `~/.cache/copilot-sdk-{version}/copilot` (or
38+
/// `copilot.exe` on Windows), verifies the SHA-256 hash, and returns the
39+
/// path. Subsequent calls return the cached result.
3940
///
4041
/// Returns `None` if no CLI was embedded at build time.
4142
pub fn path() -> Option<PathBuf> {

rust/src/session.rs

Lines changed: 25 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::time::Duration;
66
use serde_json::Value;
77
use tokio::sync::{Mutex, oneshot};
88
use tokio::task::JoinHandle;
9-
use tracing::warn;
9+
use tracing::{Instrument, warn};
1010

1111
use crate::generated::api_types::{
1212
PermissionDecision, PermissionDecisionApproveOnce, PermissionDecisionApproveOnceKind,
@@ -934,29 +934,33 @@ fn spawn_event_loop(
934934
mut requests,
935935
} = channels;
936936

937-
tokio::spawn(async move {
938-
loop {
939-
tokio::select! {
940-
Some(notification) = notifications.recv() => {
941-
handle_notification(
942-
&session_id, &client, &handler, notification, &idle_waiter, &capabilities, &event_tx,
943-
).await;
944-
}
945-
Some(request) = requests.recv() => {
946-
handle_request(
947-
&session_id, &client, &handler, hooks.as_deref(), transforms.as_deref(), request,
948-
).await;
937+
let span = tracing::error_span!("session_event_loop", session_id = %session_id);
938+
tokio::spawn(
939+
async move {
940+
loop {
941+
tokio::select! {
942+
Some(notification) = notifications.recv() => {
943+
handle_notification(
944+
&session_id, &client, &handler, notification, &idle_waiter, &capabilities, &event_tx,
945+
).await;
946+
}
947+
Some(request) = requests.recv() => {
948+
handle_request(
949+
&session_id, &client, &handler, hooks.as_deref(), transforms.as_deref(), request,
950+
).await;
951+
}
952+
else => break,
949953
}
950-
else => break,
954+
}
955+
// Channels closed — fail any pending send_and_wait.
956+
if let Some(waiter) = idle_waiter.lock().await.take() {
957+
let _ = waiter
958+
.tx
959+
.send(Err(Error::Session(SessionError::EventLoopClosed)));
951960
}
952961
}
953-
// Channels closed — fail any pending send_and_wait.
954-
if let Some(waiter) = idle_waiter.lock().await.take() {
955-
let _ = waiter
956-
.tx
957-
.send(Err(Error::Session(SessionError::EventLoopClosed)));
958-
}
959-
})
962+
.instrument(span),
963+
)
960964
}
961965

962966
fn extract_request_id(data: &Value) -> Option<RequestId> {

test/scenarios/sessions/streaming/verify.sh

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,6 @@ check "TypeScript (build)" bash -c "cd '$SCRIPT_DIR/typescript' && npm run bui
111111
check "Python (install)" bash -c "python3 -c 'import copilot' 2>/dev/null || (cd '$SCRIPT_DIR/python' && pip3 install -r requirements.txt --quiet 2>&1)"
112112
check "Python (syntax)" bash -c "python3 -c \"import ast; ast.parse(open('$SCRIPT_DIR/python/main.py').read()); print('Syntax OK')\""
113113

114-
# Go: build
115114
# Go: build
116115
check "Go (build)" bash -c "cd '$SCRIPT_DIR/go' && go build -o streaming-go . 2>&1"
117116

0 commit comments

Comments
 (0)