crates/gunnar-client/tests/push_waits_for_the_remote.rs
//! **A push must not let go of the channel until the remote has finished.**
//!
//! # The defect
//!
//! `drive_push` returned the moment it had parsed `report-status-v2`. On SSH the
//! remote is not finished at that point: `git-receive-pack` still has to exit and
//! the SSH server still has to send `exit-status` and then close the channel.
//! russh sends the client's own `Close` from `Drop`, so the two race.
//!
//! MEASURED 2026-08-03, `gunnar.real_repo_clone` — 19 real repositories
//! pushed into a real `gunnar serve`: **3 of the 19** made the server log
//!
//! ```text
//! the client was gone before the exit status status=0
//! ```
//!
//! `status=0`. Every one of those pushes succeeded, and the server could not
//! deliver the fact. Stock git cannot have this bug because it does not have the
//! shape: `git push` waits for its `ssh` subprocess, and `ssh` does not exit
//! until it has seen `exit-status`.
//!
//! # What is asserted, and why it is not the log line
//!
//! The log line lives on the server and only appears on the losing side of a
//! race, so a test that watched for it would be green on a good day and green on
//! a broken one. What is deterministic, and what actually orders the two
//! messages, is the CLIENT's behaviour at its own seam: it must read its
//! transport **to EOF** before returning. Every transport this crate has —
//! russh's channel, a pipe to a child process — delivers "the far end is done"
//! as a zero-length read and nothing else.
//!
//! So the transport here is the real `git receive-pack` over real pipes, with a
//! reader that records whether it was ever read to EOF. That flag is applied
//! output: it is false for the code as it shipped and true after the fix, and
//! `git receive-pack` is a real remote that really does close its stdout when it
//! exits.
mod common;
use std::io::{Read, Write};
use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering};
use common::*;
use gunnar_client::{push_over, PushOptions, RefSpec, Transport};
/// A reader that notices being read to EOF.
struct WatchedReader<'a, R> {
inner: R,
saw_eof: &'a AtomicBool,
reads_after_report: &'a AtomicUsize,
}
impl<R: Read> Read for WatchedReader<'_, R> {
fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
let n = self.inner.read(buf)?;
if n == 0 {
self.saw_eof.store(true, Ordering::SeqCst);
}
self.reads_after_report.fetch_add(1, Ordering::SeqCst);
Ok(n)
}
}
struct Watched<'a, R, W> {
reader: WatchedReader<'a, R>,
writer: W,
}
impl<R: Read, W: Write> Transport for Watched<'_, R, W> {
fn io(&mut self) -> (&mut dyn Read, &mut dyn Write) {
(&mut self.reader, &mut self.writer)
}
}
/// The headline. A real push over a real `git receive-pack`, and the client must
/// have seen the remote's EOF by the time it returns.
#[test]
fn the_client_waits_for_the_remotes_end_of_stream() {
for kind in BOTH_HASH_KINDS {
let f = Fixture::new(kind);
commit_file(&f.src(), "a.txt", "hello\n", "first");
commit_file(&f.src(), "b.txt", "more\n", "second");
let repo = open(&f.src());
let child = GitChild::receive_pack(&f.bare());
let (stdout, stdin, mut proc) = child.into_halves();
let saw_eof = AtomicBool::new(false);
let reads = AtomicUsize::new(0);
let mut transport = Watched {
reader: WatchedReader {
inner: stdout,
saw_eof: &saw_eof,
reads_after_report: &reads,
},
writer: stdin,
};
let specs = RefSpec::parse_all(&["refs/heads/main:refs/heads/main"]).unwrap();
let outcome = push_over(&repo, &mut transport, &specs, &PushOptions::new()).expect("push");
assert!(
outcome.is_ok(),
"{kind:?}: the push was rejected: {outcome:?}"
);
assert!(
saw_eof.load(Ordering::SeqCst),
"{kind:?}: the push returned without ever reading its transport to EOF, so the \
remote had not finished and nothing orders the client's channel close after \
the server's exit-status. {} reads were made in total",
reads.load(Ordering::SeqCst)
);
// …and the remote really did finish cleanly, which is what makes the
// EOF above the remote's own doing rather than a broken pipe.
drop(transport);
let status = proc.wait().expect("wait for git receive-pack");
assert!(
status.success(),
"{kind:?}: git receive-pack exited {status:?}"
);
// The push still did its job. An EOF observed because the client hung up
// on itself would satisfy the assertion above and nothing else.
assert_eq!(
git(&f.bare(), &["rev-parse", "refs/heads/main"]),
git(&f.src(), &["rev-parse", "refs/heads/main"]),
"{kind:?}: the ref did not move"
);
}
}
/// The same for the two paths that send a flush and no pack: a push with nothing
/// to do, and a dry run. Both used to return without a single read.
#[test]
fn the_no_pack_paths_wait_too() {
let kind = gix::hash::Kind::Sha1;
// ── nothing to do: the remote already has this ref ──────────────────────
{
let f = Fixture::new(kind);
commit_file(&f.src(), "a.txt", "hello\n", "first");
git(
&f.src(),
&[
"push",
"--quiet",
&f.bare().to_string_lossy(),
"refs/heads/main:refs/heads/main",
],
);
let repo = open(&f.src());
let (stdout, stdin, mut proc) = GitChild::receive_pack(&f.bare()).into_halves();
let saw_eof = AtomicBool::new(false);
let reads = AtomicUsize::new(0);
let mut transport = Watched {
reader: WatchedReader {
inner: stdout,
saw_eof: &saw_eof,
reads_after_report: &reads,
},
writer: stdin,
};
let specs = RefSpec::parse_all(&["refs/heads/main:refs/heads/main"]).unwrap();
let outcome = push_over(&repo, &mut transport, &specs, &PushOptions::new()).expect("push");
assert!(
outcome.report.is_none() && !outcome.up_to_date.is_empty(),
"the fixture must actually take the up-to-date path: {outcome:?}"
);
assert!(
saw_eof.load(Ordering::SeqCst),
"an up-to-date push returned without waiting for the remote"
);
drop(transport);
let _ = proc.wait();
}
// ── a dry run ───────────────────────────────────────────────────────────
{
let f = Fixture::new(kind);
commit_file(&f.src(), "a.txt", "hello\n", "first");
let repo = open(&f.src());
let (stdout, stdin, mut proc) = GitChild::receive_pack(&f.bare()).into_halves();
let saw_eof = AtomicBool::new(false);
let reads = AtomicUsize::new(0);
let mut transport = Watched {
reader: WatchedReader {
inner: stdout,
saw_eof: &saw_eof,
reads_after_report: &reads,
},
writer: stdin,
};
let mut opts = PushOptions::new();
opts.dry_run = true;
let specs = RefSpec::parse_all(&["refs/heads/main:refs/heads/main"]).unwrap();
let outcome = push_over(&repo, &mut transport, &specs, &opts).expect("dry run");
assert!(outcome.dry_run && outcome.objects_sent > 0, "{outcome:?}");
assert!(
saw_eof.load(Ordering::SeqCst),
"a dry run returned without waiting for the remote"
);
drop(transport);
let _ = proc.wait();
}
}