Browse Source

Merge pull request #614 from alvinunreal/fix/interview-service-record-pruning

fix(interview): bound abandoned interview record retention
Alvin 1 month ago
parent
commit
cd07675408
3 changed files with 181 additions and 4 deletions
  1. 133 1
      src/interview/interview.test.ts
  2. 46 3
      src/interview/service.ts
  3. 2 0
      src/interview/types.ts

+ 133 - 1
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';
 
@@ -1892,3 +1895,132 @@ describe('InterviewConfigSchema port validation', () => {
     expect(() => InterviewConfigSchema.parse({ port: 3.5 })).toThrow();
   });
 });
+
+describe('interview service abandoned-record retention', () => {
+  const RETENTION_CAP = MAX_RETAINED_ABANDONED;
+
+  async function createInterviewOnSession(
+    service: ReturnType<typeof createInterviewService>,
+    ctx: ReturnType<typeof createMockContext>,
+    index: number,
+  ): Promise<string> {
+    const output = { parts: [] as Array<{ type: string; text?: string }> };
+    await service.handleCommandExecuteBefore(
+      {
+        command: 'interview',
+        sessionID: `session-${index}`,
+        arguments: `Idea ${index}`,
+      },
+      output,
+    );
+    return requireInterviewId(
+      extractInterviewIdFromLastPrompt(ctx.client.session.prompt),
+    );
+  }
+
+  test('evicts oldest abandoned records once the retention cap is exceeded', 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 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-0' },
+        },
+      });
+
+      await expect(service.getInterviewState(abandonedIds[0])).rejects.toThrow(
+        'Interview not found',
+      );
+
+      const oldActiveState = await service.getInterviewState(oldActiveId);
+      expect(oldActiveState.mode).toBe('abandoned');
+
+      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-');
+    try {
+      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' },
+        },
+      });
+
+      // 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 });
+    }
+  });
+});

+ 46 - 3
src/interview/service.ts

@@ -43,6 +43,14 @@ import type {
 const COMMAND_NAME = 'interview';
 const DEFAULT_MAX_QUESTIONS = 2;
 
+/**
+ * Cap on retained abandoned interview records. Abandoned interviews are kept
+ * briefly so a still-open browser tab can render their final state, but
+ * without a bound the `interviewsById` and `browserOpened` collections grow
+ * for the life of a long-running session/dashboard process.
+ */
+export const MAX_RETAINED_ABANDONED = 50;
+
 function isTruthyEnvFlag(value: string | undefined): boolean {
   if (!value) {
     return false;
@@ -171,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;
@@ -287,6 +296,40 @@ export function createInterviewService(
     return interviewsById.get(interviewId) ?? null;
   }
 
+  /**
+   * Mark an interview abandoned and prune the oldest abandoned records so the
+   * 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();
+  }
+
+  function pruneAbandonedInterviews(): void {
+    const abandoned = [...interviewsById.values()].filter(
+      (record) => record.status === 'abandoned',
+    );
+    const overflow = abandoned.length - MAX_RETAINED_ABANDONED;
+    if (overflow <= 0) return;
+    abandoned
+      .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);
+        browserOpened.delete(record.id);
+      });
+  }
+
   async function createInterview(
     sessionID: string,
     idea: string,
@@ -300,7 +343,7 @@ export function createInterviewService(
           return active;
         }
 
-        active.status = 'abandoned';
+        abandonInterview(active);
       }
     }
 
@@ -338,7 +381,7 @@ export function createInterviewService(
           return active;
         }
 
-        active.status = 'abandoned';
+        abandonInterview(active);
       }
     }
 
@@ -695,7 +738,7 @@ export function createInterviewService(
         return;
       }
 
-      interview.status = 'abandoned';
+      abandonInterview(interview);
       fileCache = null;
       activeInterviewIds.delete(deletedSessionId);
       log('[interview] session deleted, interview marked abandoned', {

+ 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;
 }