Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,17 @@ jobs:
working-directory: src-tauri
run: cargo test ${{ matrix.cargo_args }} ${{ (matrix.os == 'windows-latest' && matrix.mode == 'desktop') && '--no-run' || '' }}

- name: cargo test (vendored sacp-tokio)
# `vendor/sacp-tokio` is a path dependency, NOT a workspace member, so
# the cargo test above never reaches its unit tests — including the ones
# covering codeg's own patches to it (process-exit reporting, cwd
# handling). One Linux desktop cell is enough: the crate is plain
# tokio/futures with no system deps, and its process tests are
# `#[cfg(unix)]`.
if: matrix.os == 'ubuntu-22.04' && matrix.mode == 'desktop'
working-directory: src-tauri
run: cargo test --manifest-path vendor/sacp-tokio/Cargo.toml --lib

- name: cargo clippy
# Clippy on each (os, mode) cell — desktop covers tauri-only code
# paths plus integration tests, server covers cfg-gated server-only
Expand Down
41 changes: 40 additions & 1 deletion src-tauri/src/acp/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,23 @@ pub struct AgentConnection {
/// no-op save (identical values) stays silent. Starts equal to
/// `config_fingerprint`.
pub last_observed_fingerprint: String,
/// OS process id of the spawned agent subprocess, published by the
/// vendored `sacp-tokio` `on_spawn` callback. `0` until the process has
/// launched (or if the pid was never observed). Used only as a shutdown
/// backstop: `disconnect_all` kills this pid's whole process tree
/// synchronously after the graceful-disconnect grace window, so agents
/// (and their own child processes, e.g. MCP servers) never leak as orphans
/// when the host process exits before `ChildGuard::drop` can run on the
/// connection driver thread.
///
/// Reset to `0` by the paired `on_exit` callback the moment the process is
/// *reaped* — the only moment its pid stops naming our child and becomes
/// reassignable. That reset is what keeps the backstop from ever aiming at
/// a pid the OS has since handed to an unrelated process. Notably it does
/// NOT fire merely because the connection ended: `ChildGuard::drop` signals
/// the tree without waiting, so the agent may still be alive and still
/// needs the backstop.
pub child_pid: Arc<std::sync::atomic::AtomicU32>,
}

impl AgentConnection {
Expand Down Expand Up @@ -956,7 +973,28 @@ pub async fn spawn_agent_connection(
// agree. Computed here because `working_dir` is moved into run_connection
// below.
let launch_cwd = resolve_working_dir(working_dir.as_deref());
let agent = build_agent(agent_type, &runtime_env, &launch_cwd).await?;
// Shared cell that receives the agent process's OS pid the instant it
// spawns (via `on_spawn` below). Stored on the `AgentConnection` so the
// shutdown path can `kill_tree` the process tree synchronously as a
// backstop when the connection driver thread is torn down by process exit
// before `ChildGuard::drop` can run. 0 = not spawned yet / unknown.
let child_pid = Arc::new(std::sync::atomic::AtomicU32::new(0));
let agent = build_agent(agent_type, &runtime_env, &launch_cwd)
.await?
.on_spawn({
let child_pid = Arc::clone(&child_pid);
move |pid| child_pid.store(pid, std::sync::atomic::Ordering::SeqCst)
})
// Paired with `on_spawn`: publish 0 again once the process has been
// reaped, so the shutdown backstop can never `kill_tree` a pid the OS
// has already handed to someone else. Fires ONLY on a real reap — a
// connection that merely ended keeps its pid published, because the
// vendored `ChildGuard` signals the tree without waiting and the agent
// may still be running.
.on_exit({
let child_pid = Arc::clone(&child_pid);
move || child_pid.store(0, std::sync::atomic::Ordering::SeqCst)
});

// Path policy for the ACP `fs/*` channel. Built HERE rather than inside
// `run_connection` because it needs the full `runtime_env` (only the git
Expand Down Expand Up @@ -1012,6 +1050,7 @@ pub async fn spawn_agent_connection(
prompt_lock: Arc::new(tokio::sync::Mutex::new(())),
last_observed_fingerprint: config_fingerprint.clone(),
config_fingerprint,
child_pid,
},
);

Expand Down
1 change: 1 addition & 0 deletions src-tauri/src/acp/lifecycle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1653,6 +1653,7 @@ mod tests {
prompt_lock: Arc::new(tokio::sync::Mutex::new(())),
config_fingerprint: String::new(),
last_observed_fingerprint: String::new(),
child_pid: Arc::new(std::sync::atomic::AtomicU32::new(0)),
}
}

Expand Down
Loading
Loading