Fix blank-session restarts after interrupt or failed run

[bunnysayzz]

Summary

Re-opens #1135 (auto-closed by force-push history rewrite).

Fixes #1054 (chat restarts blank after Esc interrupt / failed run).

Root cause: previousRunStateRef was only synced when client.run() settled, but Esc releases the input lock inside the abort listener, so a follow-up message could be built from stale/null ref, causing the SDK to start a blank session.

Fix

1. onAbort checkpoint invoked before input lock release (so the latest SDK state is saved before a follow-up can run)
2. Per-run generation guard (runGenerationRef/runIsCurrent) prevents a superseded run from clobbering newer state
3. Catch-path ref sync ensures errors also persist state
4. loadMostRecentChatState refuses sessionState-less run states (disk poison)

Tests

Hook-level regression tests through the real createRunConfig/client.run wiring via DI seam (setClientFactoryOverrideForTesting). 92 tests green, typecheck/prettier/build clean.

Review response

Codebuff-team asked to audit loadMostRecentChatState callers for non-null assumptions. Audit: exactly 1 production caller (use-send-message.ts:177), already guarded by if (loadedState), ref typed RunState | null. Safe. Full details in review comment on #1135.

Supersedes #1135 (closed by force-push).

[bunnysayzz]

Closing as duplicate of #1181 (Utkarsh-X opened earlier with the same fix, rebased from #1055). Our fix is the same root cause; theirs was first and already had maintainer review.

[bunnysayzz]

Note for comparison with #1181: our tests already use the DI seam (setClientFactoryOverrideForTesting) to drive the real createRunConfig/client.run wiring through the hook, exactly as codebuff-team requested on #1181. The test suite exercises the four abort/error scenarios through the actual hook code, not hand-rolled copies of the logic.

[bunnysayzz]

@codebuff-team This is the re-open of #1135 (auto-closed by the force-push history rewrite). You had reviewed it positively on Aug 29, calling it "exactly the kind of bug fix I'd want ported" and asking to audit loadMostRecentChatState callers for non-null assumptions. That audit was done and replied to on #1135.

Quick note comparing with #1181: this PR's test suite already uses the DI seam (setClientFactoryOverrideForTesting) to drive the real createRunConfig/client.run wiring through the hook, testing the four abort/error scenarios through the actual hook code rather than reimplementing the logic in the spec file. That's the approach you requested on #1181.

Branch is rebased on the latest main. Ready for another look whenever you get a chance.

[codebuff-team]

Good bug report and a plausible root cause: the abort listener released the input lock before previousRunStateRef was synced, so a message sent right after Esc could inherit stale/null state. The fix (checkpoint in onAbort before lock release, per-run generation guard via runGenerationRef/runIsCurrent, catch-path sync, and rejecting sessionState-less run states in loadMostRecentChatState) directly targets that race rather than papering over symptoms.

Strengths:

• cli/src/hooks/use-send-message.ts: generation guard is applied consistently across the abort/error/finally paths, and runIsCurrent() correctly composes chat-switch and generation checks instead of duplicating logic.
• run-state-storage.ts: changing SavedChatState.runState to RunState | null and refusing to synthesize a fake RunState when nothing is restorable is the right instinct — the old fallback (Previous run state could not be restored.) was itself the kind of poisoned state this PR is fixing. You audited the single production caller in use-send-message.ts:177 and confirmed it's already null-guarded — that's the right due diligence for a type-widening change.
• Tests exercise the actual timing invariant (onAbort sees streamStatus === 'streaming' before the lock flips to idle), not just end states, and cover the throwing-onAbort and superseded-run cases.

Concerns for the maintainer's review pass, not blockers:

• This is a meaningful concurrency change to a hot path; worth double-checking that runGeneration incrementing after the run-start guard (per the comment) can't let a requeued message slip in under the wrong generation.
• The else if/else branch renaming in the catch block (aborted vs superseded logging) is a nice touch but slightly obscures the diff; a smaller commit split (guard logic vs. logging) would ease review.

Overall this reads as a genuine fix worth porting, with tests that would catch a regression if the abort ordering broke again.