feat: persistent session attention tracking across server restarts

[dimakis]

Summary

Adds persistent session attention tracking to the "What's Next" feed to show sessions awaiting user reply, sessions with uncommitted work, and sessions waiting for input even after server restarts.

• Sessions awaiting user reply (assistant spoke last) persist across server restarts
• Sessions with uncommitted work show in attention feed with idle time
• All session-based attention items are now tier 1 (matching priority requirement)

Changes

Protocol (packages/protocol)

• Add lastSpeaker/lastSpeakerAt/closedBy to SessionMeta
• Add uncommittedWork/idleMinutes to SessionActivity
• Add EventStore.updateLastSpeaker() to record speaker on each turn
• Add EventStore.getAttentionSessions() to query sessions awaiting reply
• Add migration for last_speaker and last_speaker_at columns

Server (server/)

• chat.ts: Call updateLastSpeaker('user') on all user_message events (resume, send, interrupt)
• index.ts: Call updateLastSpeaker('assistant') on turn_end events
• session-overview.ts: Merge persistent attention sessions from EventStore into live activities
• worktree.ts: Export hasUncommittedWork() for attention tracking
• Add uncommitted work cache (5-min TTL) to avoid expensive git calls
• Update test mocks to include eventStore and mock hasUncommittedWork

Frontend (frontend/src/hooks)

• useAttentionFeed.ts: Update sessionsToAttention() to handle new fields:
  • Tier 1: done + uncommittedWork → red, "uncommitted work" + idle time
  • Tier 1: done without uncommittedWork → amber, "awaiting your reply" + idle time
  • Tier 1: waiting → red, "permission/review needed" (unchanged)

Test Plan

• Protocol builds: npx tsc -b packages/protocol
• Server builds: npx tsc -p tsconfig.build.json
• Tests pass: npm test -- server/__tests__/session-overview packages/protocol/__tests__/event-store
• Manual verification: start Mitzo server, create session, send message, restart server, verify attention feed shows session

🤖 Generated with Claude Code

[dimakis]

Centaur Review

Found 6 issue(s) (1 critical) (3 warning).

packages/protocol/src/event-store.ts

The getAttentionSessions query filters is_active = 1 but properly-completed sessions have is_active = 0, meaning persistent attention only works for crash-orphaned sessions, not normal operation. The new features lack test coverage despite the project's TDD requirement.

• 🔴 bugs (L157): getAttentionSessions filters WHERE is_active = 1, but markSessionInactive() in query-loop.ts:1348 sets is_active = 0 when the query loop finishes. This means properly-completed sessions where the assistant spoke last will have is_active = 0 and won't appear in attention results. The query only catches sessions orphaned by server crashes (where markSessionInactive never ran). For normal operation, after a session ends and the user hasn't replied, the session is invisible in the persistent attention feed. Consider using is_active = 0 or removing the is_active filter entirely (hidden sessions are already excluded). [fixable]
• 🟡 missing_tests (L508): No tests for updateLastSpeaker() or getAttentionSessions(). These are new public methods on EventStore with specific query semantics (speaker type validation, ordering, limit, active/hidden filters) that should be verified. Existing event-store.test.ts has patterns for testing similar methods (markSessionInactive, recordUsage). [fixable]
• 🔵 style (L225): migrateCloseTracking and migrateAttentionTracking each call PRAGMA table_info('sessions') independently despite running back-to-back. The existing migrations (migratePromptTracking, migrateUsageTracking, migrateWorktreeTracking) follow the same pattern, so this is consistent with the codebase — but the redundant PRAGMA calls could be consolidated if more columns are expected. [fixable]

server/__tests__/session-overview.test.ts

The getAttentionSessions query filters is_active = 1 but properly-completed sessions have is_active = 0, meaning persistent attention only works for crash-orphaned sessions, not normal operation. The new features lack test coverage despite the project's TDD requirement.

• 🟡 missing_tests: No tests added for the new features despite the project's TDD requirement. Missing coverage for: persistentToActivity() (persistent session → activity conversion), checkUncommittedCached() (TTL caching of git status), idleMinutes calculation in deriveActivity(), uncommittedWork flag propagation, and the merge logic in compute() that deduplicates live vs persistent sessions. Only mocks were added to prevent existing tests from breaking. [fixable]

server/session-overview.ts

The getAttentionSessions query filters is_active = 1 but properly-completed sessions have is_active = 0, meaning persistent attention only works for crash-orphaned sessions, not normal operation. The new features lack test coverage despite the project's TDD requirement.

• 🟡 unsafe_assumptions (L247): eventStore.getSession(sessionId) is a synchronous SQLite query called for every live session on every broadcast cycle (every 200ms coalesce window). This is uncached, unlike the hasUncommittedWork calls which have a 5-minute TTL cache. For a small number of sessions this is fine, but it adds per-broadcast latency that scales with session count. Consider caching lastSpeakerAt alongside lastEventTimes or batching the lookups. [fixable]
• 🔵 unsafe_assumptions (L303): hasUncommittedWork uses execFileSync which blocks the Node.js event loop. The 5-minute TTL cache mitigates this, but every cache miss (first check per cwd, plus periodic refreshes) will block. For persistent sessions with stale worktree paths that no longer exist, the git command fails synchronously before the catch handles it. Consider an async alternative or longer TTL for persistent (non-live) sessions. [fixable]

[dimakis]

🔴 bugs: getAttentionSessions filters WHERE is_active = 1, but markSessionInactive() in query-loop.ts:1348 sets is_active = 0 when the query loop finishes. This means properly-completed sessions where the assistant spoke last will have is_active = 0 and won't appear in attention results. The query only catches sessions orphaned by server crashes (where markSessionInactive never ran). For normal operation, after a session ends and the user hasn't replied, the session is invisible in the persistent attention feed. Consider using is_active = 0 or removing the is_active filter entirely (hidden sessions are already excluded). [fixable]

[dimakis]

🟡 missing_tests: No tests for updateLastSpeaker() or getAttentionSessions(). These are new public methods on EventStore with specific query semantics (speaker type validation, ordering, limit, active/hidden filters) that should be verified. Existing event-store.test.ts has patterns for testing similar methods (markSessionInactive, recordUsage). [fixable]

[dimakis]

🔵 style: migrateCloseTracking and migrateAttentionTracking each call PRAGMA table_info('sessions') independently despite running back-to-back. The existing migrations (migratePromptTracking, migrateUsageTracking, migrateWorktreeTracking) follow the same pattern, so this is consistent with the codebase — but the redundant PRAGMA calls could be consolidated if more columns are expected. [fixable]

[dimakis]

🟡 unsafe_assumptions: eventStore.getSession(sessionId) is a synchronous SQLite query called for every live session on every broadcast cycle (every 200ms coalesce window). This is uncached, unlike the hasUncommittedWork calls which have a 5-minute TTL cache. For a small number of sessions this is fine, but it adds per-broadcast latency that scales with session count. Consider caching lastSpeakerAt alongside lastEventTimes or batching the lookups. [fixable]

[dimakis]

🔵 unsafe_assumptions: hasUncommittedWork uses execFileSync which blocks the Node.js event loop. The 5-minute TTL cache mitigates this, but every cache miss (first check per cwd, plus periodic refreshes) will block. For persistent sessions with stale worktree paths that no longer exist, the git command fails synchronously before the catch handles it. Consider an async alternative or longer TTL for persistent (non-live) sessions. [fixable]

[dimakis]

Centaur Review

Found 6 issue(s) (3 warning).

server/session-overview.ts

Solid feature with good persistence layer tests and caching strategy. Main concerns: execFileSync on the broadcast hot path can block the event loop (up to 30s per cache miss), closedBy is dead code, and the new sessionsToAttention frontend logic lacks test coverage.

• 🟡 unsafe_assumptions (L307): hasUncommittedWork() uses execFileSync with a 30-second timeout (WORKTREE_GIT_TIMEOUT_MS), and is called synchronously inside compute() which runs on the main event loop during every broadcast() and getSnapshot(). When the TTL cache expires, this blocks the Node.js event loop for potentially up to 30s per worktree path (multiplied by number of sessions). Consider using an async check or a shorter dedicated timeout for the broadcast path. [fixable]
• 🔵 bugs (L251): idleMinutes can be negative if speakerAt is slightly in the future (e.g., due to Date.now() being called before updateLastSpeakerAt writes a value that uses a later Date.now()). Math.round((now - speakerAt) / 60_000) would yield a small negative number. The frontend guards with a.idleMinutes > 0 before displaying, so no visible bug, but Math.max(0, ...) would be more defensive. [fixable]

server/chat.ts

Solid feature with good persistence layer tests and caching strategy. Main concerns: execFileSync on the broadcast hot path can block the event loop (up to 30s per cache miss), closedBy is dead code, and the new sessionsToAttention frontend logic lacks test coverage.

• 🟡 regressions (L852): updateLastSpeaker('user') is called for resumed sessions (line 852), sendToChat (line 973), and interruptChat (line 1007), but NOT for the initial user prompt of new sessions in _startChatInner. After the assistant's first turn, turn_end sets last_speaker='assistant', which is correct. However, the asymmetry means overviewEmitter.updateLastSpeakerAt() is also never called for new session user messages, so idleMinutes for the first turn of new sessions will fall back to lastEventAt rather than reflecting the user's message time. [fixable]
• 🔵 missing_tests (L852): The three updateLastSpeaker('user') calls in chat.ts (resume path, sendToChat, interruptChat) have no direct test coverage. The event-store tests cover the method itself, but no integration test verifies that user messages actually trigger the call. [fixable]

packages/protocol/src/event-store.ts

Solid feature with good persistence layer tests and caching strategy. Main concerns: execFileSync on the broadcast hot path can block the event loop (up to 30s per cache miss), closedBy is dead code, and the new sessionsToAttention frontend logic lacks test coverage.

• 🔵 style (L221): closedBy column and SessionClosedBy type are added to the schema, migration, insert, update, and rowToSession mapping, but no code anywhere in the codebase actually sets closedBy to a non-null value. This is dead code that adds migration and schema overhead. Consider deferring this column to a PR that actually uses it, or at minimum note in the PR description that this is intentional scaffolding. [fixable]

frontend/src/hooks/useAttentionFeed.ts

Solid feature with good persistence layer tests and caching strategy. Main concerns: execFileSync on the broadcast hot path can block the event loop (up to 30s per cache miss), closedBy is dead code, and the new sessionsToAttention frontend logic lacks test coverage.

• 🟡 missing_tests (L135): The sessionsToAttention function was substantially rewritten (from a simple .filter().map() to a loop with three branches: waiting, done+uncommittedWork, done+awaitingReply), but the existing useAttentionFeed.test.ts has zero tests for session-derived attention items. The idle label formatting logic (minutes vs hours) and the uncommittedWork branching are untested. [fixable]

[dimakis]

🟡 unsafe_assumptions: hasUncommittedWork() uses execFileSync with a 30-second timeout (WORKTREE_GIT_TIMEOUT_MS), and is called synchronously inside compute() which runs on the main event loop during every broadcast() and getSnapshot(). When the TTL cache expires, this blocks the Node.js event loop for potentially up to 30s per worktree path (multiplied by number of sessions). Consider using an async check or a shorter dedicated timeout for the broadcast path. [fixable]

[dimakis]

🔵 bugs: idleMinutes can be negative if speakerAt is slightly in the future (e.g., due to Date.now() being called before updateLastSpeakerAt writes a value that uses a later Date.now()). Math.round((now - speakerAt) / 60_000) would yield a small negative number. The frontend guards with a.idleMinutes > 0 before displaying, so no visible bug, but Math.max(0, ...) would be more defensive. [fixable]

[dimakis]

🟡 regressions: updateLastSpeaker('user') is called for resumed sessions (line 852), sendToChat (line 973), and interruptChat (line 1007), but NOT for the initial user prompt of new sessions in _startChatInner. After the assistant's first turn, turn_end sets last_speaker='assistant', which is correct. However, the asymmetry means overviewEmitter.updateLastSpeakerAt() is also never called for new session user messages, so idleMinutes for the first turn of new sessions will fall back to lastEventAt rather than reflecting the user's message time. [fixable]

[dimakis]

🔵 missing_tests: The three updateLastSpeaker('user') calls in chat.ts (resume path, sendToChat, interruptChat) have no direct test coverage. The event-store tests cover the method itself, but no integration test verifies that user messages actually trigger the call. [fixable]

[dimakis]

🔵 style: closedBy column and SessionClosedBy type are added to the schema, migration, insert, update, and rowToSession mapping, but no code anywhere in the codebase actually sets closedBy to a non-null value. This is dead code that adds migration and schema overhead. Consider deferring this column to a PR that actually uses it, or at minimum note in the PR description that this is intentional scaffolding. [fixable]

[dimakis]

🟡 missing_tests: The sessionsToAttention function was substantially rewritten (from a simple .filter().map() to a loop with three branches: waiting, done+uncommittedWork, done+awaitingReply), but the existing useAttentionFeed.test.ts has zero tests for session-derived attention items. The idle label formatting logic (minutes vs hours) and the uncommittedWork branching are untested. [fixable]