Greptile Summary

This PR fixes a stale streaming footer status in the TUI that persisted after a run's active entry was cleared and no tracked session runs remained. The core change relaxes the guard in clearStaleStreamingIfNoTrackedRunRemains so a null activeChatRunId no longer prevents the cleanup from running, and routes the local /btw empty-final path through that same helper. The terminateRun fallback is intentionally removed so background /btw abort/error events cannot overwrite global activity status. Logic looks correct and the new tests cover the key scenarios.

Confidence Score: 4/5

Safe to merge; only P2 style/test clarity suggestions, no logic defects found.

The guard rewrite is logically sound, concurrent-run protections are preserved, and the three new tests cover the primary recovery paths. Two P2 observations: the ternary in the guard deserves a clarifying comment to prevent well-intentioned simplifications that would regress the fix, and one test pre-condition is achieved through direct state mutation rather than organic event flow, weakening its regression value slightly.

No files require special attention beyond the inline notes.

This is a comment left during a code review.
Path: src/tui/tui-event-handlers.ts
Line: 199-207

Comment:
**Subtle guard — null vs. absent distinction**

The rewritten guard is logically correct but the ternary `(activeRunId ? sessionRuns.has(activeRunId) : false)` is easy to misread at first glance. When `activeRunId` is `null` the sub-expression evaluates to `false`, which means the guard does **not** bail, allowing the stale-streaming clear to proceed. This is exactly the intended fix — but the semantics are the opposite of what the surrounding `||` chain might suggest to a reader expecting "any truthy sub-expression = bail".

A small inline comment would prevent future readers from accidentally "simplifying" the ternary back to `sessionRuns.has(activeRunId!)` or `!!activeRunId && sessionRuns.has(activeRunId)` (both of which would break the null case).

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: src/tui/tui-event-handlers.test.ts
Line: 605-619

Comment:
**Direct state mutation simulates an impossible in-production sequence**

Between the two `handleChatEvent` calls the test manually resets `state.activityStatus = "streaming"` and `state.activeChatRunId = null`. In practice the first "final" event would have already moved `activityStatus` to `"idle"` via `finalizeRun`, so the second event would hit the `activityStatus !== "streaming"` guard and be a no-op regardless of the fix. The test therefore validates a state that real event flow cannot produce.

If the intent is to prove the helper clears stale streaming when a duplicate final arrives while the status happens to be `"streaming"`, consider documenting why that state is reachable (e.g., an interleaved delta from another run armed streaming after the first final), or add a delta from a second run before the second final to make the pre-condition organic.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (1): Last reviewed commit: "fix(clownfish): address review for ghcra..." | Re-trigger Greptile}

Codex review: needs maintainer review before merge.

What this changes:

The PR updates TUI event handling to clear stale streaming status after unbound final and local /btw empty-final paths, adds focused regression tests, removes the inactive terminate fallback cleanup, and adds a changelog entry.

Maintainer follow-up before merge:

Leave this PR open for maintainer review. If accepted, land the narrow TUI handler recovery and regression tests for null-active-run duplicate finals and local /btw empty finals, while keeping the broader content-completion timing issues #67052/#67053 and the reconnect watchdog work in #74224 separate.

Best possible solution:

Leave this PR open for maintainer review. If accepted, land the narrow TUI handler recovery and regression tests for null-active-run duplicate finals and local /btw empty finals, while keeping the broader content-completion timing issues #67052/#67053 and the reconnect watchdog work in #74224 separate.

What I checked:

• Protected maintainer handling: GitHub PR metadata shows author_association: MEMBER, labels include maintainer, and the PR is open/unmerged at head 0a346473e3a27f1e92926ad10ab24b798b430761; cleanup policy requires explicit maintainer handling for this PR. (0a346473e3a2)
• Current main still rejects null active-run recovery: On current main, clearStaleStreamingRunIfNoTrackedRunRemains returns early when activeChatRunId is null, which is the central recovery case this PR targets. (src/tui/tui-event-handlers.ts:199, e6cd90e3fd9c)
• Duplicate final path still skips cleanup: When a run is already in finalizedRuns, current main returns on a duplicate final event without invoking stale-streaming cleanup, so a later stale streaming state can persist. (src/tui/tui-event-handlers.ts:322, e6cd90e3fd9c)
• Local /btw empty-final path still skips idle recovery: The local /btw empty-final branch currently forgets and finalizes the run, requests render, and returns without setting activityStatus to idle or calling the stale-streaming helper. (src/tui/tui-event-handlers.ts:355, e6cd90e3fd9c)
• Existing tests are adjacent but not exact: Current tests cover keeping a local /btw result visible and a non-null stale active-run orphan final, but they do not assert null-active-run duplicate-final or local /btw empty-final stale-status recovery. (src/tui/tui-event-handlers.test.ts:280, e6cd90e3fd9c)
• PR diff and security pass: The PR diff is limited to src/tui/tui-event-handlers.ts, src/tui/tui-event-handlers.test.ts, and CHANGELOG.md; it does not touch workflows, dependencies, lockfiles, package scripts, secret handling, artifact downloads, or publishing metadata. (src/tui/tui-event-handlers.ts:199, 0a346473e3a2)

Likely related people:

• vincentkoc: Path history for src/tui/tui-event-handlers.ts shows d7e67b455a74 added the current stale-streaming helper and tests for orphaned finals in the same handler area; that current-main behavior is what this PR narrows further. (role: recent maintainer and current-main stale-streaming owner; confidence: high; commits: d7e67b455a74; files: src/tui/tui-event-handlers.ts, src/tui/tui-event-handlers.test.ts, CHANGELOG.md)
• xantorres: Merged fix(stability): session skills snapshot, tool-loop guard, TUI watchdog, LM Studio preload backoff #67401 added the TUI streaming watchdog and tests in the same run-state/status path, including watchdog reset behavior that interacts with stale streaming recovery. (role: adjacent streaming watchdog contributor; confidence: medium; commits: f44ab20d4db5, 352527393079; files: src/tui/tui-event-handlers.ts, src/tui/tui-event-handlers.test.ts, CHANGELOG.md)
• joelnishanth: Commit cc8ed8d25be0 added the local-run empty-final/concurrent-run regression that this PR must preserve while adding /btw stale-status recovery. (role: adjacent local empty-final behavior contributor; confidence: medium; commits: cc8ed8d25be0; files: src/tui/tui-event-handlers.ts, src/tui/tui-event-handlers.test.ts)
• ngutman: Path history shows Add /btw side questions #45444 introduced /btw side questions and delivery semantics; this PR specifically routes a local /btw terminal path through the stale-streaming helper. (role: adjacent /btw feature introducer; confidence: low; commits: 9aac55d30614; files: src/tui/tui-event-handlers.ts, src/tui/tui-event-handlers.test.ts, docs/tools/btw.md)

Remaining risk / open question:

• Auto-closing would bypass the MEMBER author plus protected maintainer label policy and leave the exact current-main recovery path unmerged.
• The TUI run-state machine is subtle around inactive /btw terminal events; maintainer review should preserve the PR's narrow final-event recovery while avoiding broad abort/error fallback updates.
• This read-only review did not run tests; the PR body reports pnpm -s vitest run src/tui/tui-event-handlers.test.ts and pnpm check:changed, but maintainer merge review should rely on current CI or rerun the targeted TUI test lane.

Codex review notes: model gpt-5.5, reasoning high; reviewed against e6cd90e3fd9c.