import { parseUnifiedDiff } from "./parse-diff";
import type { ReviewComment, ReviewSummary } from "./types";

const RAW_DIFF = `diff --git a/src/agent/output-handler.ts b/src/agent/output-handler.ts
index 7aeabe5..7ca616a 100644
--- a/src/agent/output-handler.ts
+++ b/src/agent/output-handler.ts
@@ -74,6 +74,8 @@ interface ClaudeCliResult {
 }

 export class OutputHandler {
+  private filePositions = new Map<string, number>();
+
   constructor(
     private repository: AgentRepository,
     private eventBus?: EventBus,
@@ -101,6 +103,43 @@ export class OutputHandler {
     }
   }

+  /**
+   * Read complete lines from a file, avoiding partial lines that might still be writing.
+   * This eliminates race conditions when agents are still writing output.
+   */
+  private async readCompleteLines(filePath: string, fromPosition: number = 0): Promise<{ content: string; lastPosition: number }> {
+    try {
+      const fs = await import('node:fs/promises');
+      const content = await fs.readFile(filePath, 'utf-8');
+
+      if (fromPosition >= content.length) {
+        return { content: '', lastPosition: fromPosition };
+      }
+
+      // Get content from our last read position
+      const newContent = content.slice(fromPosition);
+
+      // Split into lines
+      const lines = newContent.split('\\n');
+
+      // If file doesn't end with newline, last element is potentially incomplete
+      // Only process complete lines (all but the last, unless file ends with \\n)
+      const hasTrailingNewline = newContent.endsWith('\\n');
+      const completeLines = hasTrailingNewline ? lines : lines.slice(0, -1);
+
+      // Calculate new position (only count complete lines)
+      const completeLinesContent = completeLines.join('\\n') + (completeLines.length > 0 && hasTrailingNewline ? '\\n' : '');
+      const newPosition = fromPosition + Buffer.byteLength(completeLinesContent, 'utf-8');
+
+      return {
+        content: completeLinesContent,
+        lastPosition: newPosition
+      };
+    } catch {
+      return { content: '', lastPosition: fromPosition };
+    }
+  }
+
   /**
    * Handle a standardized stream event from a parser.
    */
@@ -213,12 +252,27 @@ export class OutputHandler {
     if (!signalText) {
       try {
         const outputFilePath = active?.outputFilePath ?? '';
-        if (outputFilePath && await this.validateSignalFile(outputFilePath)) {
-          const fileContent = await readFile(outputFilePath, 'utf-8');
+        if (outputFilePath) {
+          // Read only complete lines from the file, avoiding race conditions
+          const lastPosition = this.filePositions.get(agentId) || 0;
+          const { content: fileContent, lastPosition: newPosition } = await this.readCompleteLines(outputFilePath, lastPosition);
+
           if (fileContent.trim()) {
+            this.filePositions.set(agentId, newPosition);
             await this.processAgentOutput(agentId, fileContent, provider, getAgentWorkdir);
             return;
           }
+
+          // If no new complete lines, but file might still be writing, try again with validation
+          if (await this.validateSignalFile(outputFilePath)) {
+            const fullContent = await readFile(outputFilePath, 'utf-8');
+            if (fullContent.trim() && fullContent.length > newPosition) {
+              // File is complete and has content beyond what we've read
+              this.filePositions.delete(agentId); // Clean up tracking
+              await this.processAgentOutput(agentId, fullContent, provider, getAgentWorkdir);
+              return;
+            }
+          }
         }
       } catch { /* file empty or missing */ }

diff --git a/src/agent/manager.ts b/src/agent/manager.ts
index a1b2c3d..e4f5g6h 100644
--- a/src/agent/manager.ts
+++ b/src/agent/manager.ts
@@ -145,6 +145,18 @@ export class MultiProviderAgentManager {
     return agent;
   }

+  /**
+   * Check if an agent has a valid completion signal that indicates
+   * it finished successfully, even if process monitoring missed it.
+   */
+  private async checkSignalCompletion(agent: AgentInfo): Promise<boolean> {
+    const signalPath = this.getSignalPath(agent);
+    if (!signalPath) return false;
+    const signal = await this.readSignalFile(signalPath);
+    if (!signal) return false;
+    return ['done', 'questions', 'error'].includes(signal.status);
+  }
+
   /**
    * Reconcile agent states after a server restart.
    * Checks which agents are still alive and updates their status.
@@ -160,8 +172,16 @@ export class MultiProviderAgentManager {
       if (isAlive) {
         this.monitorAgent(agent);
       } else {
-        // Agent process is gone — mark as crashed
-        await this.outputHandler.handleAgentError(agent.id, 'Agent process terminated unexpectedly');
+        // Agent process is gone — check signal before marking as crashed
+        const hasValidSignal = await this.checkSignalCompletion(agent);
+        if (hasValidSignal) {
+          // Agent completed normally but we missed the signal
+          this.logger.info({ agentId: agent.id }, 'Agent has valid completion signal, processing...');
+          await this.outputHandler.handleCompletion(agent.id, agent.provider);
+        } else {
+          // Truly crashed
+          await this.outputHandler.handleAgentError(agent.id, 'Agent process terminated unexpectedly');
+        }
       }
     }
   }
`;

const DUMMY_COMMENTS: ReviewComment[] = [
  {
    id: "c1",
    filePath: "src/agent/output-handler.ts",
    lineNumber: 77,
    lineType: "added",
    body: "Consider using a WeakMap here to avoid memory leaks if agent references are garbage collected. Though since we clean up in handleAgentError and completion, this is probably fine.",
    author: "agent:review-bot",
    createdAt: "2026-02-09T10:30:00Z",
    resolved: false,
  },
  {
    id: "c2",
    filePath: "src/agent/output-handler.ts",
    lineNumber: 112,
    lineType: "added",
    body: "Dynamic import of fs/promises on every call is wasteful. This should be a top-level import since it's a Node built-in and always available.",
    author: "agent:review-bot",
    createdAt: "2026-02-09T10:30:00Z",
    resolved: false,
  },
  {
    id: "c3",
    filePath: "src/agent/output-handler.ts",
    lineNumber: 131,
    lineType: "added",
    body: "Bug: `Buffer.byteLength` gives byte length but `content.slice()` works on character offsets. If the file contains multi-byte characters, the position tracking will drift. Use character length consistently, or switch to byte-based reads with `fs.read()`.",
    author: "agent:review-bot",
    createdAt: "2026-02-09T10:31:00Z",
    resolved: false,
  },
  {
    id: "c4",
    filePath: "src/agent/manager.ts",
    lineNumber: 156,
    lineType: "added",
    body: "Good approach. Checking the signal file before marking as crashed eliminates the race condition where the process exits before we can read its output.",
    author: "agent:review-bot",
    createdAt: "2026-02-09T10:32:00Z",
    resolved: true,
  },
  {
    id: "c5",
    filePath: "src/agent/manager.ts",
    lineNumber: 180,
    lineType: "added",
    body: "The log message says 'processing...' but doesn't indicate what processing means. Should clarify that this triggers handleCompletion which will update the agent's status based on the signal content.",
    author: "agent:review-bot",
    createdAt: "2026-02-09T10:33:00Z",
    resolved: false,
  },
];

const files = parseUnifiedDiff(RAW_DIFF);

export const DUMMY_REVIEW: ReviewSummary = {
  id: "review-1",
  title: "fix(agent): Eliminate race condition in completion handling",
  description:
    "Introduces incremental file position tracking to avoid reading partial lines from agent output files. Also adds signal.json checking during reconciliation to prevent false crash marking when agents complete between process checks.",
  author: "agent:slim-wildebeest",
  status: "pending",
  comments: DUMMY_COMMENTS,
  files,
  createdAt: "2026-02-09T10:00:00Z",
  sourceBranch: "fix/completion-race-condition",
  targetBranch: "main",
};
