Browse Source

fix(interview): prune abandoned records by abandonment order

Alvin Unreal 1 month ago
parent
commit
928f3dce0b
3 changed files with 108 additions and 45 deletions
  1. 93 40
      src/interview/interview.test.ts
  2. 13 5
      src/interview/service.ts
  3. 2 0
      src/interview/types.ts

+ 93 - 40
src/interview/interview.test.ts

@@ -4,7 +4,10 @@ import { createServer } from 'node:http';
 import * as path from 'node:path';
 import { InterviewConfigSchema } from '../config/schema';
 import { createInterviewServer } from './server';
-import { createInterviewService as createRealInterviewService } from './service';
+import {
+  createInterviewService as createRealInterviewService,
+  MAX_RETAINED_ABANDONED,
+} from './service';
 import type { InterviewAnswer } from './types';
 import { renderInterviewPage } from './ui';
 
@@ -1894,8 +1897,7 @@ describe('InterviewConfigSchema port validation', () => {
 });
 
 describe('interview service abandoned-record retention', () => {
-  // Mirrors MAX_RETAINED_ABANDONED in service.ts.
-  const RETENTION_CAP = 50;
+  const RETENTION_CAP = MAX_RETAINED_ABANDONED;
 
   async function createInterviewOnSession(
     service: ReturnType<typeof createInterviewService>,
@@ -1918,56 +1920,107 @@ describe('interview service abandoned-record retention', () => {
 
   test('evicts oldest abandoned records once the retention cap is exceeded', async () => {
     const tempDir = await fs.mkdtemp('/tmp/interview-test-');
-    const ctx = createMockContext({ directory: tempDir });
-    const service = createInterviewService(ctx);
-    service.setBaseUrlResolver(async () => 'http://localhost:9999');
-
-    const ids: string[] = [];
-    for (let i = 0; i < RETENTION_CAP + 2; i++) {
-      ids.push(await createInterviewOnSession(service, ctx, i));
-      // Deleting the session abandons the interview, triggering pruning.
+    try {
+      const ctx = createMockContext({ directory: tempDir });
+      const service = createInterviewService(ctx);
+      service.setBaseUrlResolver(async () => 'http://localhost:9999');
+
+      const ids: string[] = [];
+      for (let i = 0; i < RETENTION_CAP + 2; i++) {
+        ids.push(await createInterviewOnSession(service, ctx, i));
+        // Deleting the session abandons the interview, triggering pruning.
+        await service.handleEvent({
+          event: {
+            type: 'session.deleted',
+            properties: { sessionID: `session-${i}` },
+          },
+        });
+      }
+
+      // The two oldest abandoned records are evicted from the registry.
+      await expect(service.getInterviewState(ids[0])).rejects.toThrow(
+        'Interview not found',
+      );
+      await expect(service.getInterviewState(ids[1])).rejects.toThrow(
+        'Interview not found',
+      );
+
+      // The most recent abandoned record is retained and still renders.
+      const retained = await service.getInterviewState(ids[ids.length - 1]);
+      expect(retained.mode).toBe('abandoned');
+    } finally {
+      await fs.rm(tempDir, { recursive: true, force: true });
+    }
+  });
+
+  test('prunes by abandonment order instead of creation order', async () => {
+    const tempDir = await fs.mkdtemp('/tmp/interview-test-');
+    try {
+      const ctx = createMockContext({ directory: tempDir });
+      const service = createInterviewService(ctx);
+      service.setBaseUrlResolver(async () => 'http://localhost:9999');
+
+      const oldActiveId = await createInterviewOnSession(service, ctx, 0);
+      const abandonedIds: string[] = [];
+
+      for (let i = 1; i <= RETENTION_CAP; i++) {
+        const id = await createInterviewOnSession(service, ctx, i);
+        abandonedIds.push(id);
+        await service.handleEvent({
+          event: {
+            type: 'session.deleted',
+            properties: { sessionID: `session-${i}` },
+          },
+        });
+      }
+
+      // Abandoning the old active interview after the cap is full should retain
+      // that newly abandoned record and prune the earliest previously abandoned
+      // record. Its older createdAt must not make it the eviction candidate.
       await service.handleEvent({
         event: {
           type: 'session.deleted',
-          properties: { sessionID: `session-${i}` },
+          properties: { sessionID: 'session-0' },
         },
       });
-    }
 
-    // The two oldest abandoned records are evicted from the registry.
-    await expect(service.getInterviewState(ids[0])).rejects.toThrow(
-      'Interview not found',
-    );
-    await expect(service.getInterviewState(ids[1])).rejects.toThrow(
-      'Interview not found',
-    );
+      await expect(service.getInterviewState(abandonedIds[0])).rejects.toThrow(
+        'Interview not found',
+      );
 
-    // The most recent abandoned record is retained and still renders.
-    const retained = await service.getInterviewState(ids[ids.length - 1]);
-    expect(retained.mode).toBe('abandoned');
+      const oldActiveState = await service.getInterviewState(oldActiveId);
+      expect(oldActiveState.mode).toBe('abandoned');
 
-    await fs.rm(tempDir, { recursive: true, force: true });
+      const latestPreviouslyAbandoned = await service.getInterviewState(
+        abandonedIds[abandonedIds.length - 1],
+      );
+      expect(latestPreviouslyAbandoned.mode).toBe('abandoned');
+    } finally {
+      await fs.rm(tempDir, { recursive: true, force: true });
+    }
   });
 
   test('retains abandoned records that stay within the cap', async () => {
     const tempDir = await fs.mkdtemp('/tmp/interview-test-');
-    const ctx = createMockContext({ directory: tempDir });
-    const service = createInterviewService(ctx);
-    service.setBaseUrlResolver(async () => 'http://localhost:9999');
-
-    const id = await createInterviewOnSession(service, ctx, 0);
-    await service.handleEvent({
-      event: {
-        type: 'session.deleted',
-        properties: { sessionID: 'session-0' },
-      },
-    });
+    try {
+      const ctx = createMockContext({ directory: tempDir });
+      const service = createInterviewService(ctx);
+      service.setBaseUrlResolver(async () => 'http://localhost:9999');
 
-    // Below the cap, the abandoned record is kept so an open tab can still
-    // render its final state.
-    const state = await service.getInterviewState(id);
-    expect(state.mode).toBe('abandoned');
+      const id = await createInterviewOnSession(service, ctx, 0);
+      await service.handleEvent({
+        event: {
+          type: 'session.deleted',
+          properties: { sessionID: 'session-0' },
+        },
+      });
 
-    await fs.rm(tempDir, { recursive: true, force: true });
+      // Below the cap, the abandoned record is kept so an open tab can still
+      // render its final state.
+      const state = await service.getInterviewState(id);
+      expect(state.mode).toBe('abandoned');
+    } finally {
+      await fs.rm(tempDir, { recursive: true, force: true });
+    }
   });
 });

+ 13 - 5
src/interview/service.ts

@@ -49,7 +49,7 @@ const DEFAULT_MAX_QUESTIONS = 2;
  * without a bound the `interviewsById` and `browserOpened` collections grow
  * for the life of a long-running session/dashboard process.
  */
-const MAX_RETAINED_ABANDONED = 50;
+export const MAX_RETAINED_ABANDONED = 50;
 
 function isTruthyEnvFlag(value: string | undefined): boolean {
   if (!value) {
@@ -179,6 +179,7 @@ export function createInterviewService(
     | null = null;
   let onInterviewCreated: ((interview: InterviewRecord) => void) | null = null;
   let idCounter = 0;
+  let abandonedOrderCounter = 0;
 
   function setBaseUrlResolver(resolver: () => Promise<string>): void {
     resolveBaseUrl = resolver;
@@ -300,6 +301,10 @@ export function createInterviewService(
    * in-memory registry (and its browser-open tracking) stays bounded.
    */
   function abandonInterview(interview: InterviewRecord): void {
+    if (interview.status !== 'abandoned') {
+      interview.abandonedAt = nowIso();
+      interview.abandonedOrder = ++abandonedOrderCounter;
+    }
     interview.status = 'abandoned';
     pruneAbandonedInterviews();
   }
@@ -311,10 +316,13 @@ export function createInterviewService(
     const overflow = abandoned.length - MAX_RETAINED_ABANDONED;
     if (overflow <= 0) return;
     abandoned
-      .sort(
-        (a, b) =>
-          new Date(a.createdAt).getTime() - new Date(b.createdAt).getTime(),
-      )
+      .sort((a, b) => {
+        const timeDelta =
+          new Date(a.abandonedAt ?? a.createdAt).getTime() -
+          new Date(b.abandonedAt ?? b.createdAt).getTime();
+        if (timeDelta !== 0) return timeDelta;
+        return (a.abandonedOrder ?? 0) - (b.abandonedOrder ?? 0);
+      })
       .slice(0, overflow)
       .forEach((record) => {
         interviewsById.delete(record.id);

+ 2 - 0
src/interview/types.ts

@@ -43,6 +43,8 @@ export interface InterviewRecord {
   idea: string;
   markdownPath: string;
   createdAt: string;
+  abandonedAt?: string;
+  abandonedOrder?: number;
   status: 'active' | 'abandoned';
   baseMessageCount: number;
 }