Skip to content

Commit afecf46

Browse files
Rust: full in-process (FFI) parity with .NET/TS
Bring the Rust in-process transport in line with the .NET and TypeScript SDKs. Product: - Transport::InProcess passes no per-client env to the FFI worker; the worker inherits this host process's ambient environment. Per-client options that lower to env vars (env/env_remove, telemetry, github_token, base_directory) are not honored in-process, matching .NET/TS; documented as experimental, pointing at #1934. Removes the now-unused build_ffi_environment. E2E harness: - InProcessEnvGuard mirrors each test's environment onto the real process environment (which the in-process worker inherits) and restores it on drop, plus disables ambient HMAC so host-side auth picks the token the snapshots expect. Forces serial execution in-process (concurrency 1) so the process-wide env mutation is coherent; stdio/tcp are unchanged. - skip_inprocess guard skips tests for features the in-process transport can't support; applied to the telemetry file-export test (telemetry lowers to env vars) and the unknown-checkpoint test (readCheckpoint decodes the id as u32 natively in-process and rejects the i64::MAX sentinel). CI: - The in-process job now runs the whole E2E suite over the in-process transport (serially) instead of only the smoke test, mirroring the .NET/TS inprocess cell. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 6796098 commit afecf46

5 files changed

Lines changed: 123 additions & 67 deletions

File tree

.github/workflows/rust-sdk-tests.yml

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,8 +133,10 @@ jobs:
133133
# analogue of the .NET `RuntimeConnection.ForInProcess()`), mirroring the
134134
# `inprocess` transport cell in dotnet-sdk-tests.yml. Sets
135135
# COPILOT_SDK_DEFAULT_CONNECTION=inprocess so the client hosts the runtime
136-
# cdylib in-process instead of spawning a stdio child, then runs the
137-
# dedicated in-process E2E test.
136+
# cdylib in-process instead of spawning a stdio child, then runs the whole
137+
# E2E suite over the in-process transport. The suite runs serially in-process
138+
# (the harness forces concurrency to 1) because it mirrors each test's
139+
# environment onto the shared process environment the in-process worker inherits.
138140
test-inprocess:
139141
name: "Rust SDK Tests (in-process transport)"
140142
if: github.event.repository.fork == false
@@ -184,14 +186,15 @@ jobs:
184186
- name: Select in-process transport
185187
run: echo "COPILOT_SDK_DEFAULT_CONNECTION=inprocess" >> "$GITHUB_ENV"
186188

187-
- name: cargo test (in-process transport)
189+
- name: cargo test (in-process transport, full E2E suite)
188190
timeout-minutes: 60
189191
env:
190-
RUST_E2E_CONCURRENCY: 4
191192
COPILOT_HMAC_KEY: ${{ secrets.COPILOT_DEVELOPER_CLI_INTEGRATION_HMAC_KEY }}
192193
COPILOT_CLI_PATH: ${{ steps.setup-copilot.outputs.cli-path }}
193194
BUNDLED_CLI_CACHE_DIR: ${{ github.workspace }}/rust/.bundled-cli-cache
194-
run: cargo test --no-default-features --features test-support --test e2e inprocess -- --nocapture
195+
# The harness forces serial execution in-process, so RUST_E2E_CONCURRENCY is
196+
# intentionally not set here.
197+
run: cargo test --no-default-features --features test-support --test e2e -- --nocapture
195198

196199
# Validates the bundled-CLI build path on all three supported
197200
# platforms. While the regular `cargo test` job above also exercises

rust/src/lib.rs

Lines changed: 17 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,15 @@ pub enum Transport {
120120
/// entrypoint and speaks JSON-RPC over its C ABI. The runtime spawns its
121121
/// own worker; the SDK never launches a CLI child process. This is the
122122
/// Rust analogue of the .NET `RuntimeConnection.ForInProcess()`.
123+
///
124+
/// **Experimental.** Per-client options that lower to environment variables
125+
/// — [`ClientOptions::env`]/[`ClientOptions::env_remove`],
126+
/// [`ClientOptions::telemetry`], [`ClientOptions::github_token`], and
127+
/// [`ClientOptions::base_directory`] — are **not** honored with this
128+
/// transport: the runtime loads into the shared host process and its worker
129+
/// inherits that process's ambient environment. Configure the runtime via
130+
/// the host process environment instead. See
131+
/// <https://github.com/github/copilot-sdk/issues/1934>.
123132
InProcess,
124133
/// Spawn the CLI with `--port` and connect via TCP.
125134
Tcp {
@@ -1148,9 +1157,15 @@ impl Client {
11481157
)?
11491158
}
11501159
Transport::InProcess => {
1151-
let environment = Self::build_ffi_environment(&options);
1160+
// Per-client options that lower to environment variables (env,
1161+
// telemetry, github_token, base_directory) are not honored over the
1162+
// in-process transport: the runtime loads into this shared host
1163+
// process and its worker inherits this process's ambient environment,
1164+
// which a single env block cannot vary per client. Pass no per-client
1165+
// env; configure the runtime via the host process environment instead.
1166+
// See https://github.com/github/copilot-sdk/issues/1934.
11521167
info!(entrypoint = %program.display(), "hosting copilot runtime in-process (FFI)");
1153-
let host = crate::ffi::FfiHost::create(&program, environment)?;
1168+
let host = crate::ffi::FfiHost::create(&program, Vec::new())?;
11541169
let (reader, writer, shared) = host.start().await?;
11551170
let client = Self::from_transport(
11561171
reader,
@@ -1436,66 +1451,6 @@ impl Client {
14361451
});
14371452
}
14381453

1439-
fn build_ffi_environment(options: &ClientOptions) -> Vec<(String, String)> {
1440-
let mut env: std::collections::BTreeMap<String, String> = std::env::vars_os()
1441-
.map(|(k, v)| {
1442-
(
1443-
k.to_string_lossy().into_owned(),
1444-
v.to_string_lossy().into_owned(),
1445-
)
1446-
})
1447-
.collect();
1448-
let mut set = |key: &str, value: String| {
1449-
env.insert(key.to_string(), value);
1450-
};
1451-
if let Some(token) = &options.github_token {
1452-
set("COPILOT_SDK_AUTH_TOKEN", token.clone());
1453-
}
1454-
if let Some(telemetry) = &options.telemetry {
1455-
set("COPILOT_OTEL_ENABLED", "true".to_string());
1456-
if let Some(endpoint) = &telemetry.otlp_endpoint {
1457-
set("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.clone());
1458-
}
1459-
if let Some(protocol) = telemetry.otlp_protocol {
1460-
set("OTEL_EXPORTER_OTLP_PROTOCOL", protocol.as_str().to_string());
1461-
}
1462-
if let Some(path) = &telemetry.file_path {
1463-
set(
1464-
"COPILOT_OTEL_FILE_EXPORTER_PATH",
1465-
path.to_string_lossy().into_owned(),
1466-
);
1467-
}
1468-
if let Some(exporter) = telemetry.exporter_type {
1469-
set("COPILOT_OTEL_EXPORTER_TYPE", exporter.as_str().to_string());
1470-
}
1471-
if let Some(source) = &telemetry.source_name {
1472-
set("COPILOT_OTEL_SOURCE_NAME", source.clone());
1473-
}
1474-
if let Some(capture) = telemetry.capture_content {
1475-
set(
1476-
"OTEL_INSTRUMENTATION_GENAI_CAPTURE_MESSAGE_CONTENT",
1477-
if capture { "true" } else { "false" }.to_string(),
1478-
);
1479-
}
1480-
}
1481-
if let Some(dir) = &options.base_directory {
1482-
set("COPILOT_HOME", dir.to_string_lossy().into_owned());
1483-
}
1484-
if options.mode == ClientMode::Empty {
1485-
set("COPILOT_DISABLE_KEYTAR", "1".to_string());
1486-
}
1487-
for (key, value) in &options.env {
1488-
env.insert(
1489-
key.to_string_lossy().into_owned(),
1490-
value.to_string_lossy().into_owned(),
1491-
);
1492-
}
1493-
for key in &options.env_remove {
1494-
env.remove(&*key.to_string_lossy());
1495-
}
1496-
env.into_iter().collect()
1497-
}
1498-
14991454
fn build_command(program: &Path, options: &ClientOptions) -> Command {
15001455
let mut command = Command::new(program);
15011456
for arg in &options.prefix_args {

rust/tests/e2e/rpc_workspace_checkpoints.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@ async fn should_list_no_checkpoints_for_fresh_session() {
4040

4141
#[tokio::test]
4242
async fn should_return_null_or_empty_content_for_unknown_checkpoint() {
43+
// In-process, session.workspaces.readCheckpoint is answered by the native runtime,
44+
// which decodes the checkpoint number as a u32 and rejects the i64::MAX sentinel this
45+
// test uses. Covered by the default (stdio) transport. See issue #1934.
46+
if super::support::skip_inprocess("readCheckpoint decodes the id as u32 in-process") {
47+
return;
48+
}
4349
with_e2e_context(
4450
"rpc_workspace_checkpoints",
4551
"should_return_null_or_empty_content_for_unknown_checkpoint",

rust/tests/e2e/support.rs

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ where
3636
.await
3737
.unwrap_or_else(|err| panic!("create E2E context: {err}"));
3838

39+
// In-process hosting: the runtime loads into this test process and its worker
40+
// inherits the ambient environment (per-client env is not honored in-process, see
41+
// https://github.com/github/copilot-sdk/issues/1934), so mirror this context's env
42+
// onto the process for the duration of the test and restore on drop. Safe because
43+
// E2E_CONCURRENCY is 1 in-process, serializing the whole critical section.
44+
let _env_guard = InProcessEnvGuard::activate(&ctx);
45+
3946
let timed_out = tokio::time::timeout(default_test_timeout(), test(&mut ctx))
4047
.await
4148
.is_err();
@@ -65,6 +72,10 @@ where
6572
.await
6673
.unwrap_or_else(|err| panic!("create E2E context: {err}"));
6774

75+
// See `with_e2e_context` for why the in-process transport mirrors env onto the
76+
// process (restored on drop).
77+
let _env_guard = InProcessEnvGuard::activate(&ctx);
78+
6879
let timed_out = tokio::time::timeout(default_test_timeout(), test(&mut ctx))
6980
.await
7081
.is_err();
@@ -546,13 +557,89 @@ fn default_test_timeout() -> Duration {
546557
}
547558

548559
fn e2e_concurrency() -> usize {
560+
// The in-process transport mirrors per-test environment onto the shared process
561+
// environment (see `InProcessEnvGuard`), which is only coherent when one test runs
562+
// at a time. Force serial execution in-process; otherwise honor RUST_E2E_CONCURRENCY.
563+
if is_inprocess_default() {
564+
return 1;
565+
}
549566
std::env::var("RUST_E2E_CONCURRENCY")
550567
.ok()
551568
.and_then(|value| value.parse::<usize>().ok())
552569
.filter(|&value| value > 0)
553570
.unwrap_or(4)
554571
}
555572

573+
/// True when the E2E suite runs over the in-process (FFI) transport, i.e. the SDK
574+
/// resolves `COPILOT_SDK_DEFAULT_CONNECTION=inprocess` to [`Transport::InProcess`].
575+
pub fn is_inprocess_default() -> bool {
576+
std::env::var("COPILOT_SDK_DEFAULT_CONNECTION")
577+
.map(|value| value.eq_ignore_ascii_case("inprocess"))
578+
.unwrap_or(false)
579+
}
580+
581+
/// Skip guard for E2E tests exercising features the in-process (FFI) transport does not
582+
/// support (the runtime loads into the shared host process). Returns `true` — and logs —
583+
/// when running in-process so the caller can `return` early; such tests remain covered
584+
/// by the default (stdio) transport. See <https://github.com/github/copilot-sdk/issues/1934>.
585+
pub fn skip_inprocess(reason: &str) -> bool {
586+
if is_inprocess_default() {
587+
eprintln!("skipping test over the in-process (FFI) transport: {reason}");
588+
true
589+
} else {
590+
false
591+
}
592+
}
593+
594+
/// Mirrors an [`E2eContext`]'s environment onto the real process environment for the
595+
/// in-process transport, whose worker inherits this process's ambient environment
596+
/// rather than a per-client env block. Restores the previous values on drop. Only the
597+
/// in-process transport needs this; for stdio/tcp the environment is handed to the
598+
/// spawned child directly. Auth flows via GH_TOKEN/GITHUB_TOKEN and HMAC is disabled so
599+
/// host-side auth resolution picks the token the replay snapshots expect.
600+
struct InProcessEnvGuard {
601+
saved: Vec<(OsString, Option<OsString>)>,
602+
}
603+
604+
impl InProcessEnvGuard {
605+
/// Returns `Some` guard (having applied the env) when in-process, else `None`.
606+
fn activate(ctx: &E2eContext) -> Option<Self> {
607+
if !is_inprocess_default() {
608+
return None;
609+
}
610+
let mut pairs: Vec<(OsString, OsString)> = ctx.environment();
611+
pairs.push(("GH_TOKEN".into(), "fake-token-for-e2e-tests".into()));
612+
pairs.push(("GITHUB_TOKEN".into(), "fake-token-for-e2e-tests".into()));
613+
614+
let mut saved: Vec<(OsString, Option<OsString>)> = Vec::new();
615+
for (key, value) in &pairs {
616+
saved.push((key.clone(), std::env::var_os(key)));
617+
// SAFETY: the E2E suite runs serially in-process (concurrency 1), so no
618+
// other thread races these process-wide env mutations.
619+
unsafe { std::env::set_var(key, value) };
620+
}
621+
for key in ["COPILOT_HMAC_KEY", "CAPI_HMAC_KEY"] {
622+
let key = OsString::from(key);
623+
saved.push((key.clone(), std::env::var_os(&key)));
624+
// SAFETY: as above.
625+
unsafe { std::env::remove_var(&key) };
626+
}
627+
Some(Self { saved })
628+
}
629+
}
630+
631+
impl Drop for InProcessEnvGuard {
632+
fn drop(&mut self) {
633+
for (key, previous) in self.saved.iter().rev() {
634+
// SAFETY: as in `activate` — serial execution in-process.
635+
match previous {
636+
Some(value) => unsafe { std::env::set_var(key, value) },
637+
None => unsafe { std::env::remove_var(key) },
638+
}
639+
}
640+
}
641+
}
642+
556643
pub fn get_system_message(exchange: &serde_json::Value) -> String {
557644
exchange
558645
.get("request")

rust/tests/e2e/telemetry.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ use super::support::{assistant_message_content, with_e2e_context};
1313

1414
#[tokio::test]
1515
async fn should_export_file_telemetry_for_sdk_interactions() {
16+
// Telemetry lowers to environment variables the in-process worker cannot receive
17+
// per-client; covered by the default (stdio) transport. See issue #1934.
18+
if super::support::skip_inprocess("telemetry configuration is not honored in-process") {
19+
return;
20+
}
1621
with_e2e_context(
1722
"telemetry",
1823
"should_export_file_telemetry_for_sdk_interactions",

0 commit comments

Comments
 (0)