[Condense] Fix bug where bedrock requires a user message first

src/core/condense/__tests__/index.test.ts

@@ -5,6 +5,7 @@ import { describe, expect, it, jest, beforeEach } from "@jest/globals"
 import { TelemetryService } from "@roo-code/telemetry"
 
 import { ApiHandler } from "../../../api"
+import { AwsBedrockHandler } from "../../../api/providers"
 import { ApiMessage } from "../../task-persistence/apiMessages"
 import { maybeRemoveImageBlocks } from "../../../api/transform/image-cleaning"
 import { summarizeConversation, getMessagesSinceLastSummary, N_MESSAGES_TO_KEEP } from "../index"
@@ -25,14 +26,20 @@ const taskId = "test-task-id"
 const DEFAULT_PREV_CONTEXT_TOKENS = 1000
 
 describe("getMessagesSinceLastSummary", () => {
+	let mockApiHandler: ApiHandler
+
+	beforeEach(() => {
+		mockApiHandler = {} as unknown as ApiHandler
+	})
+
 	it("should return all messages when there is no summary", () => {
 		const messages: ApiMessage[] = [
 			{ role: "user", content: "Hello", ts: 1 },
 			{ role: "assistant", content: "Hi there", ts: 2 },
 			{ role: "user", content: "How are you?", ts: 3 },
 		]
 
-		const result = getMessagesSinceLastSummary(messages)
+		const result = getMessagesSinceLastSummary(messages, mockApiHandler)
 		expect(result).toEqual(messages)
 	})
 
@@ -45,7 +52,7 @@ describe("getMessagesSinceLastSummary", () => {
 			{ role: "assistant", content: "I'm good", ts: 5 },
 		]
 
-		const result = getMessagesSinceLastSummary(messages)
+		const result = getMessagesSinceLastSummary(messages, mockApiHandler)
 		expect(result).toEqual([
 			{ role: "assistant", content: "Summary of conversation", ts: 3, isSummary: true },
 			{ role: "user", content: "How are you?", ts: 4 },
@@ -62,17 +69,63 @@ describe("getMessagesSinceLastSummary", () => {
 			{ role: "user", content: "What's new?", ts: 5 },
 		]
 
-		const result = getMessagesSinceLastSummary(messages)
+		const result = getMessagesSinceLastSummary(messages, mockApiHandler)
 		expect(result).toEqual([
 			{ role: "assistant", content: "Second summary", ts: 4, isSummary: true },
 			{ role: "user", content: "What's new?", ts: 5 },
 		])
 	})
 
 	it("should handle empty messages array", () => {
-		const result = getMessagesSinceLastSummary([])
+		const result = getMessagesSinceLastSummary([], mockApiHandler)
 		expect(result).toEqual([])
 	})
+
+	it("should prepend user message when using AwsBedrockHandler with summary as first message", () => {
+		const mockAwsBedrockHandler = new AwsBedrockHandler({
+			apiModelId: "anthropic.claude-3-5-sonnet-20241022-v2:0",
+			awsAccessKey: "test-key",
+			awsSecretKey: "test-secret",
+			awsRegion: "us-east-1",
+		})
+
+		const messages: ApiMessage[] = [
+			{ role: "user", content: "Hello", ts: 1 },
+			{ role: "assistant", content: "Hi there", ts: 2 },
+			{ role: "assistant", content: "Summary of conversation", ts: 1000, isSummary: true },
+			{ role: "user", content: "How are you?", ts: 1001 },
+			{ role: "assistant", content: "I'm good", ts: 1002 },
+		]
+
+		const result = getMessagesSinceLastSummary(messages, mockAwsBedrockHandler)
+
+		// Should prepend user message before the summary
+		expect(result).toEqual([
+			{ role: "user", content: "Please continue from the following summary:", ts: 999 },
+			{ role: "assistant", content: "Summary of conversation", ts: 1000, isSummary: true },
+			{ role: "user", content: "How are you?", ts: 1001 },
+			{ role: "assistant", content: "I'm good", ts: 1002 },
+		])
+	})
+
+	it("should not prepend user message when using non-AwsBedrockHandler", () => {
+		const messages: ApiMessage[] = [
+			{ role: "user", content: "Hello", ts: 1 },
+			{ role: "assistant", content: "Hi there", ts: 2 },
+			{ role: "assistant", content: "Summary of conversation", ts: 1000, isSummary: true },
+			{ role: "user", content: "How are you?", ts: 1001 },
+			{ role: "assistant", content: "I'm good", ts: 1002 },
+		]
+
+		const result = getMessagesSinceLastSummary(messages, mockApiHandler)
+
+		// Should not prepend user message for non-AwsBedrockHandler
+		expect(result).toEqual([
+			{ role: "assistant", content: "Summary of conversation", ts: 1000, isSummary: true },
+			{ role: "user", content: "How are you?", ts: 1001 },
+			{ role: "assistant", content: "I'm good", ts: 1002 },
+		])
+	})
 })
 
 describe("summarizeConversation", () => {

src/core/condense/index.ts

@@ -6,6 +6,7 @@ import { t } from "../../i18n"
 import { ApiHandler } from "../../api"
 import { ApiMessage } from "../task-persistence/apiMessages"
 import { maybeRemoveImageBlocks } from "../../api/transform/image-cleaning"
+import { AwsBedrockHandler } from "../../api/providers"
 
 export const N_MESSAGES_TO_KEEP = 3
 
@@ -98,7 +99,30 @@ export async function summarizeConversation(
 	)
 
 	const response: SummarizeResponse = { messages, cost: 0, summary: "" }
-	const messagesToSummarize = getMessagesSinceLastSummary(messages.slice(0, -N_MESSAGES_TO_KEEP))
+
+	// Use condensing API handler if provided, otherwise use main API handler
+	let handlerToUse = condensingApiHandler || apiHandler
+
+	// Check if the chosen handler supports the required functionality
+	if (!handlerToUse || typeof handlerToUse.createMessage !== "function") {
+		console.warn(
+			"Chosen API handler for condensing does not support message creation or is invalid, falling back to main apiHandler.",
+		)
+
+		handlerToUse = apiHandler // Fallback to the main, presumably valid, apiHandler
+
+		// Ensure the main apiHandler itself is valid before this point or add another check.
+		if (!handlerToUse || typeof handlerToUse.createMessage !== "function") {
+			// This case should ideally not happen if main apiHandler is always valid.
+			// Consider throwing an error or returning a specific error response.
+			console.error("Main API handler is also invalid for condensing. Cannot proceed.")
+			// Return an appropriate error structure for SummarizeResponse
+			const error = t("common:errors.condense_handler_invalid")
+			return { ...response, error }
+		}
+	}
+
+	const messagesToSummarize = getMessagesSinceLastSummary(messages.slice(0, -N_MESSAGES_TO_KEEP), handlerToUse)
 
 	if (messagesToSummarize.length <= 1) {
 		const error =
@@ -126,32 +150,10 @@ export async function summarizeConversation(
 		({ role, content }) => ({ role, content }),
 	)
 
-	// Note: this doesn't need to be a stream, consider using something like apiHandler.completePrompt
 	// Use custom prompt if provided and non-empty, otherwise use the default SUMMARY_PROMPT
 	const promptToUse = customCondensingPrompt?.trim() ? customCondensingPrompt.trim() : SUMMARY_PROMPT
 
-	// Use condensing API handler if provided, otherwise use main API handler
-	let handlerToUse = condensingApiHandler || apiHandler
-
-	// Check if the chosen handler supports the required functionality
-	if (!handlerToUse || typeof handlerToUse.createMessage !== "function") {
-		console.warn(
-			"Chosen API handler for condensing does not support message creation or is invalid, falling back to main apiHandler.",
-		)
-
-		handlerToUse = apiHandler // Fallback to the main, presumably valid, apiHandler
-
-		// Ensure the main apiHandler itself is valid before this point or add another check.
-		if (!handlerToUse || typeof handlerToUse.createMessage !== "function") {
-			// This case should ideally not happen if main apiHandler is always valid.
-			// Consider throwing an error or returning a specific error response.
-			console.error("Main API handler is also invalid for condensing. Cannot proceed.")
-			// Return an appropriate error structure for SummarizeResponse
-			const error = t("common:errors.condense_handler_invalid")
-			return { ...response, error }
-		}
-	}
-
+	// Note: this doesn't need to be a stream, consider using something like apiHandler.completePrompt
 	const stream = handlerToUse.createMessage(promptToUse, requestMessages)
 
 	let summary = ""
@@ -205,13 +207,28 @@ export async function summarizeConversation(
 }
 
 /* Returns the list of all messages since the last summary message, including the summary. Returns all messages if there is no summary. */
-export function getMessagesSinceLastSummary(messages: ApiMessage[]): ApiMessage[] {
+export function getMessagesSinceLastSummary(messages: ApiMessage[], apiHandler: ApiHandler): ApiMessage[] {
 	let lastSummaryIndexReverse = [...messages].reverse().findIndex((message) => message.isSummary)
 
 	if (lastSummaryIndexReverse === -1) {
 		return messages
 	}
 
 	const lastSummaryIndex = messages.length - lastSummaryIndexReverse - 1
-	return messages.slice(lastSummaryIndex)
+	const messagesSinceSummary = messages.slice(lastSummaryIndex)
+	return maybePrependUserMessage(messagesSinceSummary, apiHandler)
+}
+
+function maybePrependUserMessage(messages: ApiMessage[], apiHandler: ApiHandler): ApiMessage[] {
+	if (messages.length === 0 || !messages[0].isSummary || !(apiHandler instanceof AwsBedrockHandler)) {
+		return messages
+	}
+	// Bedrock requires the first message to be a user message.
+	// See https://github.com/RooCodeInc/Roo-Code/issues/4147
+	const userMessage: ApiMessage = {
+		role: "user",
+		content: "Please continue from the following summary:",
+		ts: messages[0]?.ts ? messages[0].ts - 1 : Date.now(),
+	}
+	return [userMessage, ...messages]
 }

src/core/task/Task.ts

@@ -1650,7 +1650,7 @@ export class Task extends EventEmitter<ClineEvents> {
 			}
 		}
 
-		const messagesSinceLastSummary = getMessagesSinceLastSummary(this.apiConversationHistory)
+		const messagesSinceLastSummary = getMessagesSinceLastSummary(this.apiConversationHistory, this.api)
 		const cleanConversationHistory = maybeRemoveImageBlocks(messagesSinceLastSummary, this.api).map(
 			({ role, content }) => ({ role, content }),
 		)