Revert "fix: handle inverse race condition where API user message is before clineMessage"

daniel-lxs · daniel-lxs · commit f6096b173da7 · 2025-12-15T20:10:21.000-05:00
This reverts commit acc5567.
diff --git a/src/core/message-manager/index.spec.ts b/src/core/message-manager/index.spec.ts
@@ -848,61 +848,5 @@ describe("MessageManager", () => {
 			expect(apiCall[2].ts).toBe(200)
 			expect(apiCall[2].role).toBe("assistant")
 		})
-
-		it("should handle inverse race condition where user API message is BEFORE clineMessage timestamp", async () => {
-			// This test covers the edge case where the API user message (tool_result)
-			// is logged BEFORE the clineMessage timestamp due to inverse timing.
-			//
-			// Timeline (inverse race condition scenario):
-			// - T1 (50):  API user (initial request) - Turn 1
-			// - T2 (100): API assistant (response) - Turn 1
-			// - T3 (120): API user (tool_result for Turn 2, logged early due to race) - Turn 2, SHOULD DELETE
-			// - T4 (150): clineMessage "user_feedback" being deleted - Turn 2
-			// - T5 (180): API assistant (response to Turn 2) - Turn 2, SHOULD DELETE
-			// - T6 (250): API user (Turn 3) - Turn 3, SHOULD KEEP
-			//
-			// When deleting the clineMessage at T4=150, we should:
-			// - Keep Turn 1 (ts=50, ts=100)
-			// - Remove Turn 2's API user at ts=120 (even though it's before clineMessage!)
-			// - Remove Turn 2's assistant at ts=180
-			// - Keep Turn 3 (ts=250)
-			//
-			// The current fix only looks for user messages at or AFTER cutoff,
-			// so it would incorrectly skip the user at ts=120 and find ts=250 instead.
-
-			mockTask.clineMessages = [
-				{ ts: 50, say: "user", text: "Initial request" },
-				{ ts: 150, say: "user_feedback", text: "Turn 2 feedback" }, // Being deleted
-				{ ts: 250, say: "user", text: "Turn 3" },
-			]
-
-			mockTask.apiConversationHistory = [
-				{ ts: 50, role: "user", content: [{ type: "text", text: "Initial request" }] },
-				{ ts: 100, role: "assistant", content: [{ type: "text", text: "Response 1" }] },
-				{
-					ts: 120, // Inverse race: logged BEFORE clineMessage at ts=150!
-					role: "user",
-					content: [{ type: "tool_result", tool_use_id: "tool_1", content: "Turn 2 feedback" }],
-				},
-				{
-					ts: 180, // After clineMessage
-					role: "assistant",
-					content: [{ type: "text", text: "Response to Turn 2" }],
-				},
-				{ ts: 250, role: "user", content: [{ type: "text", text: "Turn 3" }] },
-			]
-
-			// Delete the user_feedback clineMessage at ts=150
-			await manager.rewindToTimestamp(150)
-
-			// Expected: Keep Turn 1 (ts=50, ts=100), remove Turn 2 (ts=120, ts=180)
-			// The API user at ts=120 should be removed even though it's before the clineMessage
-			const apiCall = mockTask.overwriteApiConversationHistory.mock.calls[0][0]
-			expect(apiCall).toHaveLength(2)
-			expect(apiCall[0].ts).toBe(50) // Turn 1 user - kept
-			expect(apiCall[1].ts).toBe(100) // Turn 1 assistant - kept
-			// ts=120 (Turn 2 user) should be removed
-			// ts=180 (Turn 2 assistant) should be removed
-		})
 	})
 })
diff --git a/src/core/message-manager/index.ts b/src/core/message-manager/index.ts
@@ -136,15 +136,11 @@ export class MessageManager {
 	 *
 	 * Note on timestamp handling:
 	 * Due to async execution during streaming, clineMessage timestamps may not
-	 * perfectly align with API message timestamps. There are two race condition scenarios:
-	 *
-	 * 1. Original race: clineMessage timestamp is BEFORE the assistant API message
-	 *    (tool execution happens concurrently with stream completion)
-	 *    Solution: Find the first API user message at or after the cutoff
-	 *
-	 * 2. Inverse race: API user message (tool_result) timestamp is BEFORE the clineMessage
-	 *    (the tool_result was logged before the user_feedback clineMessage)
-	 *    Solution: Find the last assistant message before cutoff and use that as boundary
+	 * perfectly align with API message timestamps. Specifically, a "user_feedback"
+	 * clineMessage can have a timestamp BEFORE the assistant API message that
+	 * triggered it (because tool execution happens concurrently with stream
+	 * completion). To handle this race condition, we find the first API user
+	 * message at or after the cutoff and use its timestamp as the actual boundary.
 	 */
 	private async truncateApiHistoryWithCleanup(
 		cutoffTs: number,
@@ -163,35 +159,20 @@ export class MessageManager {
 		let actualCutoff: number = cutoffTs
 
 		if (!hasExactMatch && hasMessageBeforeCutoff) {
-			// No exact match but there are earlier messages - check for race conditions
-			//
-			// First, check for "inverse race" pattern:
-			// Find the last assistant message before cutoff
-			const lastAssistantBeforeCutoff = this.findLastAssistantBeforeCutoff(apiHistory, cutoffTs)
-
-			if (lastAssistantBeforeCutoff !== undefined) {
-				// Check if there are user messages between the last assistant and cutoff
-				// This indicates an "inverse race" where the user's tool_result was logged
-				// before the clineMessage timestamp
-				const hasUserBetweenAssistantAndCutoff = apiHistory.some(
-					(m) =>
-						m.ts !== undefined && m.ts > lastAssistantBeforeCutoff && m.ts < cutoffTs && m.role === "user",
-				)
-
-				if (hasUserBetweenAssistantAndCutoff) {
-					// Inverse race detected: use the assistant timestamp + 1 as cutoff
-					// This ensures we keep the assistant response but remove the user
-					// message that belongs to the turn being deleted
-					actualCutoff = lastAssistantBeforeCutoff + 1
-				} else {
-					// No inverse race, check for original race condition
-					// Look for the first API user message at or after the cutoff
-					actualCutoff = this.findFirstUserCutoff(apiHistory, cutoffTs)
-				}
-			} else {
-				// No assistant before cutoff, use original race condition logic
-				actualCutoff = this.findFirstUserCutoff(apiHistory, cutoffTs)
+			// No exact match but there are earlier messages means we might have a race
+			// condition where the clineMessage timestamp is earlier than any API message
+			// due to async execution. In this case, look for the first API user message
+			// at or after the cutoff to use as the actual boundary.
+			// This ensures assistant messages that preceded the user's response are preserved.
+			const firstUserMsgIndexToRemove = apiHistory.findIndex(
+				(m) => m.ts !== undefined && m.ts >= cutoffTs && m.role === "user",
+			)
+
+			if (firstUserMsgIndexToRemove !== -1) {
+				// Use the user message's timestamp as the actual cutoff
+				actualCutoff = apiHistory[firstUserMsgIndexToRemove].ts!
 			}
+			// else: no user message found, use original cutoffTs (fallback)
 		}
 
 		// Step 2: Filter by the actual cutoff timestamp
@@ -234,38 +215,4 @@ export class MessageManager {
 			await this.task.overwriteApiConversationHistory(apiHistory)
 		}
 	}
-
-	/**
-	 * Find the timestamp of the last assistant message before the cutoff.
-	 * Returns undefined if no assistant message exists before cutoff.
-	 */
-	private findLastAssistantBeforeCutoff(apiHistory: ApiMessage[], cutoffTs: number): number | undefined {
-		let lastAssistantTs: number | undefined
-
-		for (const msg of apiHistory) {
-			if (msg.ts !== undefined && msg.ts < cutoffTs && msg.role === "assistant") {
-				if (lastAssistantTs === undefined || msg.ts > lastAssistantTs) {
-					lastAssistantTs = msg.ts
-				}
-			}
-		}
-
-		return lastAssistantTs
-	}
-
-	/**
-	 * Find the cutoff based on the first user message at or after the original cutoff.
-	 * Falls back to the original cutoff if no user message is found.
-	 */
-	private findFirstUserCutoff(apiHistory: ApiMessage[], cutoffTs: number): number {
-		const firstUserMsgIndex = apiHistory.findIndex(
-			(m) => m.ts !== undefined && m.ts >= cutoffTs && m.role === "user",
-		)
-
-		if (firstUserMsgIndex !== -1) {
-			return apiHistory[firstUserMsgIndex].ts!
-		}
-
-		return cutoffTs
-	}
 }
