Never let one unreadable line take a transcript down
Removing `Event::TaskNote` hours after adding it made every transcript that
had recorded one unreadable. `Transcript::open` parses every line, so `launch`
failed for those sessions and `SessionManager::new` logged
"couldn't relaunch session <id>" and skipped them -- and a skipped session has
no pump and no driver. On the phone that is no status, no history and nothing
sendable, for every live session that had run a background task. One
unfamiliar word took down every conversation it appeared in.
A transcript is append-only and permanent, so the set of kinds one can hold
only ever grows: what this build writes is not what it may have to read. A
line can come from a newer server, or from an older one that wrote a kind
since dropped, and neither may be able to end the file.
`Indexed::parse_at` degrades a line it cannot make sense of to
`Event::Unreadable { kind }` instead of failing the whole read. It keeps the
line's seq -- the cursors, the page bisection and the next-seq counter are all
addressed by it, and dropping the line would hand out a seq the file already
contains -- and carries the word the line called itself, so the phone can say
what is missing rather than that something is. A line with no readable seq is
still an error: that one cannot be placed at all.
`Event::TaskNote` comes back retired rather than deleted: deserializable,
never constructed, dated, with the reason on it. The phone folds it to no row,
which is the point -- an unreadable line correctly draws a placeholder, and
one per background task is the wall the row was removed for in the first
place.
Found while diagnosing a report that live sessions had lost their status and
could not be sent to. 173 server tests pass, including the new one, which
fails on the old code within a second.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
9cc52beb09
commit
fd71d876e1
6 files changed
+215
-7
No files matched your search
@@ -296,6 +296,19 @@ written, and the fold uses that same predicate to decide a reply is settled.
|
|||||||
|
|
||||||
## Things that have bitten
|
## Things that have bitten
|
||||||
|
|
||||||
|
- **A transcript outlives the enum.** Removing `Event::TaskNote` hours after
|
||||||
|
adding it made every transcript that had recorded one unreadable, so
|
||||||
|
`launch` failed for those sessions and `SessionManager::new` skipped them —
|
||||||
|
no status, nothing sendable, no new messages, for every live session that
|
||||||
|
had run a background task. **The set of kinds a transcript can hold only
|
||||||
|
ever grows**: a line may come from a newer server or from an older one that
|
||||||
|
wrote a kind since dropped, and one unfamiliar word must never be able to
|
||||||
|
end the file. `Indexed::parse_at` degrades a line it cannot read to
|
||||||
|
`Event::Unreadable { kind }`, keeping its seq — which is what everything
|
||||||
|
downstream is addressed by — and the phone draws it as a placeholder saying
|
||||||
|
which kind. Never delete a variant instead of retiring it; `Event::TaskNote`
|
||||||
|
is what retiring looks like, and the phone folds it to no row.
|
||||||
|
|
||||||
Project-specific only — a lesson that would bite any project on this machine
|
Project-specific only — a lesson that would bite any project on this machine
|
||||||
belongs in `~/.claude/TOOLCHAIN.md` or `~/.claude/MACHINE.md` instead.
|
belongs in `~/.claude/TOOLCHAIN.md` or `~/.claude/MACHINE.md` instead.
|
||||||
|
|
||||||
|
|||||||
@@ -706,6 +706,34 @@ those, and `open_tasks` covers the backgrounded command, which has no subagent
|
|||||||
to be found in the registry at all. Both are needed and neither subsumes the
|
to be found in the registry at all. Both are needed and neither subsumes the
|
||||||
other.
|
other.
|
||||||
|
|
||||||
|
### A transcript outlives this enum (2026-09-06)
|
||||||
|
|
||||||
|
**The set of event kinds a transcript can hold only ever grows.** It is
|
||||||
|
append-only and permanent, so what this build *writes* is not what it may have
|
||||||
|
to *read*: a line can come from a newer server, or from an older one that wrote
|
||||||
|
a kind since dropped.
|
||||||
|
|
||||||
|
That was learned the expensive way. `Event::TaskNote` was added and removed
|
||||||
|
again within hours, and every transcript that had recorded one became
|
||||||
|
unreadable — `Transcript::open` parses every line, so `launch` failed for those
|
||||||
|
sessions and `SessionManager::new` skipped them. On the phone that is a session
|
||||||
|
with no status, no history and nothing sendable: one unfamiliar word took down
|
||||||
|
every live conversation it appeared in.
|
||||||
|
|
||||||
|
Two rules now. `Indexed::parse_at` degrades a line it cannot make sense of to
|
||||||
|
`Event::Unreadable { kind }` rather than failing the file, keeping its seq —
|
||||||
|
which is what the cursors, the bisection and the next-seq counter are all
|
||||||
|
addressed by — and carrying the word the line called itself, so the reader is
|
||||||
|
told what they are missing rather than that something is. The seq is still
|
||||||
|
required: a line that cannot say where it sits is not one this file can hold,
|
||||||
|
and dropping it silently would hand out a seq the file already contains.
|
||||||
|
|
||||||
|
And a variant is **retired, not deleted**: kept deserializable, never
|
||||||
|
constructed, with the date and the reason on it. `Event::TaskNote` is the
|
||||||
|
example, and the phone folds it to no row — which is the point, since an
|
||||||
|
unreadable line correctly draws a placeholder and one per background task is
|
||||||
|
the wall the row was removed for.
|
||||||
|
|
||||||
### A limit a subagent hits is the session's (2026-09-06)
|
### A limit a subagent hits is the session's (2026-09-06)
|
||||||
|
|
||||||
A background Task runs on long after its parent's turn ended, so **the account
|
A background Task runs on long after its parent's turn ended, so **the account
|
||||||
|
|||||||
@@ -108,6 +108,26 @@ sealed class SessionEvent {
|
|||||||
val turnStart: Long? = null,
|
val turnStart: Long? = null,
|
||||||
) : SessionEvent()
|
) : SessionEvent()
|
||||||
|
|
||||||
|
/**
|
||||||
|
* A line in the transcript this build cannot read: a kind a newer server wrote, or one an older
|
||||||
|
* server wrote that has since been dropped.
|
||||||
|
*
|
||||||
|
* [kind] is the word the line called itself, so the row can say what is missing rather than
|
||||||
|
* that something is. The server makes these when reading; no driver sends one.
|
||||||
|
*/
|
||||||
|
data class Unreadable(val kind: String) : SessionEvent()
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Retired on 2026-09-06, hours after it was added: a background task finishing, which turned
|
||||||
|
* out to be a screenful of notices about work nobody was asking after.
|
||||||
|
*
|
||||||
|
* Kept because a transcript is append-only -- the sessions that ran a background task in that
|
||||||
|
* window have these lines for ever. It draws no row, which is the whole reason it is still
|
||||||
|
* named here rather than left to fall through to [Unknown]: that would draw a placeholder per
|
||||||
|
* background task, which is the same wall the row was removed for.
|
||||||
|
*/
|
||||||
|
object RetiredTaskNote : SessionEvent()
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* A command the session was asked to run on itself and cannot run yet. Resolved by
|
* A command the session was asked to run on itself and cannot run yet. Resolved by
|
||||||
* [CommandSent] with the same id; a command that ran straight away has only that one.
|
* [CommandSent] with the same id; a command that ran straight away has only that one.
|
||||||
@@ -252,6 +272,8 @@ fun parseSeqEvent(json: String): SeqEvent {
|
|||||||
body.getString("text"),
|
body.getString("text"),
|
||||||
if (body.has("turnStart")) body.getLong("turnStart") else null,
|
if (body.has("turnStart")) body.getLong("turnStart") else null,
|
||||||
)
|
)
|
||||||
|
"unreadable" -> SessionEvent.Unreadable(body.getString("kind"))
|
||||||
|
"taskNote" -> SessionEvent.RetiredTaskNote
|
||||||
"commandQueued" ->
|
"commandQueued" ->
|
||||||
SessionEvent.CommandQueued(body.getString("id"), body.getString("text"))
|
SessionEvent.CommandQueued(body.getString("id"), body.getString("text"))
|
||||||
"commandSent" -> SessionEvent.CommandSent(body.getString("id"), body.getString("text"))
|
"commandSent" -> SessionEvent.CommandSent(body.getString("id"), body.getString("text"))
|
||||||
|
|||||||
@@ -515,6 +515,12 @@ fun foldEvent(items: List<TranscriptItem>, entry: SeqEvent): List<TranscriptItem
|
|||||||
is SessionEvent.Compacted ->
|
is SessionEvent.Compacted ->
|
||||||
items + TranscriptItem.CompactedNote(entry.seq, event.preTokens, event.postTokens)
|
items + TranscriptItem.CompactedNote(entry.seq, event.preTokens, event.postTokens)
|
||||||
is SessionEvent.Unknown -> items + TranscriptItem.Note(entry.seq, "[${event.type}]")
|
is SessionEvent.Unknown -> items + TranscriptItem.Note(entry.seq, "[${event.type}]")
|
||||||
|
// Said rather than skipped: a line the server could not read is a hole in the conversation,
|
||||||
|
// and one that draws nothing is a hole nothing on screen ever mentions.
|
||||||
|
is SessionEvent.Unreadable ->
|
||||||
|
items + TranscriptItem.Note(entry.seq, "[unreadable: ${event.kind}]")
|
||||||
|
// No row: see [SessionEvent.RetiredTaskNote].
|
||||||
|
is SessionEvent.RetiredTaskNote -> items
|
||||||
// Screen-level state, not transcript rows -- see SessionScreen.
|
// Screen-level state, not transcript rows -- see SessionScreen.
|
||||||
is SessionEvent.UsageDelta -> items
|
is SessionEvent.UsageDelta -> items
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -208,6 +208,48 @@ pub enum Event {
|
|||||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||||
turn_start: Option<u64>,
|
turn_start: Option<u64>,
|
||||||
},
|
},
|
||||||
|
/// **Retired on 2026-09-06, hours after it was added.** Kept only so the
|
||||||
|
/// transcripts written while it existed still read: a session that ran a
|
||||||
|
/// background task in that window has these lines for ever, and a
|
||||||
|
/// transcript is append-only, so there is no pass that could remove them.
|
||||||
|
///
|
||||||
|
/// Never constructed. It reported a background task finishing, and drawing
|
||||||
|
/// a row per one turned out to be a screen of notices about work the
|
||||||
|
/// reader was not asking after -- see PLAN.md's "Two turns must never be
|
||||||
|
/// drawn as one". The phone folds it to no row at all, which is what makes
|
||||||
|
/// keeping it cheap.
|
||||||
|
///
|
||||||
|
/// Deleting the variant instead is what broke every live session, and
|
||||||
|
/// [`Event::Unreadable`] is the reason that cannot happen again. This is
|
||||||
|
/// still here rather than left to that: an unreadable line draws a
|
||||||
|
/// placeholder, correctly, and one per background task is the same wall
|
||||||
|
/// the row was removed for.
|
||||||
|
TaskNote {
|
||||||
|
about: String,
|
||||||
|
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||||
|
title: Option<String>,
|
||||||
|
status: String,
|
||||||
|
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||||
|
summary: Option<String>,
|
||||||
|
},
|
||||||
|
/// A line in a transcript that this build cannot read: a kind a newer
|
||||||
|
/// server wrote, a kind an older one wrote that has since been dropped, or
|
||||||
|
/// a line whose contents do not fit the kind it names.
|
||||||
|
///
|
||||||
|
/// **Only ever made when reading, never sent by a driver**, and it is the
|
||||||
|
/// reason a transcript can outlive a change to this enum. See
|
||||||
|
/// `Indexed::parse_at` for the incident: removing a variant after
|
||||||
|
/// transcripts had recorded it made every read of those files fail, so
|
||||||
|
/// every session in them lost its status, its history and its ability to
|
||||||
|
/// be sent to.
|
||||||
|
///
|
||||||
|
/// It carries the word the line called itself so a reader is told what
|
||||||
|
/// they are missing rather than that something is missing. `kind` is not
|
||||||
|
/// an enum for the obvious reason: the whole point of this variant is the
|
||||||
|
/// words that are not in one.
|
||||||
|
Unreadable {
|
||||||
|
kind: String,
|
||||||
|
},
|
||||||
/// The manager's record of a question being answered, so a rendered
|
/// The manager's record of a question being answered, so a rendered
|
||||||
/// question card resolves on every device rather than only the one that
|
/// question card resolves on every device rather than only the one that
|
||||||
/// answered.
|
/// answered.
|
||||||
|
|||||||
@@ -309,18 +309,70 @@ impl<'a> Indexed<'a> {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn parse(&self, range: Range<usize>) -> Result<Vec<SeqEvent>> {
|
fn parse(&self, range: Range<usize>) -> Result<Vec<SeqEvent>> {
|
||||||
self.lines[range]
|
self.lines[range.clone()]
|
||||||
.iter()
|
.iter()
|
||||||
.map(|at| {
|
.enumerate()
|
||||||
serde_json::from_str(&self.text[at.clone()])
|
.map(|(offset, at)| self.parse_at(range.start + offset, at.clone()))
|
||||||
.with_context(|| format!("bad transcript line in {}", self.path.display()))
|
|
||||||
})
|
|
||||||
.collect()
|
.collect()
|
||||||
}
|
}
|
||||||
|
|
||||||
fn parse_one(&self, index: usize) -> Result<SeqEvent> {
|
fn parse_one(&self, index: usize) -> Result<SeqEvent> {
|
||||||
serde_json::from_str(&self.text[self.lines[index].clone()])
|
self.parse_at(index, self.lines[index].clone())
|
||||||
.with_context(|| format!("bad transcript line in {}", self.path.display()))
|
}
|
||||||
|
|
||||||
|
/// One line, degrading to [`Event::Unreadable`] rather than failing when
|
||||||
|
/// this build cannot make sense of the event on it.
|
||||||
|
///
|
||||||
|
/// **A transcript is append-only and permanent, so the set of kinds that
|
||||||
|
/// can appear in one only ever grows.** What this build writes is not what
|
||||||
|
/// it may have to read: a line may come from a newer server, or from an
|
||||||
|
/// older one that wrote a kind since dropped. Refusing the whole file for
|
||||||
|
/// one such line is what happened on 2026-09-06 -- a kind was removed after
|
||||||
|
/// transcripts had recorded it, every read of those files failed, and the
|
||||||
|
/// sessions in them could not be opened, listed, paged or sent to. One
|
||||||
|
/// unfamiliar word took down every conversation it appeared in.
|
||||||
|
///
|
||||||
|
/// So the line survives as a line. It keeps its seq, which is the part
|
||||||
|
/// everything downstream is addressed by, and says what it was rather than
|
||||||
|
/// pretending to be something -- there is a row for that on the phone
|
||||||
|
/// already.
|
||||||
|
///
|
||||||
|
/// The seq itself is still required, and this still fails without one: a
|
||||||
|
/// line that cannot say where it sits in the sequence is not a line this
|
||||||
|
/// file can hold, and quietly dropping it would hand out a seq the file
|
||||||
|
/// already contains.
|
||||||
|
fn parse_at(&self, index: usize, at: Range<usize>) -> Result<SeqEvent> {
|
||||||
|
let line = &self.text[at];
|
||||||
|
match serde_json::from_str(line) {
|
||||||
|
Ok(entry) => Ok(entry),
|
||||||
|
Err(err) => {
|
||||||
|
#[derive(Deserialize)]
|
||||||
|
struct JustPlace {
|
||||||
|
seq: u64,
|
||||||
|
ts: f64,
|
||||||
|
#[serde(rename = "type")]
|
||||||
|
kind: Option<String>,
|
||||||
|
}
|
||||||
|
let place: JustPlace = serde_json::from_str(line).with_context(|| {
|
||||||
|
format!("bad transcript line in {}", self.path.display())
|
||||||
|
})?;
|
||||||
|
// Debug rather than a warning: a transcript written against a
|
||||||
|
// newer build has one of these per line it wrote, and the row
|
||||||
|
// on the phone is where this is actually reported.
|
||||||
|
tracing::debug!(
|
||||||
|
"transcript line {index} of {} (seq {}) is not one this build can read: {err}",
|
||||||
|
self.path.display(),
|
||||||
|
place.seq,
|
||||||
|
);
|
||||||
|
Ok(SeqEvent {
|
||||||
|
seq: place.seq,
|
||||||
|
ts: place.ts,
|
||||||
|
event: Event::Unreadable {
|
||||||
|
kind: place.kind.unwrap_or_else(|| "no kind".to_string()),
|
||||||
|
},
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The newest `limit` *rows* ending at line `end`, with each run of
|
/// The newest `limit` *rows* ending at line `end`, with each run of
|
||||||
@@ -396,6 +448,51 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// The failure that took every live session down on 2026-09-06: an event
|
||||||
|
/// kind was removed from the enum after transcripts had already recorded
|
||||||
|
/// it, so every read of those files failed and the sessions in them could
|
||||||
|
/// not be opened, listed, paged or sent to.
|
||||||
|
///
|
||||||
|
/// A transcript is append-only and permanent, so **the set of kinds that
|
||||||
|
/// can appear in one only ever grows** -- what this build writes is not
|
||||||
|
/// what it may have to read. A line it cannot make sense of has to be a
|
||||||
|
/// line, not the end of the file.
|
||||||
|
#[test]
|
||||||
|
fn a_line_of_a_kind_this_build_does_not_know_does_not_break_the_file() {
|
||||||
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
|
let path = dir.path().join("transcript.jsonl");
|
||||||
|
std::fs::write(
|
||||||
|
&path,
|
||||||
|
concat!(
|
||||||
|
r#"{"seq":1,"ts":1.0,"type":"status","state":"running"}"#,
|
||||||
|
"\n",
|
||||||
|
r#"{"seq":2,"ts":2.0,"type":"assistantText","delta":"hi"}"#,
|
||||||
|
"\n",
|
||||||
|
r#"{"seq":3,"ts":3.0,"type":"taskNote","about":"t","title":"h","status":"completed"}"#,
|
||||||
|
"\n",
|
||||||
|
r#"{"seq":4,"ts":4.0,"type":"somethingFromTheFuture","whatever":[1,2]}"#,
|
||||||
|
"\n",
|
||||||
|
r#"{"seq":5,"ts":5.0,"type":"status","state":"idle"}"#,
|
||||||
|
"\n",
|
||||||
|
),
|
||||||
|
)
|
||||||
|
.expect("write");
|
||||||
|
|
||||||
|
let entries = read_after(&path, 0).expect("a strange line is not a broken file");
|
||||||
|
assert_eq!(entries.len(), 5, "every line is still a line: {entries:?}");
|
||||||
|
assert_eq!(entries[4].seq, 5);
|
||||||
|
|
||||||
|
let transcript = Transcript::open(&path).expect("open");
|
||||||
|
assert_eq!(transcript.last_status(), Some(SessionStatus::Idle));
|
||||||
|
// The next seq is counted from the newest *line*, whatever kind it is.
|
||||||
|
// Skipping the ones this build cannot read would hand out a seq the
|
||||||
|
// file already contains.
|
||||||
|
assert_eq!(transcript.next_seq, 6);
|
||||||
|
|
||||||
|
let window = read_window(&path, None, None, 80, false).expect("window");
|
||||||
|
assert_eq!(window.len(), 5, "{window:?}");
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn assigns_increasing_seqs_and_replays_after_a_cursor() {
|
fn assigns_increasing_seqs_and_replays_after_a_cursor() {
|
||||||
let dir = tempfile::tempdir().expect("tempdir");
|
let dir = tempfile::tempdir().expect("tempdir");
|
||||||
|
|||||||
Reference in new issue
Block a user