Skip to content

fix(acp): bound Unix terminal cancellation#370

Open
keh4l wants to merge 1 commit into
xintaofei:mainfrom
keh4l:codex/upstream-acp-terminal-cancel
Open

fix(acp): bound Unix terminal cancellation#370
keh4l wants to merge 1 commit into
xintaofei:mainfrom
keh4l:codex/upstream-acp-terminal-cancel

Conversation

@keh4l

@keh4l keh4l commented Jul 22, 2026

Copy link
Copy Markdown

Summary

  • launch Unix ACP terminal commands in isolated process groups and clean them up with bounded SIGINT (500 ms), SIGTERM (1500 ms), then SIGKILL (2000 ms) escalation
  • avoid holding the child mutex across exit waits, clean multiple session terminals concurrently, and cap Cancel's outer terminal-cleanup wait at 5 seconds
  • always converge the connection to TurnComplete(cancelled) even when terminal cleanup times out, while reporting incomplete cleanup as a recoverable error
  • track process-group ownership with leader/descendant/retired lease states, retire permanently on ESRCH or expiry, and serialize leader-exit observation with cleanup to prevent stale-PGID signalling

Verification

  • terminal-runtime tests — 29 passed, including a real stubborn process group, unrelated-session isolation, delayed release, lease expiry/ESRCH, and the leader-reap concurrency barrier
  • Cancel convergence test — 1 passed
  • cargo check --no-default-features --bin codeg-server
  • cargo test --no-default-features --bin codeg-server --lib — 1650 passed, 1 ignored
  • cargo clippy --no-default-features --bin codeg-server --lib -- -D warnings
  • cargo check --no-default-features --bin codeg-mcp
  • cargo clippy --no-default-features --bin codeg-mcp -- -D warnings
  • no controlled-test child processes remained after the suite

Scope and limitations

  • Unix behavior is changed; Windows keeps the existing kill_tree path.
  • This does not change agent acknowledgement or late-result reconciliation semantics tracked in [Bug] ACP cancellation is finalized before agent confirmation, so Pi can keep running and late output is lost #364. It addresses bounded terminal-process cleanup and connection-state convergence after Cancel.
  • POSIX has no generation-bound process-group handle, so a final probe/signal syscall-level TOCTOU remains within the non-renewing five-second descendant lease. The previous unbounded stale-PGID window is removed.
  • Desktop cargo check is not reported because this host lacks the GTK/GLib development packages required by glib-sys.
  • Full-repository cargo fmt --check already fails on upstream main; this PR adds no new failing file and formats terminal_runtime.rs, which is part of the upstream failure set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants