github / github/copilot-sdk

Expose a child-process configuration hook on `ClientOptions` so embedders can isolate and reap the agent's process tree

Open
#1,935 0 comments 0 reactions 0 assignees View on GitHub
enhancement rust
Dominant language
Java
Stars
10.5k
Forks
1.5k
Avg merge
1d 11h
Merged PRs (30d)
127

Description

## Summary

When the Rust SDK spawns the agent (`Client::start` → `spawn_stdio` / `spawn_tcp` → `build_command` → `Command::spawn`), the embedder has no way to configure the child process at fork time, and the SDK's teardown paths act only on the immediate child PID. That makes it impossible for a long-lived host embedding the SDK to:

1. **Guarantee the agent's whole descendant tree is reaped** on shutdown or crash. `Client::stop` calls `child.kill().await`, and `Client::force_stop` / `Drop for ClientInner` call `child.start_kill()` — all of these signal only the immediate CLI child. Any processes the agent itself spawns (tool subprocesses, shells, MCP servers, language servers) are **not** part of that signal. Because the child is spawned in the embedder's own process group (nothing in `build_command` calls `process_group` / sets a new session), there is no distinct process group to signal, so those descendants are reparented to `init` and survive as orphans holding file handles and network connections.
2. **Place the agent in its own process group / session** so the embedder *can* do a clean `kill(-pgid)` + reap of the entire tree.
3. **Drop privileges before `execve`** (`setuid`/`setgid`/`setgroups`) so the agent doesn't inherit the host's full uid/gid.
4. **Install a parent-death signal** (Linux `prctl(PR_SET_PDEATHSIG)`) so a hard host crash (SIGKILL, OOM, abort) — where no Rust `Drop` runs — still tears the agent tree down instead of leaving it orphaned.

All four require running code in the child between `fork` and `execve`, or setting the standard `std::os::unix::process::CommandExt` knobs on the command the SDK builds. Today `ClientOptions` exposes none of these, and since the SDK owns the `Command` internally, the embedder can't reach it. (An embedder building its *own* `std::process::Command` could set these via `CommandExt` and then `tokio::process::Command::from(..)`, but that lever isn't available when the SDK does the spawning.)

## Why this matters

Embedders that run the agent as a long-lived, network-connected daemon on behalf of many sessions need agent subprocesses to be **confined and fully reaped** — no orphaned tool/MCP processes should outlive the agent, and none should outlive a host crash. Process-group / session isolation at spawn time plus a parent-death signal are the standard OS primitives for that guarantee, and they can only be set by the process that forks the child.

## Current behavior (public source, for reference)

- `ClientOptions` carries `program`, `prefix_args`, `working_directory`, `env`, `env_remove`, `extra_args`, `transport`, token/auth, telemetry, `base_directory`, etc. — **no** child-process/spawn configuration.
- `build_command` builds a `tokio::process::Command` and never calls `process_group`, sets a session, drops privileges, or installs a `pre_exec` hook.
- `Client::stop` → `child.kill().await` (immediate child only).
- `Client::force_stop` and `Drop for ClientInner` → `child.start_kill()` (immediate child only).
- `Client::pid()` exposes the immediate child PID; there is no `process_group()` accessor.

## Proposed API

Two complementary pieces; either alone is a big step, both together fully unblock the use cases above.

### 1. Structured, safe pass-through fields on `ClientOptions`

Thin wrappers over `std::os::unix::process::CommandExt` (Unix) and `std::os::windows::process::CommandExt` (Windows), applied in `build_command`:

```rust
pub struct ClientOptions {
// … existing fields …

/// Place the spawned agent in a process group. `Some(0)` makes the
/// child a new process-group leader (its PGID == its PID); `Some(pgid)`
/// joins an existing group. `None` (default) inherits the caller's
/// group — today's behavior. Maps to `CommandExt::process_group`.
pub process_group: Option,

/// Unix privilege drop applied before `execve`. Map to
/// `CommandExt::uid` / `gid` / `groups`.
#[cfg(unix)] pub uid: Option,
#[cfg(unix)] pub gid: Option,
#[cfg(unix)] pub groups: Option>,

/// Windows process creation flags (e.g. `CREATE_NEW_PROCESS_GROUP`),
/// OR-ed into the existing `CREATE_NO_WINDOW`. Maps to
/// `CommandExt::creation_flags`.
#[cfg(windows)] pub creation_flags: Option,
}
```

`process_group`, `uid`, `gid`, `groups` are all **safe** methods on `CommandExt`, so this subset needs no `unsafe` API surface and directly covers process-group isolation + privilege drop.

### 2. An escape-hatch child-setup hook

For everything the structured fields don't cover — `prctl(PR_SET_PDEATHSIG)`, `setrlimit`, sandbox entry, mount/user namespace setup — expose a callback the SDK runs on the built command just before `spawn`:

```rust
/// Called with the fully-built command immediately before spawn, so the
/// embedder can apply platform-specific configuration (e.g. `pre_exec`,
/// resource limits, sandbox entry) that the structured fields don't cover.
pub command_customizer: Option>,
```

A `command_customizer` keeps the SDK from having to enumerate and re-export every platform knob; the embedder applies `CommandExt::pre_exec` (which is `unsafe` and carries the usual async-signal-safety contract) or any other config itself. If a callback surface is undesirable, an explicit `pre_exec: Option io::Result<()> + Send + Sync>>` field would also work.

### 3. Reap the group, not just the PID (when the SDK created one)

If the agent is spawned as a process-group leader (via field #1), the SDK's own teardown paths (`stop`, `force_stop`, `Drop`) should signal the **process group** (`kill(-pgid, …)` on Unix; a Job Object or `GenerateConsoleCtrlEvent` on Windows) rather than only the immediate child, so descendants are reaped on the paths the embedder doesn't drive directly. At minimum, add a `Client::process_group() -> Option` accessor so an embedder that opted into a new group can perform the group reap itself.

## Alternatives considered

- **Embedder wraps `program` in a launcher** that `setsid`s / drops privileges / execs the real agent. Fragile: it fights the bundled-CLI resolution (the embedder doesn't know the extracted binary path), breaks `Client::pid()` semantics, and `setsid(1)` isn't portable (absent on macOS). A first-class SDK hook is cleaner and portable.
- **Post-spawn `setpgid` from the parent.** Not possible: `Client::start` returns after `execve`, and `setpgid(2)` fails with `EACCES` once the child has exec'd.

## Acceptance

- `ClientOptions` can request a new process group / session for the agent, privilege drop, and arbitrary child-side setup before `execve`.
- With a new group requested, the SDK reaps the whole group on `stop` / `force_stop` / `Drop`, **or** exposes the group id so the embedder can.
- Defaults are unchanged (no new group, no privilege drop, no customizer) so existing consumers are unaffected.

Contributor guide

Open the contributing guide

Research direction

Start with Client::start, spawn_stdio, spawn_tcp, and build_command, then trace Client::stop, Client::force_stop, and Drop for ClientInner. Define the cross-platform configuration and teardown behavior described in the acceptance criteria, preserving unchanged defaults and either reaping the created process group or exposing its identifier.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
operating-systems
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.