Skip to content

Fix session list timestamp reset and missing new sessions#326

Open
dimakis wants to merge 2 commits into
mainfrom
fix/session-list-timestamp-and-filter
Open

Fix session list timestamp reset and missing new sessions#326
dimakis wants to merge 2 commits into
mainfrom
fix/session-list-timestamp-and-filter

Conversation

@dimakis
Copy link
Copy Markdown
Owner

@dimakis dimakis commented May 16, 2026

Summary

Fixes two bugs in the session list:

  1. All sessions show same timestamp after server restart — when sessions are resumed (e.g., after a server restart), their updated_at timestamps were being reset to "now" instead of being preserved. This caused all recently-resumed sessions to show identical relative times like "18m ago".

  2. Newly created sessions disappear from list — sessions with zero turns and zero prompts that aren't currently active were being filtered out, hiding newly created sessions that hadn't completed their first turn yet.

Changes

server/chat.ts:687-697 — Resume timestamp preservation

  • Read existing session metadata via eventStore.getSession() before calling upsertSession() on resume
  • Pass existing updatedAt explicitly to prevent EventStore from defaulting to current time
  • Only applies when session already exists (resume path); new sessions get natural timestamp

server/chat.ts:1354-1358 — Grace period for new sessions

  • Keep zero-turn, zero-prompt, inactive sessions visible for 1 hour after creation
  • After 1 hour with no activity, apply the filter as before (hides automated/discovered sessions)
  • Handles race conditions where session is created but query loop hasn't finished first turn

Test Plan

  • Verify session list shows correct relative timestamps (not all identical)
  • Verify newly created sessions appear in list immediately
  • Verify sessions created >1h ago with no turns are still filtered out
  • Restart server and confirm session timestamps don't reset

🤖 Generated with Claude Code

…-turn sessions

Two bugs in session list rendering:

1. **Timestamp reset on resume**: When sessions were resumed (e.g. on
   server restart), upsertSession() was called without updatedAt, causing
   the EventStore to default to "now". This made all recently-resumed
   sessions show identical timestamps. Fix: read existing updatedAt via
   getSession() and pass it through explicitly.

2. **Missing new sessions**: Zero-turn, zero-prompt, inactive sessions
   were filtered out, hiding newly created sessions that hadn't completed
   their first turn yet. Fix: keep sessions visible for 1 hour after
   creation, then apply the filter.

Fixes session list showing all sessions with same "18m ago" timestamp
and newly created sessions disappearing before first message completes.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
Copy link
Copy Markdown
Owner Author

@dimakis dimakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Centaur Review

Found 4 issue(s) (2 warning).

server/chat.ts

Correct and well-scoped fix for two related session-list bugs; main gap is missing test coverage for the new 1-hour grace period filter logic.

  • 🟡 missing_tests (L1356): The 1-hour grace period logic is not covered by existing tests in server/tests/session-cache.test.ts. The test suite verifies that zero-turn inactive sessions are hidden and that active sessions are shown, but no test creates a recent zero-turn inactive session to verify it's kept, nor an old one to verify it's hidden. Add two tests: one with createdAt < 1 hour ago (should show) and one with createdAt > 1 hour ago (should hide). [fixable]
  • 🟡 regressions (L1356): The old filter unconditionally hid zero-turn, zero-prompt, inactive sessions. The new filter keeps them visible for up to 1 hour. This means sessions discovered from the filesystem (e.g. automated code review sessions) that were created within the last hour will now appear in the session list, where previously they were always hidden. This is likely intentional but is a behavioral change — the PR title mentions 'missing new sessions', so this is presumably the fix for that. Worth a note in the PR description.
  • 🔵 missing_tests (L689): The timestamp-preservation path (passing existing updatedAt on resume) has no direct test at the chat.ts integration level. The underlying EventStore.upsertSession respecting an explicit updatedAt is well-tested in protocol/tests/event-store.test.ts, so the risk is low, but a test in session-cache.test.ts or a chat-level test verifying that resuming a session doesn't change its updatedAt would confirm the end-to-end behavior. [fixable]
  • 🔵 style (L1356): The constant ONE_HOUR is defined inline inside the filter callback and re-created on every call to getSessionsCached (and for every session in the list). Consider hoisting it to module scope or into constants.ts alongside other server-wide constants for reuse and discoverability. [fixable]

Comment thread server/chat.ts
if (m.numTurns === 0 && m.promptCount === 0 && !m.isActive) return false;
// always show regardless of turn count. Recently created sessions
// (< 1 hour) are kept even with no turns — they may still be starting.
if (m.numTurns === 0 && m.promptCount === 0 && !m.isActive) {
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 missing_tests: The 1-hour grace period logic is not covered by existing tests in server/tests/session-cache.test.ts. The test suite verifies that zero-turn inactive sessions are hidden and that active sessions are shown, but no test creates a recent zero-turn inactive session to verify it's kept, nor an old one to verify it's hidden. Add two tests: one with createdAt < 1 hour ago (should show) and one with createdAt > 1 hour ago (should hide). [fixable]

Comment thread server/chat.ts
if (m.numTurns === 0 && m.promptCount === 0 && !m.isActive) return false;
// always show regardless of turn count. Recently created sessions
// (< 1 hour) are kept even with no turns — they may still be starting.
if (m.numTurns === 0 && m.promptCount === 0 && !m.isActive) {
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 regressions: The old filter unconditionally hid zero-turn, zero-prompt, inactive sessions. The new filter keeps them visible for up to 1 hour. This means sessions discovered from the filesystem (e.g. automated code review sessions) that were created within the last hour will now appear in the session list, where previously they were always hidden. This is likely intentional but is a behavioral change — the PR title mentions 'missing new sessions', so this is presumably the fix for that. Worth a note in the PR description.

Comment thread server/chat.ts
// even if the query loop dies before the first assistant event.
// Preserve existing updatedAt so server restarts don't reset all timestamps.
if (options.resume) {
const existingMeta = eventStore.getSession(options.resume);
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 missing_tests: The timestamp-preservation path (passing existing updatedAt on resume) has no direct test at the chat.ts integration level. The underlying EventStore.upsertSession respecting an explicit updatedAt is well-tested in protocol/tests/event-store.test.ts, so the risk is low, but a test in session-cache.test.ts or a chat-level test verifying that resuming a session doesn't change its updatedAt would confirm the end-to-end behavior. [fixable]

Comment thread server/chat.ts
if (m.numTurns === 0 && m.promptCount === 0 && !m.isActive) return false;
// always show regardless of turn count. Recently created sessions
// (< 1 hour) are kept even with no turns — they may still be starting.
if (m.numTurns === 0 && m.promptCount === 0 && !m.isActive) {
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 style: The constant ONE_HOUR is defined inline inside the filter callback and re-created on every call to getSessionsCached (and for every session in the list). Consider hoisting it to module scope or into constants.ts alongside other server-wide constants for reuse and discoverability. [fixable]

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@dimakis
Copy link
Copy Markdown
Owner Author

dimakis commented May 16, 2026

Centaur Review

Found 4 issue(s) (2 warning).

server/__tests__/session-cache.test.ts

Solid fix for timestamp preservation and grace period filtering. Main concern is the existing test at line 79 now relies on NaN arithmetic to pass — it should explicitly set createdAt to an old timestamp to properly exercise the new code path.

  • 🟡 bugs (L79): Existing test 'hides sessions with numTurns=0...' no longer matches the implementation's logic path. After this PR, the filter accesses m.createdAt, but the mock at line 79 doesn't include createdAt. The test passes accidentally because Date.now() - undefined yields NaN, and NaN < ZERO_TURN_GRACE_MS is false. This is fragile — if createdAt handling changes (e.g., a default fallback), the test could silently break. Add createdAt: 1000 (or a similarly old timestamp) to the sess-unused mock to explicitly test the 'past grace period' path. [fixable]
  • 🟡 missing_tests (L392): The 'resume timestamp preservation' tests re-implement the upsert logic from chat.ts inline rather than calling the actual function (_startChatInner or a testable wrapper). They verify that the mock pattern works, not that the real code does the right thing. If the implementation in chat.ts drifts from this pattern (e.g., someone changes the spread order or condition), these tests will still pass while the real behavior is broken. Consider testing through the actual exported function if possible, or at minimum add a comment acknowledging this is a pattern test.

server/chat.ts

Solid fix for timestamp preservation and grace period filtering. Main concern is the existing test at line 79 now relies on NaN arithmetic to pass — it should explicitly set createdAt to an old timestamp to properly exercise the new code path.

  • 🔵 unsafe_assumptions (L1357): Date.now() is called on every filter iteration inside getSessionsCached(). For large session lists, this creates slightly different timestamps across items. Not a practical bug, but extracting const now = Date.now() before the .filter() call would be more precise and consistent — and avoids repeated syscalls. [fixable]
  • 🔵 style (L38): The constant ZERO_TURN_GRACE_MS is well-named but lives at module top-level in chat.ts, which is already a large file. Consider colocating it with SESSION_PAGE_SIZE in constants.ts since it's also a session-list behavioral constant. [fixable]

@dimakis
Copy link
Copy Markdown
Owner Author

dimakis commented May 16, 2026

/review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant