Browse Source

fix(interview): bound abandoned interview record retention

interviewsById and browserOpened grew without limit: when a session was
deleted (or a new interview replaced an active one on the same session),
the record was marked 'abandoned' but never removed, and browserOpened
entries were never released. In a long-running session/dashboard
process this leaked one entry per interview for the life of the process.

Add abandonInterview()/pruneAbandonedInterviews() that mark a record
abandoned and evict the oldest abandoned records beyond a cap from both
collections. Recent abandoned interviews are retained so an open browser
tab can still render their final state.

Follow-up to the resource audit in #597 / #600.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Zerdeşt Taifour 1 month ago
parent
commit
7fe8dca5f6
2 changed files with 117 additions and 3 deletions
  1. 79 0
      src/interview/interview.test.ts
  2. 38 3
      src/interview/service.ts

+ 79 - 0
src/interview/interview.test.ts

@@ -1892,3 +1892,82 @@ describe('InterviewConfigSchema port validation', () => {
     expect(() => InterviewConfigSchema.parse({ port: 3.5 })).toThrow();
   });
 });
+
+describe('interview service abandoned-record retention', () => {
+  // Mirrors MAX_RETAINED_ABANDONED in service.ts.
+  const RETENTION_CAP = 50;
+
+  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-');
+    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');
+
+    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' },
+      },
+    });
+
+    // 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');
+
+    await fs.rm(tempDir, { recursive: true, force: true });
+  });
+});

+ 38 - 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.
+ */
+const MAX_RETAINED_ABANDONED = 50;
+
 function isTruthyEnvFlag(value: string | undefined): boolean {
   if (!value) {
     return false;
@@ -287,6 +295,33 @@ 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 {
+    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) =>
+          new Date(a.createdAt).getTime() - new Date(b.createdAt).getTime(),
+      )
+      .slice(0, overflow)
+      .forEach((record) => {
+        interviewsById.delete(record.id);
+        browserOpened.delete(record.id);
+      });
+  }
+
   async function createInterview(
     sessionID: string,
     idea: string,
@@ -300,7 +335,7 @@ export function createInterviewService(
           return active;
         }
 
-        active.status = 'abandoned';
+        abandonInterview(active);
       }
     }
 
@@ -338,7 +373,7 @@ export function createInterviewService(
           return active;
         }
 
-        active.status = 'abandoned';
+        abandonInterview(active);
       }
     }
 
@@ -695,7 +730,7 @@ export function createInterviewService(
         return;
       }
 
-      interview.status = 'abandoned';
+      abandonInterview(interview);
       fileCache = null;
       activeInterviewIds.delete(deletedSessionId);
       log('[interview] session deleted, interview marked abandoned', {