github / github/copilot-sdk

[runtime] In-process host must consume per-client env_json instead of reading ambient process environment

Offen
#2,533 0 Kommentare 0 Reaktionen 0 zugewiesene Personen Auf GitHub ansehen
enhancement runtime sdk-v2
Vorherrschende Sprache
Java
Sterne
10.5k
Forks
1.5k
Ø Merge
1 T. 11 Std.
Gemergte PRs (30 T.)
127

Beschreibung

## Summary

Split out from #2523 (v2 redesign of process/transport configuration APIs). #2523's "Required changes" explicitly ask to "ensure the runtime consumes the `host_start` `env_json` contract for host-side reads so configuration remains per client rather than ambient to the host process," and notes: "the first-class configuration and `env_json` work may not be possible without runtime changes. If runtime changes are required, file a single follow-up issue describing the complete runtime work and treat implementation of those runtime changes as out of scope for this SDK issue."

Investigation confirms runtime-side changes are required. This issue consolidates the complete runtime work; implementing it is explicitly out of scope for #2523.

## Evidence

PR #1920 (.NET E2E harness) documented and worked around this exact gap rather than fixing it at the source:

> Some runtime code paths run *host-side* in the SDK's own process (the loaded cdylib) and read the **ambient process environment** rather than the environment passed to `copilot_runtime_host_start`:
> - native `fetch_copilot_user` reads `COPILOT_DEBUG_GITHUB_API_URL` via `std::env::var`
> - the gh-CLI auth fallback spawns `gh auth token`, which inherits `GH_TOKEN` / `GITHUB_TOKEN` / `GH_CONFIG_DIR`

Their harness fix was scoped as a workaround (mirroring select vars onto the real process env, gated to in-process sessions only) with an explicit note: "the proper long-term fix belongs in the runtime: thread the `host_start` environment into these host-side reads instead of consulting the global process env."

## Why this matters beyond the E2E harness

For a single in-process client per host process, ambient-env reads happen to resolve correctly today because the one client's env and the process's env coincide. But the SDKs' in-process (FFI) transport is explicitly designed to let one host process load the runtime and serve **multiple independently-configured clients** (see #1976, which added Go/Python/Rust in-process parity and the shared `InProcessConnection` boundary). Any host-side runtime code path that consults the ambient process environment instead of the per-client `env_json` passed to `host_start` cannot be configured per-client — the second client to start will silently observe the first client's (or the host process's own) values for these specific paths, or vice versa. This is a real correctness gap for any embedder hosting more than one client, not just a harness quirk.

## Required runtime work

1. Audit all host-side (native, in-process-loaded) code paths in copilot-agent-runtime for reads of the raw process environment (`std::env::var`/equivalent) for values that are supposed to be per-client/per-`host_start`-call configuration. Known instances from #1920:
- `fetch_copilot_user`'s use of `COPILOT_DEBUG_GITHUB_API_URL`
- the `gh auth token` fallback's inheritance of `GH_TOKEN` / `GITHUB_TOKEN` / `GH_CONFIG_DIR` (spawned subprocess inherits ambient env rather than being given an explicit env derived from the `host_start` `env_json`)
2. Thread the `env_json` supplied to `copilot_runtime_host_start` through to these paths so they resolve per-client configuration instead of ambient host-process state, consistent with the `env_json` contract's intent as already implemented for other in-process configuration (log level, idle timeout, remote/auth token, base directory, keytar-disable — see #1976, #1993).
3. Add or extend runtime-side (or cross-SDK E2E) test coverage that starts two in-process clients with different auth/config in the same host process and asserts neither observes the other's (or the ambient process's) values for the affected paths — this is the scenario that would have caught the original gap without needing a harness-only workaround.
4. Once fixed, evaluate whether the .NET E2E harness workaround from #1920 (mirroring select env vars onto the real process environment via `setenv`, gated to in-process sessions) can be simplified or removed, since the underlying runtime gap it compensates for will no longer exist.

## Out of scope for this issue

- SDK-side public API changes (client-wide vs. transport-scoped option placement, "out of process" naming, validation) — these are handled in #2523 directly and do not require runtime changes.
- Runtime artifact acquisition/embedding (owned by #2524) and lifecycle/SQLite work (owned by #2525).

## References

- #2523 (parent, requires this follow-up)
- #1920 (original discovery + harness-only workaround)
- #1976 (in-process transport parity across Go/Python/Rust, establishes multi-client-capable in-process hosting as an explicit design goal)
- #1993

Beitragsleitfaden

Beitragsleitfaden öffnen

Rechercherichtung

Start at the copilot_runtime_host_start entry point and trace env_json into the host-side fetch_copilot_user path and gh auth token fallback. Add runtime-side or cross-SDK coverage for two in-process clients with different environments, then verify affected reads are isolated per client and evaluate the .NET harness workaround from #1920.

Vom Indexierungsmodell aus dem Issue-Text verfasst.

Bewertung

Tech-Stack
rust
Bereich
api, authentication, backend
Issue-Typ
Bug
Schwierigkeit
5/5
Geschätzter Aufwand
Über eine Woche
Aktivitätsstatus
Aktiv
Klarheit
Größtenteils klar
Anfängerfreundlichkeit
38/100

Neue Issues direkt in Ihr Postfach

Eine kurze Übersicht über anfängerfreundliche GitHub-Issues.