From 0fa5c6ad8a3d4dc2b6fc8f8ff7eef80f7b7b6de2 Mon Sep 17 00:00:00 2001 From: Samuel Newman Date: Mon, 15 Jun 2026 12:39:04 +0300 Subject: [PATCH] enforce convo-present invariant in snapshot instead of guarding in isConvoActive the active convo states declare `convo` as non-optional, but a lifecycle event (background/suspend) can move the machine into an active status before setup() populates this.convo. generateSnapshot papered over this with `this.convo!`, so isConvoActive trusted the status alone and downstream consumers crashed dereferencing an undefined convo. instead of guarding at the isConvoActive call site, make the invalid state unrepresentable: generateSnapshot now reports Initializing whenever an active status has no convo yet, which lets us drop all four non-null assertions. the compiler now enforces that an active snapshot always carries a convo, so the isConvoActive guard added earlier is reverted as redundant. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/state/messages/convo/agent.ts | 53 +++++++++++++++++++++---------- src/state/messages/convo/util.ts | 15 +++------ 2 files changed, 41 insertions(+), 27 deletions(-) diff --git a/src/state/messages/convo/agent.ts b/src/state/messages/convo/agent.ts index f294216b1d..3603026d78 100644 --- a/src/state/messages/convo/agent.ts +++ b/src/state/messages/convo/agent.ts @@ -237,22 +237,40 @@ export class Convo { removeReaction: undefined, } + /* + * Captured as a local so the `if (convo)` narrowing below survives the + * `this.getItems()` call - TS discards narrowing on mutable `this` members + * after a method call, but not on a const. + */ + const convo = this.convo + + /* + * A lifecycle event (e.g. `Background` or `Suspend`) can move us into an + * active status before `setup()` has resolved and populated `convo`. The + * active states declare `convo` as non-optional, so we can't build one + * without it - fall back to reporting `Initializing` until the convo lands. + * This keeps the snapshot's type and runtime in agreement, so consumers can + * trust that an active status always has a `convo`. + */ + const stillInitializing = (): ConvoState => ({ + status: ConvoStatus.Initializing, + items: [], + convo, + error: undefined, + ...shared, + ...emptyMethods, + }) + switch (this.status) { case ConvoStatus.Initializing: { - return { - status: ConvoStatus.Initializing, - items: [], - convo: this.convo, - error: undefined, - ...shared, - ...emptyMethods, - } + return stillInitializing() } case ConvoStatus.Disabled: { + if (!convo) return stillInitializing() return { - status: this.status, + status: ConvoStatus.Disabled, items: this.getItems(), - convo: this.convo!, + convo, relatedProfiles: this.relatedProfiles, error: undefined, ...shared, @@ -260,10 +278,11 @@ export class Convo { } } case ConvoStatus.Suspended: { + if (!convo) return stillInitializing() return { - status: this.status, + status: ConvoStatus.Suspended, items: this.getItems(), - convo: this.convo!, + convo, relatedProfiles: this.relatedProfiles, error: undefined, ...shared, @@ -271,10 +290,11 @@ export class Convo { } } case ConvoStatus.Backgrounded: { + if (!convo) return stillInitializing() return { - status: this.status, + status: ConvoStatus.Backgrounded, items: this.getItems(), - convo: this.convo!, + convo, relatedProfiles: this.relatedProfiles, error: undefined, ...shared, @@ -282,10 +302,11 @@ export class Convo { } } case ConvoStatus.Ready: { + if (!convo) return stillInitializing() return { - status: this.status, + status: ConvoStatus.Ready, items: this.getItems(), - convo: this.convo!, + convo, relatedProfiles: this.relatedProfiles, error: undefined, ...shared, diff --git a/src/state/messages/convo/util.ts b/src/state/messages/convo/util.ts index 8a2d0b71ef..5301d10bbb 100644 --- a/src/state/messages/convo/util.ts +++ b/src/state/messages/convo/util.ts @@ -21,19 +21,12 @@ export type ActiveConvoStates = * Checks if a `Convo` has a `status` that is "active", meaning the chat is * loaded and ready to be used, or its in a suspended or background state, and * ready for resumption. - * - * The `convo` object must also be present. The status can transition into an - * active state before the convo has finished loading (e.g. `Initializing` - * receives a `Background` event before `setup()` resolves), and every - * `ActiveConvoStates` member declares `convo` as non-optional, so we guard - * against that race here rather than crashing downstream consumers. */ export function isConvoActive(convo: ConvoState): convo is ActiveConvoStates { return ( - convo.convo !== undefined && - (convo.status === ConvoStatus.Ready || - convo.status === ConvoStatus.Backgrounded || - convo.status === ConvoStatus.Suspended || - convo.status === ConvoStatus.Disabled) + convo.status === ConvoStatus.Ready || + convo.status === ConvoStatus.Backgrounded || + convo.status === ConvoStatus.Suspended || + convo.status === ConvoStatus.Disabled ) }