From 7d2f0c13b3049a3026a50b8ce39f6af5b225bed8 Mon Sep 17 00:00:00 2001
From: Dan <dan@soundmedia.com>
Date: Mon, 17 Aug 2026 14:01:22 -0500
Subject: [PATCH] fix: do not discard the shell's exit status when stdin closes
 first

The client stamps its fallback exit status of 1 into the result slot as
soon as EITHER pipe thread finishes. A terminal that closes the client's
stdin before the shell's ExitStatus frame arrives therefore reported 1 no
matter how the shell exited. Wait the existing bounded detach window for
the socket side before defaulting; the wait returns as soon as the frame
lands, so a normal exit is unaffected.

Regression test: close the attach client's stdin while the shell is still
sleeping toward exit 19; on master the client reports 1, patched it
reports 19.
---
 libshpool/src/protocol.rs  | 23 ++++++++++++++++++++
 shpool/tests/regression.rs | 43 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 65 insertions(+), 1 deletion(-)

diff --git a/libshpool/src/protocol.rs b/libshpool/src/protocol.rs
index 1ab50cbb..e1ec02bb 100644
--- a/libshpool/src/protocol.rs
+++ b/libshpool/src/protocol.rs
@@ -395,6 +395,29 @@ impl Client {
                     + (sock_to_stdout_h.is_finished() as usize);
 
                 if nfinished_threads > 0 {
+                    // A finished stdin->sock thread only means our stdin closed;
+                    // the shell's exit status may still be in flight. Stamping
+                    // the fallback now makes sock->stdout bail before reading
+                    // the ExitStatus frame, reporting 1 instead of the real
+                    // status, so wait briefly for the socket side. It returns as
+                    // soon as the frame lands, not after the whole window.
+                    if stdin_to_sock_h.is_finished() && !sock_to_stdout_h.is_finished() {
+                        common::sleep_unless(
+                            MAX_DETACH_WAIT_DUR,
+                            || {
+                                sock_to_stdout_h.is_finished()
+                                    || result_slot.lock().unwrap().is_some()
+                            },
+                            common::PollStrategy::Backoff {
+                                initial_interval: DETACH_BACKOFF_INITIAL_DUR,
+                                factor: 2.0,
+                                max_interval: DETACH_BACKOFF_MAX_STEP_DUR,
+                            },
+                        );
+                        nfinished_threads = (stdin_to_sock_h.is_finished() as usize)
+                            + (sock_to_stdout_h.is_finished() as usize);
+                    }
+
                     // If one of the threads has exited, but not the other
                     // make sure that the exit result slot has some contents
                     // so the other thread will exit the next time it wakes
diff --git a/shpool/tests/regression.rs b/shpool/tests/regression.rs
index e8a8e14d..d950b5ee 100644
--- a/shpool/tests/regression.rs
+++ b/shpool/tests/regression.rs
@@ -1,6 +1,6 @@
 use std::{fs, io::Write, process::Command, sync::mpsc, time::Duration};
 
-use anyhow::Context;
+use anyhow::{anyhow, Context};
 use ntest::timeout;
 
 mod support;
@@ -334,3 +334,44 @@ fn pager_exit_transitions_to_shell() -> anyhow::Result<()> {
 
     Ok(())
 }
+
+/// Regression test for the attach client discarding the shell's real exit
+/// status. The client stamps its fallback status of 1 into the result slot as
+/// soon as EITHER pipe thread finishes. A terminal that closes the client's
+/// stdin before the shell's ExitStatus frame arrives -- which any terminal
+/// does when it hangs up first -- therefore reported 1 no matter how the
+/// shell actually exited.
+///
+/// We close the client's stdin while the shell is still sleeping toward a
+/// distinctive exit code, so the stdin->sock thread always finishes first.
+#[test]
+#[timeout(30000)]
+fn stdin_close_does_not_discard_exit_status() -> anyhow::Result<()> {
+    let mut daemon_proc = support::daemon::Proc::new("norc.toml", DaemonArgs::default())
+        .context("starting daemon proc")?;
+
+    let mut attach_proc =
+        daemon_proc.attach("sh1", Default::default()).context("starting attach proc")?;
+    daemon_proc.await_event("daemon-bidi-stream-enter")?;
+
+    let mut line_matcher = attach_proc.line_matcher()?;
+    attach_proc.run_cmd("echo ready")?;
+    line_matcher.scan_until_re("ready$")?;
+
+    // MAX_DETACH_WAIT_DUR is 300ms: the frame must arrive inside the bounded
+    // window the client waits after its stdin side finishes.
+    attach_proc.run_cmd("sleep 0.15; exit 19")?;
+    // Close our stdin immediately: the fallback status must not win the race
+    // against the ExitStatus frame that arrives moments later.
+    drop(attach_proc.proc.stdin.take());
+
+    let code = attach_proc
+        .proc
+        .wait()
+        .context("waiting for attach proc to exit")?
+        .code()
+        .ok_or(anyhow!("no exit code"))?;
+    assert_eq!(code, 19, "shell exit status was discarded");
+
+    Ok(())
+}