Skip to content

fix: preserve sessions after client write resets - #444

Open
TeacherLi07 wants to merge 1 commit into
shell-pool:masterfrom
TeacherLi07:fix/client-write-disconnect
Open

TeacherLi07 wants to merge 1 commit into
shell-pool:masterfrom
TeacherLi07:fix/client-write-disconnect

Conversation

@TeacherLi07

@TeacherLi07 TeacherLi07 commented Oct 13, 2026 •

Copy link
Copy Markdown

Issue Link

Related: #335 and #442 (unattachable sessions; the triggers discussed there differ).

AI Policy Ack

The AI Policy has been read and acknowledged.

This PR was mainly completed by GPT-6.1-Sol on Codex , including investigation, implementation, regression tests, and local validation. The explanation below is explicitly agent-generated.

This PR was:

  • mostly or completely vibe coded
  • mostly or completely meat coded
  • bit of both

Description

Agent-generated explanation — GPT-6.1-Sol on Codex:

A heartbeat or MaybeSwitch write returning ECONNRESET currently terminates the session's long-lived shell->client worker. The shell stays alive, but subsequent attaches cannot send a new connection to the worker's disconnected control channel.

Treat client write/flush failures as hangups: shut down the socket to wake the input thread, clear the worker's client connection, and retain the output worker, PTY, and restore spool. A failed heartbeat acknowledges client_present=false so the existing attach cleanup can complete.

Two regression tests inject one ECONNRESET while the client remains connected. They verify client cleanup, the original shell PID, three successful attach/detach cycles with command output, and cleared attachment records. Both tests fail with the original error handling and pass with this change. The injection is disabled in builds without test_hooks.

Local validation on Ubuntu 24.04 / Rust 1.85.0:

  • Regression suite: 12 passed; var suite: 7 passed; normal template-switch tests: 2 passed.
  • A separate Linux harness also exercised a real kernel ECONNRESET: it fills a reduced Unix-socket send buffer with heartbeats and closes the peer during a blocked write. Reattach preserves the original shell and remains interactive.
  • cargo check --locked --workspace, nightly formatting, and git diff --check passed.
  • Clippy passed with -D warnings -A clippy::unwrap_or_default; without that allowance, it reports the existing .unwrap_or(vec![]) in the unchanged server.rs:128.
  • Existing detach::reattach and attach::force_attach fail on both the original implementation and the fix: their first-line assertions encounter ESC[?2004l instead of the expected shell output. The full suite and macOS were not run locally.

This change prevents these client write failures from killing the worker. Recovery of an already-dead worker and bounded writes for clients that stop reading remain separate work.

@google-cla

google-cla Bot commented Oct 13, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

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.

1 participant