Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
102 changes: 102 additions & 0 deletions server/__tests__/session-cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,51 @@ describe('getSessionsCached', () => {
cwd: '/projects/bar',
});
});

it('keeps a recent zero-turn inactive session within the grace period', async () => {
const thirtyMinutesAgo = Date.now() - 30 * 60 * 1000;
mockListSessionsMeta.mockReturnValue([
{
sessionId: 'sess-recent-zero',
summary: 'Just created',
numTurns: 0,
promptCount: 0,
isActive: false,
createdAt: thirtyMinutesAgo,
updatedAt: thirtyMinutesAgo,
branch: null,
cwd: null,
},
]);

const { getSessionsCached } = await import('../chat.js');
const { sessions } = getSessionsCached();

expect(sessions).toHaveLength(1);
expect(sessions[0].id).toBe('sess-recent-zero');
});

it('hides an old zero-turn inactive session past the grace period', async () => {
const twoHoursAgo = Date.now() - 2 * 60 * 60 * 1000;
mockListSessionsMeta.mockReturnValue([
{
sessionId: 'sess-old-zero',
summary: 'Stale empty session',
numTurns: 0,
promptCount: 0,
isActive: false,
createdAt: twoHoursAgo,
updatedAt: twoHoursAgo,
branch: null,
cwd: null,
},
]);

const { getSessionsCached } = await import('../chat.js');
const { sessions } = getSessionsCached();

expect(sessions).toHaveLength(0);
});
});

describe('syncSessionTimestamps', () => {
Expand Down Expand Up @@ -347,6 +392,63 @@ describe('syncSessionTimestamps', () => {
});
});

describe('resume timestamp preservation', () => {
it('preserves updatedAt when resuming an existing session', async () => {
const originalUpdatedAt = Date.now() - 3600_000; // 1 hour ago
const existingMeta = {
sessionId: 'sess-resume',
summary: 'Old session',
updatedAt: originalUpdatedAt,
createdAt: originalUpdatedAt - 7200_000,
numTurns: 5,
promptCount: 3,
isActive: false,
branch: 'main',
cwd: '/projects/foo',
manuallyRenamed: false,
};
mockGetSession.mockReturnValue(existingMeta);

// Simulate the resume upsert pattern from startSession (chat.ts L688-701):
// const existingMeta = eventStore.getSession(id);
// eventStore.upsertSession({ ..., ...(existingMeta ? { updatedAt: existingMeta.updatedAt } : {}) });
const sessionId = 'sess-resume';
const retrieved = mockEventStore.getSession(sessionId);
mockEventStore.upsertSession({
sessionId,
cwd: '/projects/foo',
mode: 'code',
branch: 'main',
...(retrieved ? { updatedAt: retrieved.updatedAt } : {}),
});

expect(mockUpsertSession).toHaveBeenCalledWith(
expect.objectContaining({
sessionId: 'sess-resume',
updatedAt: originalUpdatedAt,
}),
);
});

it('omits updatedAt when resuming a session with no prior metadata', async () => {
mockGetSession.mockReturnValue(null);

const sessionId = 'sess-new-resume';
const retrieved = mockEventStore.getSession(sessionId);
mockEventStore.upsertSession({
sessionId,
cwd: '/projects/foo',
mode: 'code',
branch: 'main',
...(retrieved ? { updatedAt: retrieved.updatedAt } : {}),
});

const call = mockUpsertSession.mock.calls[0][0];
expect(call.sessionId).toBe('sess-new-resume');
expect(call).not.toHaveProperty('updatedAt');
});
});

describe('hideSession persistence', () => {
it('calls eventStore.hideSession to persist deletion', async () => {
const { hideSession } = await import('../chat.js');
Expand Down
14 changes: 10 additions & 4 deletions server/chat.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
SESSION_PAGE_SIZE,
SESSION_MESSAGES_LIMIT,
USER_CLOSEOUT_TIMEOUT_MS,
ZERO_TURN_GRACE_MS,
} from './constants.js';
import { INTERNAL_TOKEN } from './internal-token.js';
import { buildTaskSystemPrompt } from './task-context.js';
Expand Down Expand Up @@ -689,7 +690,9 @@ async function _startChatInner(

// Pre-register resumed sessions in EventStore so they're discoverable
// 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]

eventStore.upsertSession({
sessionId: options.resume,
cwd,
Expand All @@ -698,6 +701,7 @@ async function _startChatInner(
isActive: true,
...(worktreePath ? { wtId } : {}),
...(options.telosTaskId ? { telosTaskId: options.telosTaskId } : {}),
...(existingMeta ? { updatedAt: existingMeta.updatedAt } : {}),
});
}

Expand Down Expand Up @@ -1506,13 +1510,15 @@ export async function getSessions(offset = 0, limit = SESSION_PAGE_SIZE) {
* Returns the same shape as getSessions() for API compatibility.
*/
export function getSessionsCached(offset = 0, limit = SESSION_PAGE_SIZE) {
const now = Date.now();
const all = eventStore.listSessions().filter((m) => {
// Hide sessions that were never used through Mitzo (e.g. automated
// code review sessions discovered from filesystem). Active sessions
// always show regardless of turn count.
// Note: new sessions are created with is_active=1 by default (see EventStore
// schema), so a brand-new session will never be filtered out here.
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]

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.

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]

return now - m.createdAt < ZERO_TURN_GRACE_MS;
}
return true;
});
const page = all.slice(offset, offset + limit);
Expand Down
4 changes: 4 additions & 0 deletions server/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ export const SHUTDOWN_GRACE_MS = 5_000;
export const GUARD_STATS_INTERVAL_MS = 5 * 60 * 1000; // 5 minutes
export const WORKTREE_CLEANUP_INTERVAL_MS = 6 * 60 * 60 * 1000; // 6 hours

// --- Session List ---
/** Grace period for zero-turn inactive sessions — recently created ones stay visible. */
export const ZERO_TURN_GRACE_MS = 60 * 60 * 1000; // 1 hour

// --- Query loop ---
// If the Agent SDK yields no events within this window, we treat the turn
// as unreachable (e.g. model unavailable on the configured provider) and
Expand Down
Loading