Make the process record survive a crash mid-write
Reviewing the reattach code found the fault it exists to prevent, sitting in its own save point. `process::write` used `fs::write`, which truncates before it fills. A crash inside that window leaves no readable record -- and a missing record reads as "nothing is running", which is the single answer that makes the next launch start a *second* CLI against a conversation that already has one. The window is not rare: the record is rewritten on every read that makes progress, so many times a second while a turn is producing output. Written to a neighbouring file and renamed over the real name now. The rename is atomic, so a reader sees the whole old record or the whole new one. That also makes the fixed-size padding pointless -- a rename replaces the file rather than overwriting part of it -- so it goes. Two more from the same pass: - A failed read of the stdout log was logged and nothing else. The session then went deaf with nothing on screen: no more output, no error, a status that stayed wherever it was. It now says so, closes the queue rather than stranding messages in it, and reports `Unknown` -- not `Exited`, because the process may well still be running; what failed is this server's ability to hear it. - Sizing the stderr log by reading it. `read_from` with a large offset answers "how long is it" by allocating the whole file first, which on a chatty process is a large pointless read on every reattach. `size_of` asks the filesystem. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VETa8afmpWaYezLCqJhDB8
This commit is contained in:
1 parent
aff2cb90e2
commit
3a74bd9c35
2 files changed
+101
-28
No files matched your search
@@ -137,7 +137,7 @@ impl Queue {
|
||||
/// Reported rather than dropped. These are messages somebody typed
|
||||
/// that never reached the session and never reached the transcript, so
|
||||
/// this is the only place they can be mentioned at all.
|
||||
fn close(&mut self, sink: &EventSink) {
|
||||
fn close(&mut self, sink: &EventSink, why: &str) {
|
||||
self.closed = true;
|
||||
self.running = false;
|
||||
let lost: Vec<String> = self.waiting.drain(..).map(|(text, _)| text).collect();
|
||||
@@ -146,7 +146,7 @@ impl Queue {
|
||||
}
|
||||
let _ = sink.send(Event::Error {
|
||||
message: format!(
|
||||
"the session ended before it read {}: {}",
|
||||
"{why} before it read {}: {}",
|
||||
if lost.len() == 1 {
|
||||
"this message".to_string()
|
||||
} else {
|
||||
@@ -526,17 +526,35 @@ async fn follow(
|
||||
// run of this server was watching when it was written, so a reattach
|
||||
// starts at the end of it rather than repeating it. The tail is still
|
||||
// read from the file if the process dies, which is when it matters.
|
||||
let mut stderr_at = match process::read_from(&stderr_path, u64::MAX) {
|
||||
Ok((_, at)) => at,
|
||||
Err(_) => 0,
|
||||
};
|
||||
let mut stderr_at = process::size_of(&stderr_path);
|
||||
let mut said_unknown = false;
|
||||
|
||||
while reading.load(Ordering::SeqCst) {
|
||||
let (bytes, _) = match process::read_from(&stdout_path, offset) {
|
||||
Ok(found) => found,
|
||||
Err(err) => {
|
||||
// Reported, not only logged. This is the end of the
|
||||
// session's output as far as anyone watching is
|
||||
// concerned, and a phone told nothing shows a session
|
||||
// that is merely quiet -- indistinguishable from one
|
||||
// thinking. The status is `Unknown` rather than `Exited`
|
||||
// because the process may well still be running; what
|
||||
// has failed is this server's ability to hear it.
|
||||
tracing::error!("couldn't read {}: {err:#}", stdout_path.display());
|
||||
let _ = sink.send(Event::Error {
|
||||
message: format!(
|
||||
"lost track of {label}: its output can't be read ({err:#}). The process \
|
||||
may still be running; restarting the backend will try to pick it up \
|
||||
again."
|
||||
),
|
||||
});
|
||||
queue
|
||||
.lock()
|
||||
.unwrap()
|
||||
.close(&sink, "this server lost track of the session");
|
||||
let _ = sink.send(Event::Status {
|
||||
state: SessionStatus::Unknown,
|
||||
});
|
||||
return;
|
||||
}
|
||||
};
|
||||
@@ -596,7 +614,7 @@ async fn follow(
|
||||
// for ever and the exit would never be reported.
|
||||
process::Liveness::Dead if complete > 0 => {}
|
||||
process::Liveness::Dead => {
|
||||
queue.lock().unwrap().close(&sink);
|
||||
queue.lock().unwrap().close(&sink, "the session ended");
|
||||
let detail = stderr_tail(&stderr_path);
|
||||
if !detail.is_empty() {
|
||||
let _ = sink.send(Event::Error {
|
||||
@@ -901,7 +919,7 @@ mod tests {
|
||||
};
|
||||
queue.waiting.push_back(("first".into(), "{}".into()));
|
||||
queue.waiting.push_back(("second".into(), "{}".into()));
|
||||
queue.close(&sink);
|
||||
queue.close(&sink, "the session ended");
|
||||
|
||||
// Named rather than counted, because these never reached the
|
||||
// transcript: this message is the only record they existed.
|
||||
@@ -909,6 +927,7 @@ mod tests {
|
||||
panic!("closing a queue holding messages must report them");
|
||||
};
|
||||
assert!(message.contains("2 queued messages"), "{message}");
|
||||
assert!(message.starts_with("the session ended"), "{message}");
|
||||
assert!(
|
||||
message.contains("first") && message.contains("second"),
|
||||
"{message}"
|
||||
@@ -924,7 +943,7 @@ mod tests {
|
||||
fn closing_an_empty_queue_says_nothing() {
|
||||
let (sink, mut received) = mpsc::unbounded_channel();
|
||||
let mut queue = Queue::default();
|
||||
queue.close(&sink);
|
||||
queue.close(&sink, "the session ended");
|
||||
// A session that exits with nothing held has lost nothing, and an
|
||||
// error saying so would be noise on every ordinary exit.
|
||||
assert!(received.try_recv().is_err());
|
||||
|
||||
Reference in new issue
Block a user