소스 검색

fix: settle dashboard delivery on dispose

Alvin Unreal 1 개월 전
부모
커밋
6701d2121c
3개의 변경된 파일382개의 추가작업 그리고 219개의 파일을 삭제
  1. 5 1
      src/interview/codemap.md
  2. 255 218
      src/interview/dashboard-manager.ts
  3. 122 0
      src/interview/manager.test.ts

+ 5 - 1
src/interview/codemap.md

@@ -116,7 +116,8 @@
 
 - `handleCommandExecuteBefore`
   - blank input with no active interview starts ideation,
-  - matching slug/path resumes an existing interview,
+  - matching slug/path resumes an existing interview only for its durable
+    frontmatter owner session,
   - otherwise creates a new interview and injects kickoff prompt.
 
 - `handleEvent`
@@ -150,3 +151,6 @@
   browser submissions. Session polling claims each queued value and must
   acknowledge it after service delivery; rejected deliveries roll the claim
   back without clearing the answer, chat message, block comment, or nudge.
+- Markdown documents persist their owning `sessionID` in frontmatter. A
+  different session cannot resume or mutate an owned document, while the
+  original session can resume it across plugin processes.

+ 255 - 218
src/interview/dashboard-manager.ts

@@ -72,6 +72,7 @@ export function createDashboardManager(
   >();
   const nonRedeliverableInProcessClaims = new Set<string>();
   const inFlightInProcessClaims = new Map<string, symbol>();
+  const activeInProcessEvents = new Set<Promise<void>>();
 
   function acquireInProcessClaim(claimId: string): symbol | null {
     if (
@@ -399,6 +400,13 @@ export function createDashboardManager(
   }
 
   let disposePromise: Promise<void> | null = null;
+
+  async function waitForActiveInProcessEvents(): Promise<void> {
+    while (activeInProcessEvents.size > 0) {
+      await Promise.all([...activeInProcessEvents]);
+    }
+  }
+
   function dispose(): Promise<void> {
     if (disposePromise) return disposePromise;
 
@@ -407,16 +415,17 @@ export function createDashboardManager(
     const sessionsToUnregister = [...registeredSessions];
     registeredSessions.clear();
     pendingAcks.clear();
-    pendingInProcessAcks.clear();
-    inFlightDeliveries.clear();
-    inFlightInProcessClaims.clear();
-    nonRedeliverableClaims.clear();
-    nonRedeliverableInProcessClaims.clear();
     service.setStatePushCallback(() => {});
     service.setOnInterviewCreated(() => {});
 
     disposePromise = (async () => {
       await initPromise;
+      await waitForActiveInProcessEvents();
+      pendingInProcessAcks.clear();
+      inFlightDeliveries.clear();
+      inFlightInProcessClaims.clear();
+      nonRedeliverableClaims.clear();
+      nonRedeliverableInProcessClaims.clear();
       stopFallbackTimer();
       if (!isDashboard && dashboardBaseUrl && authToken) {
         await Promise.all(
@@ -623,242 +632,270 @@ export function createDashboardManager(
       await service.handleCommandExecuteBefore(input, output);
     },
     handleEvent: async (input) => {
-      await ensureInitialized();
-      if (disposed) return;
-      const { event } = input;
-      const properties = event.properties ?? {};
-      const info = properties.info;
-      const sessionID =
-        typeof info === 'object' &&
-        info !== null &&
-        'id' in info &&
-        typeof info.id === 'string'
-          ? info.id
-          : typeof properties.sessionID === 'string'
-            ? properties.sessionID
-            : null;
-
-      await service.handleEvent(input);
-
-      // Event hook: Session is idle. Check for any pending user submissions
-      // queued on the dashboard and deliver them to OpenCode.
-      if (event.type === 'session.status' || event.type === 'session.idle') {
-        const status = properties.status as { type?: string } | undefined;
-        const isIdleEvent =
-          event.type === 'session.idle' || status?.type === 'idle';
-        if (sessionID && isIdleEvent) {
-          const interviewId = service.getActiveInterviewId(sessionID);
-          if (!isDashboard && dashboardBaseUrl) {
-            // Session mode: HTTP poll the dashboard
-            await pollPendingAnswers(sessionID);
-            await pollNudgeAction(sessionID);
-            await pollBlockComment(sessionID);
-            await pollChat(sessionID);
-          } else if (interviewId && dashboard) {
-            // Dashboard mode: read directly from in-process cache
-            const pending = canClaimInProcess(interviewId, 'answers')
-              ? dashboard.claimPendingAnswers(interviewId)
-              : null;
-            const owner = pending
-              ? acquireInProcessClaim(pending.claimId)
+      let finishEvent!: () => void;
+      const eventDone = new Promise<void>((resolve) => {
+        finishEvent = resolve;
+      });
+      activeInProcessEvents.add(eventDone);
+
+      try {
+        await ensureInitialized();
+        if (disposed) return;
+        const { event } = input;
+        const properties = event.properties ?? {};
+        const info = properties.info;
+        const sessionID =
+          typeof info === 'object' &&
+          info !== null &&
+          'id' in info &&
+          typeof info.id === 'string'
+            ? info.id
+            : typeof properties.sessionID === 'string'
+              ? properties.sessionID
               : null;
-            if (pending && owner) {
-              log('[interview] delivering pending answers (in-process)', {
-                interviewId,
-                count: pending.answers.length,
-              });
-              try {
-                await service.submitAnswers(interviewId, pending.answers);
-                if (
-                  inFlightInProcessClaims.get(pending.claimId) === owner &&
-                  !dashboard.acknowledgePending(
-                    interviewId,
-                    'answers',
-                    pending.claimId,
-                  )
-                ) {
-                  rememberInProcessAck(interviewId, 'answers', pending.claimId);
-                  log('[interview] answer delivery acknowledgement failed', {
-                    interviewId,
+
+        await service.handleEvent(input);
+
+        // Event hook: Session is idle. Check for any pending user submissions
+        // queued on the dashboard and deliver them to OpenCode.
+        if (event.type === 'session.status' || event.type === 'session.idle') {
+          const status = properties.status as { type?: string } | undefined;
+          const isIdleEvent =
+            event.type === 'session.idle' || status?.type === 'idle';
+          if (sessionID && isIdleEvent) {
+            const interviewId = service.getActiveInterviewId(sessionID);
+            if (!isDashboard && dashboardBaseUrl) {
+              // Session mode: HTTP poll the dashboard
+              await pollPendingAnswers(sessionID);
+              await pollNudgeAction(sessionID);
+              await pollBlockComment(sessionID);
+              await pollChat(sessionID);
+            } else if (interviewId && dashboard) {
+              // Dashboard mode: read directly from in-process cache
+              const pending = canClaimInProcess(interviewId, 'answers')
+                ? dashboard.claimPendingAnswers(interviewId)
+                : null;
+              const owner = pending
+                ? acquireInProcessClaim(pending.claimId)
+                : null;
+              if (pending && owner) {
+                log('[interview] delivering pending answers (in-process)', {
+                  interviewId,
+                  count: pending.answers.length,
+                });
+                try {
+                  await service.submitAnswers(interviewId, pending.answers);
+                  if (
+                    inFlightInProcessClaims.get(pending.claimId) === owner &&
+                    !dashboard.acknowledgePending(
+                      interviewId,
+                      'answers',
+                      pending.claimId,
+                    )
+                  ) {
+                    rememberInProcessAck(
+                      interviewId,
+                      'answers',
+                      pending.claimId,
+                    );
+                    log('[interview] answer delivery acknowledgement failed', {
+                      interviewId,
+                    });
+                  }
+                } catch (error) {
+                  if (inFlightInProcessClaims.get(pending.claimId) === owner) {
+                    dashboard.rollbackPending(
+                      interviewId,
+                      'answers',
+                      pending.claimId,
+                    );
+                  }
+                  log('[interview] answer delivery rolled back', {
+                    error:
+                      error instanceof Error ? error.message : String(error),
                   });
+                } finally {
+                  releaseInProcessClaim(pending.claimId, owner);
                 }
-              } catch (error) {
-                if (inFlightInProcessClaims.get(pending.claimId) === owner) {
-                  dashboard.rollbackPending(
-                    interviewId,
-                    'answers',
-                    pending.claimId,
-                  );
-                }
-                log('[interview] answer delivery rolled back', {
-                  error: error instanceof Error ? error.message : String(error),
-                });
-              } finally {
-                releaseInProcessClaim(pending.claimId, owner);
               }
-            }
-            const nudge = canClaimInProcess(interviewId, 'nudge')
-              ? dashboard.claimNudgeAction(interviewId)
-              : null;
-            const nudgeOwner = nudge
-              ? acquireInProcessClaim(nudge.claimId)
-              : null;
-            if (nudge && nudgeOwner) {
-              log('[interview] delivering nudge action (in-process)', {
-                interviewId,
-                action: nudge.action,
-              });
-              try {
-                await service.handleNudgeAction(interviewId, nudge.action);
-                if (
-                  inFlightInProcessClaims.get(nudge.claimId) === nudgeOwner &&
-                  !dashboard.acknowledgePending(
-                    interviewId,
-                    'nudge',
-                    nudge.claimId,
-                  )
-                ) {
-                  rememberInProcessAck(interviewId, 'nudge', nudge.claimId);
-                  log('[interview] nudge delivery acknowledgement failed', {
-                    interviewId,
+              const nudge = canClaimInProcess(interviewId, 'nudge')
+                ? dashboard.claimNudgeAction(interviewId)
+                : null;
+              const nudgeOwner = nudge
+                ? acquireInProcessClaim(nudge.claimId)
+                : null;
+              if (nudge && nudgeOwner) {
+                log('[interview] delivering nudge action (in-process)', {
+                  interviewId,
+                  action: nudge.action,
+                });
+                try {
+                  await service.handleNudgeAction(interviewId, nudge.action);
+                  if (
+                    inFlightInProcessClaims.get(nudge.claimId) === nudgeOwner &&
+                    !dashboard.acknowledgePending(
+                      interviewId,
+                      'nudge',
+                      nudge.claimId,
+                    )
+                  ) {
+                    rememberInProcessAck(interviewId, 'nudge', nudge.claimId);
+                    log('[interview] nudge delivery acknowledgement failed', {
+                      interviewId,
+                    });
+                  }
+                } catch (error) {
+                  if (
+                    inFlightInProcessClaims.get(nudge.claimId) === nudgeOwner
+                  ) {
+                    dashboard.rollbackPending(
+                      interviewId,
+                      'nudge',
+                      nudge.claimId,
+                    );
+                  }
+                  log('[interview] nudge delivery rolled back', {
+                    error:
+                      error instanceof Error ? error.message : String(error),
                   });
+                } finally {
+                  releaseInProcessClaim(nudge.claimId, nudgeOwner);
                 }
-              } catch (error) {
-                if (inFlightInProcessClaims.get(nudge.claimId) === nudgeOwner) {
-                  dashboard.rollbackPending(
-                    interviewId,
-                    'nudge',
-                    nudge.claimId,
-                  );
-                }
-                log('[interview] nudge delivery rolled back', {
-                  error: error instanceof Error ? error.message : String(error),
-                });
-              } finally {
-                releaseInProcessClaim(nudge.claimId, nudgeOwner);
               }
-            }
-            const comment = canClaimInProcess(interviewId, 'block-comment')
-              ? dashboard.claimBlockComment(interviewId)
-              : null;
-            const commentOwner = comment
-              ? acquireInProcessClaim(comment.claimId)
-              : null;
-            if (comment && commentOwner) {
-              log('[interview] delivering block comment (in-process)', {
-                interviewId,
-                section: comment.comment.section,
-              });
-              try {
-                await service.submitBlockComment(
+              const comment = canClaimInProcess(interviewId, 'block-comment')
+                ? dashboard.claimBlockComment(interviewId)
+                : null;
+              const commentOwner = comment
+                ? acquireInProcessClaim(comment.claimId)
+                : null;
+              if (comment && commentOwner) {
+                log('[interview] delivering block comment (in-process)', {
                   interviewId,
-                  comment.comment.section,
-                  comment.comment.comment,
-                );
-                if (
-                  inFlightInProcessClaims.get(comment.claimId) ===
-                    commentOwner &&
-                  !dashboard.acknowledgePending(
-                    interviewId,
-                    'block-comment',
-                    comment.claimId,
-                  )
-                ) {
-                  rememberInProcessAck(
-                    interviewId,
-                    'block-comment',
-                    comment.claimId,
-                  );
-                  log(
-                    '[interview] block comment delivery acknowledgement failed',
-                    { interviewId },
-                  );
-                }
-              } catch (error) {
-                if (
-                  inFlightInProcessClaims.get(comment.claimId) === commentOwner
-                ) {
-                  dashboard.rollbackPending(
+                  section: comment.comment.section,
+                });
+                try {
+                  await service.submitBlockComment(
                     interviewId,
-                    'block-comment',
-                    comment.claimId,
+                    comment.comment.section,
+                    comment.comment.comment,
                   );
+                  if (
+                    inFlightInProcessClaims.get(comment.claimId) ===
+                      commentOwner &&
+                    !dashboard.acknowledgePending(
+                      interviewId,
+                      'block-comment',
+                      comment.claimId,
+                    )
+                  ) {
+                    rememberInProcessAck(
+                      interviewId,
+                      'block-comment',
+                      comment.claimId,
+                    );
+                    log(
+                      '[interview] block comment delivery acknowledgement failed',
+                      { interviewId },
+                    );
+                  }
+                } catch (error) {
+                  if (
+                    inFlightInProcessClaims.get(comment.claimId) ===
+                    commentOwner
+                  ) {
+                    dashboard.rollbackPending(
+                      interviewId,
+                      'block-comment',
+                      comment.claimId,
+                    );
+                  }
+                  log('[interview] block comment delivery rolled back', {
+                    error:
+                      error instanceof Error ? error.message : String(error),
+                  });
+                } finally {
+                  releaseInProcessClaim(comment.claimId, commentOwner);
                 }
-                log('[interview] block comment delivery rolled back', {
-                  error: error instanceof Error ? error.message : String(error),
-                });
-              } finally {
-                releaseInProcessClaim(comment.claimId, commentOwner);
               }
-            }
-            const chat = canClaimInProcess(interviewId, 'chat')
-              ? dashboard.claimChatMessage(interviewId)
-              : null;
-            const chatOwner = chat ? acquireInProcessClaim(chat.claimId) : null;
-            if (chat && chatOwner) {
-              log('[interview] delivering chat message (in-process)', {
-                interviewId,
-              });
-              try {
-                await service.submitChat(interviewId, chat.message);
-                if (
-                  inFlightInProcessClaims.get(chat.claimId) === chatOwner &&
-                  !dashboard.acknowledgePending(
-                    interviewId,
-                    'chat',
-                    chat.claimId,
-                  )
-                ) {
-                  rememberInProcessAck(interviewId, 'chat', chat.claimId);
-                  log('[interview] chat delivery acknowledgement failed', {
-                    interviewId,
+              const chat = canClaimInProcess(interviewId, 'chat')
+                ? dashboard.claimChatMessage(interviewId)
+                : null;
+              const chatOwner = chat
+                ? acquireInProcessClaim(chat.claimId)
+                : null;
+              if (chat && chatOwner) {
+                log('[interview] delivering chat message (in-process)', {
+                  interviewId,
+                });
+                try {
+                  await service.submitChat(interviewId, chat.message);
+                  if (
+                    inFlightInProcessClaims.get(chat.claimId) === chatOwner &&
+                    !dashboard.acknowledgePending(
+                      interviewId,
+                      'chat',
+                      chat.claimId,
+                    )
+                  ) {
+                    rememberInProcessAck(interviewId, 'chat', chat.claimId);
+                    log('[interview] chat delivery acknowledgement failed', {
+                      interviewId,
+                    });
+                  }
+                } catch (error) {
+                  if (inFlightInProcessClaims.get(chat.claimId) === chatOwner) {
+                    dashboard.rollbackPending(
+                      interviewId,
+                      'chat',
+                      chat.claimId,
+                    );
+                  }
+                  log('[interview] chat delivery rolled back', {
+                    error:
+                      error instanceof Error ? error.message : String(error),
                   });
+                } finally {
+                  releaseInProcessClaim(chat.claimId, chatOwner);
                 }
-              } catch (error) {
-                if (inFlightInProcessClaims.get(chat.claimId) === chatOwner) {
-                  dashboard.rollbackPending(interviewId, 'chat', chat.claimId);
-                }
-                log('[interview] chat delivery rolled back', {
-                  error: error instanceof Error ? error.message : String(error),
-                });
-              } finally {
-                releaseInProcessClaim(chat.claimId, chatOwner);
               }
             }
-          }
 
-          // Refresh state: calls getInterviewState → syncInterview →
-          // onStateChange. Runs AFTER nudge/answer processing so
-          // sessionBusy is accurate.
-          if (interviewId) {
-            service.getInterviewState(interviewId).catch((err) => {
-              log('[interview] failed to refresh state', {
-                error: err instanceof Error ? err.message : String(err),
+            // Refresh state: calls getInterviewState → syncInterview →
+            // onStateChange. Runs AFTER nudge/answer processing so
+            // sessionBusy is accurate.
+            if (interviewId) {
+              service.getInterviewState(interviewId).catch((err) => {
+                log('[interview] failed to refresh state', {
+                  error: err instanceof Error ? err.message : String(err),
+                });
               });
-            });
+            }
           }
         }
-      }
 
-      // Clean up when a session is deleted
-      if (event.type === 'session.deleted' && sessionID) {
-        registeredSessions.delete(sessionID);
-        if (!isDashboard && registeredSessions.size === 0) {
-          stopFallbackTimer();
-        }
-        if (dashboard) {
-          dashboard.removeSession(sessionID);
-        } else if (dashboardBaseUrl && authToken) {
-          unregisterSessionViaHttp(
-            dashboardBaseUrl,
-            authToken,
-            sessionID,
-          ).catch((err) => {
-            log('[interview] failed to unregister deleted session:', {
-              error: err instanceof Error ? err.message : String(err),
+        // Clean up when a session is deleted
+        if (event.type === 'session.deleted' && sessionID) {
+          registeredSessions.delete(sessionID);
+          if (!isDashboard && registeredSessions.size === 0) {
+            stopFallbackTimer();
+          }
+          if (dashboard) {
+            dashboard.removeSession(sessionID);
+          } else if (dashboardBaseUrl && authToken) {
+            unregisterSessionViaHttp(
+              dashboardBaseUrl,
+              authToken,
+              sessionID,
+            ).catch((err) => {
+              log('[interview] failed to unregister deleted session:', {
+                error: err instanceof Error ? err.message : String(err),
+              });
             });
-          });
+          }
         }
+      } finally {
+        finishEvent();
+        activeInProcessEvents.delete(eventDone);
       }
     },
     dispose,

+ 122 - 0
src/interview/manager.test.ts

@@ -703,6 +703,128 @@ describe('interview manager - edge cases', () => {
     }
   });
 
+  test('waits for accepted delivery before disposal and replacement polling', async () => {
+    const tempDir = await fs.mkdtemp('/tmp/manager-test-');
+    const ctx = createMockContext({ directory: tempDir });
+    const freePort = await findFreePort();
+    const config = createTestConfig({ port: freePort, dashboard: true });
+    const messages: Array<{
+      info?: { role: string };
+      parts?: Array<{ type: string; text?: string }>;
+    }> = [];
+    let signalFirstDelivery!: () => void;
+    const firstDeliveryStarted = new Promise<void>((resolve) => {
+      signalFirstDelivery = resolve;
+    });
+    let releaseFirstDelivery!: () => void;
+    const firstDeliveryGate = new Promise<void>((resolve) => {
+      releaseFirstDelivery = resolve;
+    });
+    let submitAttempts = 0;
+    const runtime = {
+      messages: async () => messages,
+      notify: async () => {},
+      continue: async () => {
+        submitAttempts++;
+        signalFirstDelivery();
+        await firstDeliveryGate;
+      },
+      rename: async () => {},
+    };
+    const manager = createDashboardManager(ctx, config, freePort, 'interview', {
+      runtime,
+    });
+
+    try {
+      await manager.handleCommandExecuteBefore(
+        {
+          command: 'interview',
+          sessionID: 'session-dispose-race',
+          arguments: 'Dispose Race Test',
+        },
+        { parts: [] },
+      );
+      const interviewId = manager.service.getActiveInterviewId(
+        'session-dispose-race',
+      );
+      expect(interviewId).not.toBeNull();
+      messages.push({
+        info: { role: 'assistant' },
+        parts: [
+          {
+            type: 'text',
+            text: '<interview_state>{"summary":"Draft","questions":[{"id":"q-1","question":"What?","options":["A"]}]}</interview_state>',
+          },
+        ],
+      });
+      const auth = await readDashboardAuthFile(freePort);
+      expect(auth).not.toBeNull();
+      const queued = await fetch(
+        `http://127.0.0.1:${freePort}/api/interviews/${interviewId}/answers?token=${auth?.token}`,
+        {
+          method: 'POST',
+          headers: { 'content-type': 'application/json' },
+          body: JSON.stringify({
+            answers: [{ questionId: 'q-1', answer: 'A' }],
+          }),
+        },
+      );
+      expect(queued.status).toBe(202);
+
+      const idleEvent = {
+        event: {
+          type: 'session.status',
+          properties: {
+            sessionID: 'session-dispose-race',
+            status: { type: 'idle' },
+          },
+        },
+      };
+      const delivery = manager.handleEvent(idleEvent);
+      await firstDeliveryStarted;
+      let disposed = false;
+      const disposal = manager.dispose().then(() => {
+        disposed = true;
+      });
+      await Promise.resolve();
+      expect(disposed).toBe(false);
+
+      releaseFirstDelivery();
+      await delivery;
+      await disposal;
+      expect(submitAttempts).toBe(1);
+
+      const files = await fs.readdir(`${tempDir}/interview`);
+      const documentPath = `${tempDir}/interview/${files.find((file) => file.endsWith('.md'))}`;
+      const replacement = createDashboardManager(
+        ctx,
+        config,
+        freePort,
+        'interview',
+        { runtime },
+      );
+      try {
+        await replacement.handleCommandExecuteBefore(
+          {
+            command: 'interview',
+            sessionID: 'session-dispose-race',
+            arguments: documentPath,
+          },
+          { parts: [] },
+        );
+        await replacement.handleEvent(idleEvent);
+        expect(submitAttempts).toBe(1);
+        expect(await fs.readFile(documentPath, 'utf8')).toContain('A: A');
+      } finally {
+        await replacement.dispose();
+      }
+    } finally {
+      releaseFirstDelivery();
+      await manager.dispose();
+      await fs.rm(tempDir, { recursive: true, force: true });
+    }
+  });
+
   test('handles session.status event with idle status', async () => {
     const tempDir = await fs.mkdtemp('/tmp/manager-test-');
     const ctx = createMockContext({ directory: tempDir });