Browse Source

fix: address Greptile review comments on interview wizard

alvinreal 1 month ago
parent
commit
05879a7714

+ 8 - 0
src/interview/dashboard.ts

@@ -185,6 +185,7 @@ export function createDashboardServer(config: DashboardConfig): {
   consumeBlockComment: (
     interviewId: string,
   ) => { section: string; comment: string } | null;
+  consumeChatMessage: (interviewId: string) => string | null;
   authToken: string;
   discoverSessionDirectories: () => Promise<void>;
   addManualFolder: (dir: string) => void;
@@ -1354,6 +1355,13 @@ export function createDashboardServer(config: DashboardConfig): {
       entry.pendingBlockComment = null;
       return comment;
     },
+    consumeChatMessage: (id: string) => {
+      const entry = stateCache.get(id);
+      if (!entry?.pendingChatMessage) return null;
+      const message = entry.pendingChatMessage;
+      entry.pendingChatMessage = null;
+      return message;
+    },
     authToken,
     discoverSessionDirectories,
     addManualFolder: (dir: string) => {

+ 11 - 3
src/interview/document.ts

@@ -134,7 +134,12 @@ export function buildInterviewDocument(
   idea: string,
   summary: string,
   history: string,
-  meta?: { sessionID?: string; baseMessageCount?: number },
+  meta?: {
+    sessionID?: string;
+    baseMessageCount?: number;
+    owner?: string;
+    tags?: string[];
+  },
 ): string {
   const normalizedSummary = summary.trim() || 'Waiting for interview answers.';
   const normalizedHistory = history.trim() || 'No answers yet.';
@@ -142,6 +147,9 @@ export function buildInterviewDocument(
   const now = new Date();
   const dateStr = now.toISOString().split('T')[0];
 
+  const owner = meta?.owner ?? 'agent';
+  const tags = meta?.tags ?? ['spec', 'diagnostic'];
+
   const frontmatter = meta?.sessionID
     ? [
         '---',
@@ -150,8 +158,8 @@ export function buildInterviewDocument(
         `updatedAt: ${now.toISOString()}`,
         `version: 1.0`,
         `date_created: ${dateStr}`,
-        `owner: oh-my-opencode-slim`,
-        `tags: [spec, diagnostic, omo-slim]`,
+        `owner: ${owner}`,
+        `tags: [${tags.join(', ')}]`,
         '---',
         '',
       ].join('\n')

+ 21 - 0
src/interview/manager.ts

@@ -422,6 +422,8 @@ export function createInterviewManager(
             // 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 = dashboard.consumePendingAnswers(interviewId);
@@ -440,6 +442,25 @@ export function createInterviewManager(
               });
               await service.handleNudgeAction(interviewId, nudge);
             }
+            const comment = dashboard.consumeBlockComment(interviewId);
+            if (comment) {
+              log('[interview] delivering block comment (in-process)', {
+                interviewId,
+                section: comment.section,
+              });
+              await service.submitBlockComment(
+                interviewId,
+                comment.section,
+                comment.comment,
+              );
+            }
+            const chat = dashboard.consumeChatMessage(interviewId);
+            if (chat) {
+              log('[interview] delivering chat message (in-process)', {
+                interviewId,
+              });
+              await service.submitChat(interviewId, chat);
+            }
           }
 
           // Refresh state: calls getInterviewState → syncInterview → onStateChange

+ 19 - 0
src/interview/parser.test.ts

@@ -168,6 +168,25 @@ describe('parseAssistantState', () => {
 
     expect(result.state?.questions).toHaveLength(5);
   });
+
+  test('repairs unescaped newlines inside strings and handles backslash escapes correctly', () => {
+    // Let's use raw unescaped newlines inside the JSON string to test the parser's repair:
+    const textWithLiteralNewlines = [
+      '<interview_state>',
+      '{',
+      '  "summary": "This is a summary',
+      'with a newline and escaped \\"quotes\\" and \\\\ backslash.",',
+      '  "questions": []',
+      '}',
+      '</interview_state>',
+    ].join('\n');
+    const result = parseAssistantState(textWithLiteralNewlines);
+
+    expect(result.state).not.toBeNull();
+    expect(result.state?.summary).toBe(
+      'This is a summary\nwith a newline and escaped "quotes" and \\ backslash.',
+    );
+  });
 });
 
 describe('flattenMessage', () => {

+ 34 - 12
src/interview/parser.ts

@@ -46,6 +46,39 @@ function normalizeQuestion(
   };
 }
 
+function repairJsonNewlines(json: string): string {
+  let result = '';
+  let inString = false;
+  let escaped = false;
+  for (let i = 0; i < json.length; i++) {
+    const char = json[i];
+    if (inString) {
+      if (escaped) {
+        result += char;
+        escaped = false;
+      } else if (char === '\\') {
+        result += char;
+        escaped = true;
+      } else if (char === '"') {
+        result += char;
+        inString = false;
+      } else if (char === '\n') {
+        result += '\\n';
+      } else if (char === '\r') {
+        result += '\\r';
+      } else {
+        result += char;
+      }
+    } else {
+      if (char === '"') {
+        inString = true;
+      }
+      result += char;
+    }
+  }
+  return result;
+}
+
 export function flattenMessage(message: InterviewMessage): string {
   return (message.parts ?? [])
     .map((part) => part.text ?? '')
@@ -92,18 +125,7 @@ export function parseAssistantState(
     JSON.parse(rawJson);
   } catch {
     // Try to normalize literal newlines inside string values:
-    rawJson = rawJson.replace(
-      /:[ \t]*"([\s\S]*?)"([ \t]*[,}])/g,
-      (_m, content, suffix) => {
-        // Escape real newlines and backslash escapes in content
-        const escaped = content
-          .replace(/\\/g, '\\\\')
-          .replace(/\n/g, '\\n')
-          .replace(/\r/g, '\\r')
-          .replace(/"/g, '\\"');
-        return `: "${escaped}"${suffix}`;
-      },
-    );
+    rawJson = repairJsonNewlines(rawJson);
   }
 
   try {

+ 9 - 4
src/interview/service.ts

@@ -642,11 +642,16 @@ export function createInterviewService(
         // Sync state on busy → idle so the browser gets the final push
         if (wasBusy && status?.type !== 'busy') {
           const activeId = activeInterviewIds.get(sessionID);
-          const interview = activeId
-            ? interviewsById.get(activeId)
-            : null;
+          const interview = activeId ? interviewsById.get(activeId) : null;
           if (interview && interview.status === 'active') {
-            syncInterview(interview).catch(() => {});
+            syncInterview(interview).catch((err) => {
+              log(
+                '[interview] failed to sync interview state in event handler:',
+                {
+                  error: err instanceof Error ? err.message : String(err),
+                },
+              );
+            });
           }
         }
       }

+ 25 - 0
src/interview/ui.ts

@@ -1849,6 +1849,17 @@ export function renderInterviewPage(
         updateSubmitButton();
         updateTocSidebar(data);
         updateChatPanel(data);
+
+        // If we transitioned to a non-terminal state and connection/polling is stopped, restart them
+        const terminalModes = ['abandoned', 'completed', 'session-disconnected'];
+        if (!terminalModes.includes(data.mode)) {
+          if (!sseConnected && !activeEs) {
+            connectSse();
+          }
+          if (!sseConnected && !pollFallbackTimer) {
+            schedulePoll();
+          }
+        }
       }
 
       async function refresh() {
@@ -1962,10 +1973,15 @@ export function renderInterviewPage(
       // ── Real-time updates via SSE ─────────────────────────────────
       let sseConnected = false;
       let pollFallbackTimer = null;
+      let activeEs = null;
 
       function connectSse() {
+        if (activeEs) {
+          activeEs.close();
+        }
         const sseUrl = '/api/interviews/' + encodeURIComponent(interviewId) + '/events';
         const es = new EventSource(sseUrl);
+        activeEs = es;
 
         es.addEventListener('state', (e) => {
           sseConnected = true;
@@ -1982,6 +1998,11 @@ export function renderInterviewPage(
         es.onerror = () => {
           sseConnected = false;
           es.close();
+          activeEs = null;
+          const terminalModes = ['abandoned', 'completed', 'session-disconnected'];
+          if (state.data && terminalModes.includes(state.data.mode)) {
+            return;
+          }
           // Retry SSE after 3s, fall back to polling in the meantime
           if (!pollFallbackTimer) schedulePoll();
           setTimeout(() => connectSse(), 3000);
@@ -1991,6 +2012,10 @@ export function renderInterviewPage(
       function schedulePoll() {
         // Only poll if SSE is not connected
         if (sseConnected) return;
+        const terminalModes = ['abandoned', 'completed', 'session-disconnected'];
+        if (state.data && terminalModes.includes(state.data.mode)) {
+          return;
+        }
         pollFallbackTimer = setTimeout(async () => {
           try { await refresh(); } catch (_) {}
           pollFallbackTimer = null;