fix: four correctness and code-quality bugs from deep review

[dylanneve1]

Summary

Deep review of the full codebase surfaced four verified bugs. Each fix is small and targeted.

---

Bug 1 — fastestResponseMs permanently broken after process restart (sessions.ts)

Severity: High

SessionUsage.fastestResponseMs is initialized to Infinity. JSON.stringify serialises Infinity as null, so every persisted session file on disk contains "fastestResponseMs": null. On reload, the migration guard:

if (
  session.usage.fastestResponseMs === undefined ||
  session.usage.fastestResponseMs === 0
)
  session.usage.fastestResponseMs = Infinity;

…does not catch null (null !== undefined, null !== 0). The field stays null. All subsequent comparisons like turn.durationMs < null are always false (null coerces to 0, all durations are positive), so the fastest-response tracker never updates after a restart.

Fix: added || session.usage.fastestResponseMs === null to the migration guard.

---

Bug 2 — Duplicate normalizeForDedupe / isDuplicateOfDelivered in stream.ts (stream.ts)

Severity: Medium

stream.ts contained full copies of both dedup helpers that already live in shared/delivered-text.ts. The copy in stream.ts had a subtle signature difference (string[] instead of readonly string[] for deliveredNorms), meaning a fix to the shared version wouldn't automatically fix the copy.

Fix: removed the duplicate implementations and replaced them with a re-export from the shared module. Consumers that import from stream.ts (including end-turn.test.ts) continue to work unchanged, and there is now a single source of truth.

---

Bug 3 — close() leaves a pending waitForInput promise hanging forever (terminal/input.ts)

Severity: Low

waitForInput() stores a pendingResolve callback that is normally cleared when the user presses Enter or Escape. close() shuts down stdin but never calls or clears pendingResolve, so any caller awaiting waitForInput() at the time of shutdown is leaked — the promise never settles.

Fix: close() now resolves the pending promise with an empty string (matching the Escape-cancel behaviour) before tearing down stdin.

---

Bug 4 — onToolUse callback errors silently discarded in handler.ts (handler.ts)

Severity: Low

The onToolUse callback is marked non-fatal (correct — a broken observer must not abort the turn), but the catch block was empty, making callback exceptions completely invisible. Debugging a misbehaving onToolUse hook was impossible.

Fix: the catch block now logs at warn level with the tool name and error message.

---

Test plan

• Full unit-test suite passes (npm test): 2365 tests pass, 0 regressions introduced
• The 2 pre-existing failures are integration tests that require the Claude Code native binary and are unrelated to these changes
• end-turn.test.ts (which imports normalizeForDedupe/isDuplicateOfDelivered from stream.js) continues to pass after the re-export refactor

---

Generated by Claude Code

[claudiusthebot]

Heartbeat #305 pushed a format fix (commit a2ebd1d).

The Bug 4 logWarn(...) call in handler.ts was a single long line that exceeded Prettier's print-width — wrapped it to 3 lines. No logic change.

All other checks (Tests, Backend Live, Functional, Integration, Fuzz, etc.) already passed. CI Status should go green once the new run completes.

[dylanneve1]

Superseded by #246. I reviewed the old changes and ported the valid/current fixes onto current main in the consolidated PR, while leaving out stale removed-backend changes and broad churn that no longer applies.