max / alloy

run: wait for a command's output before declaring it done try_wait can reap a child whose last lines are still in the pipe, unread by the pump threads. The post-exit drain used try_recv, saw Empty, and dropped those lines with the Running that went out of scope next. Two things were losing output: a failing command's explanation, and the capture a Stage::Resolve parses -- root_partition and deployment_dir could be handed an empty string mid-install and misreport why they failed. The sequence tests caught it about two runs in three. Drain blocking to Disconnected after exit instead. That is the event meaning both pumps hit EOF and forwarded everything, and it is bounded because the child is already dead.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: Max Johnson <me@maxj.phd> · 2026-07-20 17:17 UTC

Signed with PGP, not checked

Commit: f099d9b70bdec6c95eae3f5aa90a3b448353ae02

Parent: 78944d0

1 file changed, +33 insertions, -8 deletions

M crates/alloy/src/run.rs +33 -8

			@@ -95,12 +95,7 @@
95	95		fn drain(&self, into: &mut Vec<String>) {
96	96		loop {
97	97		match self.lines.try_recv() {
98		-	Ok(line) => {
99		-	if into.len() == SCROLLBACK {
100		-	into.remove(0);
101		-	}
102		-	into.push(line);
103		-	}
	98	+	Ok(line) => push_line(into, line),
104	99		// Disconnected means both readers are done, which is a fact
105	100		// about the pipes rather than about the process: the child may
106	101		// still be exiting. `finished` is what decides that.
			@@ -109,12 +104,39 @@
109	104		}
110	105		}
111	106
	107	+	/// Drain everything the command will ever produce, blocking until the
	108	+	/// readers are done.
	109	+	///
	110	+	/// Only correct once the child has exited, and only safe then: reaping the
	111	+	/// child is not the same event as its output arriving. `try_wait` can reap
	112	+	/// a process whose last lines are still sitting in the pipe, unread by the
	113	+	/// pump threads, and a `try_recv` at that moment reports `Empty` and takes
	114	+	/// the lines to the grave with the `Running` that gets dropped next.
	115	+	///
	116	+	/// Blocking to `Disconnected` waits for the one event that means all
	117	+	/// output has been forwarded: both pump threads reaching EOF and dropping
	118	+	/// their senders. The wait is bounded because the child is already dead,
	119	+	/// so both pipes are closed and EOF is already on its way.
	120	+	fn drain_to_end(&self, into: &mut Vec<String>) {
	121	+	while let Ok(line) = self.lines.recv() {
	122	+	push_line(into, line);
	123	+	}
	124	+	}
	125	+
112	126		/// The exit status if the child is done, `None` while it runs.
113	127		fn finished(&mut self) -> Option<std::io::Result<ExitStatus>> {
114	128		self.child.try_wait().transpose()
115	129		}
116	130		}
117	131
	132	+	/// Append a line, holding the buffer to [`SCROLLBACK`].
	133	+	fn push_line(into: &mut Vec<String>, line: String) {
	134	+	if into.len() == SCROLLBACK {
	135	+	into.remove(0);
	136	+	}
	137	+	into.push(line);
	138	+	}
	139	+
118	140		/// A queue of commands, run one at a time, with their output streamed.
119	141		///
120	142		/// Owns the whole sequence rather than one command because the install is a
			@@ -199,10 +221,13 @@
199	221		// The pipes can still hold output written just before exit,
200	222		// so drain once more after reaping. Without this the last
201	223		// lines of a failing command — the ones that say why — are
202		-	// the ones that get lost.
	224	+	// the ones that get lost, and a resolver reading this
	225	+	// command's output parses whatever happened to arrive in
	226	+	// time. Blocking, because a non-blocking drain here is a
	227	+	// race against the pump threads that the pumps often lose.
203	228		let mut lines = std::mem::take(&mut self.output);
204	229		let mut finished = self.current.take().expect("current was Some");
205		-	finished.drain(&mut lines);
	230	+	finished.drain_to_end(&mut lines);
206	231		self.output = lines;
207	232		self.done_count += 1;
208	233