Fix saving of OpenAI compatible headers (RooCodeInc#3415)

* Fix saving of OpenAI compatible headers

* Code cleanup

* Add test

Author: mrubens

webview-ui/src/components/settings/ApiOptions.tsx

@@ -1,4 +1,5 @@
 import React, { memo, useCallback, useEffect, useMemo, useState } from "react"
+import { convertHeadersToObject } from "./utils/headers"
 import { useDebounce } from "react-use"
 import { VSCodeLink } from "@vscode/webview-ui-toolkit/react"
 
@@ -86,25 +87,6 @@ const ApiOptions = ({
 	}, [apiConfiguration?.openAiHeaders, customHeaders])
 
 	// Helper to convert array of tuples to object (filtering out empty keys).
-	const convertHeadersToObject = (headers: [string, string][]): Record<string, string> => {
-		const result: Record<string, string> = {}
-
-		// Process each header tuple.
-		for (const [key, value] of headers) {
-			const trimmedKey = key.trim()
-
-			// Skip empty keys.
-			if (!trimmedKey) {
-				continue
-			}
-
-			// For duplicates, the last one in the array wins.
-			// This matches how HTTP headers work in general.
-			result[trimmedKey] = value.trim()
-		}
-
-		return result
-	}
 
 	// Debounced effect to update the main configuration when local
 	// customHeaders state stabilizes.

webview-ui/src/components/settings/providers/OpenAICompatible.tsx

@@ -1,7 +1,8 @@
-import { useState, useCallback } from "react"
+import { useState, useCallback, useEffect } from "react"
 import { useEvent } from "react-use"
 import { Checkbox } from "vscrui"
 import { VSCodeButton, VSCodeTextField } from "@vscode/webview-ui-toolkit/react"
+import { convertHeadersToObject } from "../utils/headers"
 
 import { ModelInfo, ReasoningEffort as ReasoningEffortType } from "@roo/schemas"
 import { ProviderSettings, azureOpenAiDefaultApiVersion, openAiModelInfoSaneDefaults } from "@roo/shared/api"
@@ -67,6 +68,18 @@ export const OpenAICompatible = ({ apiConfiguration, setApiConfigurationField }:
 		setCustomHeaders((prev) => prev.filter((_, i) => i !== index))
 	}, [])
 
+	// Helper to convert array of tuples to object
+
+	// Add effect to update the parent component's state when local headers change
+	useEffect(() => {
+		const timer = setTimeout(() => {
+			const headerObject = convertHeadersToObject(customHeaders)
+			setApiConfigurationField("openAiHeaders", headerObject)
+		}, 300)
+
+		return () => clearTimeout(timer)
+	}, [customHeaders, setApiConfigurationField])
+
 	const handleInputChange = useCallback(
 		<K extends keyof ProviderSettings, E>(
 			field: K,

webview-ui/src/components/settings/utils/__tests__/headers.test.ts

@@ -0,0 +1,122 @@
+import { convertHeadersToObject } from "../headers"
+
+describe("convertHeadersToObject", () => {
+	it("should convert headers array to object", () => {
+		const headers: [string, string][] = [
+			["Content-Type", "application/json"],
+			["Authorization", "Bearer token123"],
+		]
+
+		const result = convertHeadersToObject(headers)
+
+		expect(result).toEqual({
+			"Content-Type": "application/json",
+			Authorization: "Bearer token123",
+		})
+	})
+
+	it("should trim whitespace from keys and values", () => {
+		const headers: [string, string][] = [
+			["  Content-Type  ", "  application/json  "],
+			["  Authorization  ", "  Bearer token123  "],
+		]
+
+		const result = convertHeadersToObject(headers)
+
+		expect(result).toEqual({
+			"Content-Type": "application/json",
+			Authorization: "Bearer token123",
+		})
+	})
+
+	it("should handle empty headers array", () => {
+		const headers: [string, string][] = []
+
+		const result = convertHeadersToObject(headers)
+
+		expect(result).toEqual({})
+	})
+
+	it("should skip headers with empty keys", () => {
+		const headers: [string, string][] = [
+			["Content-Type", "application/json"],
+			["", "This value should be skipped"],
+			["  ", "This value should also be skipped"],
+			["Authorization", "Bearer token123"],
+		]
+
+		const result = convertHeadersToObject(headers)
+
+		expect(result).toEqual({
+			"Content-Type": "application/json",
+			Authorization: "Bearer token123",
+		})
+
+		// Specifically verify empty keys are not present
+		expect(result[""]).toBeUndefined()
+		expect(result["  "]).toBeUndefined()
+	})
+
+	it("should use last occurrence when handling duplicate keys", () => {
+		const headers: [string, string][] = [
+			["Content-Type", "application/json"],
+			["Authorization", "Bearer token123"],
+			["Content-Type", "text/plain"], // Duplicate key - should override previous value
+			["Content-Type", "application/xml"], // Another duplicate - should override again
+		]
+
+		const result = convertHeadersToObject(headers)
+
+		// Verify the last value for "Content-Type" is used
+		expect(result["Content-Type"]).toBe("application/xml")
+		expect(result).toEqual({
+			"Content-Type": "application/xml",
+			Authorization: "Bearer token123",
+		})
+	})
+
+	it("should preserve case sensitivity while trimming keys", () => {
+		const headers: [string, string][] = [
+			[" Content-Type", "application/json"],
+			["content-type ", "text/plain"], // Different casing (lowercase) with spacing
+		]
+
+		const result = convertHeadersToObject(headers)
+
+		// Keys should be trimmed but case sensitivity preserved
+		// JavaScript object keys are case-sensitive
+		expect(Object.keys(result)).toHaveLength(2)
+		expect(result["Content-Type"]).toBe("application/json")
+		expect(result["content-type"]).toBe("text/plain")
+	})
+
+	it("should handle empty values", () => {
+		const headers: [string, string][] = [
+			["Empty-Value", ""],
+			["Whitespace-Value", "   "],
+		]
+
+		const result = convertHeadersToObject(headers)
+
+		// Empty values should be included but trimmed
+		expect(result["Empty-Value"]).toBe("")
+		expect(result["Whitespace-Value"]).toBe("")
+	})
+
+	it("should handle complex duplicate key scenarios with mixed casing and spacing", () => {
+		const headers: [string, string][] = [
+			["content-type", "application/json"], // Original entry
+			["  Content-Type  ", "text/html"], // Different case with spacing
+			["content-type", "application/xml"], // Same case as first, should override it
+			["Content-Type", "text/plain"], // Same case as second, should override it
+		]
+
+		const result = convertHeadersToObject(headers)
+
+		// JavaScript object keys are case-sensitive
+		// We should have two keys with different cases, each with the last value
+		expect(Object.keys(result).sort()).toEqual(["Content-Type", "content-type"].sort())
+		expect(result["content-type"]).toBe("application/xml")
+		expect(result["Content-Type"]).toBe("text/plain")
+	})
+})

webview-ui/src/components/settings/utils/headers.ts

@@ -0,0 +1,25 @@
+/**
+ * Converts an array of header key-value pairs to a Record object.
+ *
+ * @param headers Array of [key, value] tuples representing HTTP headers
+ * @returns Record with trimmed keys and values
+ */
+export const convertHeadersToObject = (headers: [string, string][]): Record<string, string> => {
+	const result: Record<string, string> = {}
+
+	// Process each header tuple.
+	for (const [key, value] of headers) {
+		const trimmedKey = key.trim()
+
+		// Skip empty keys.
+		if (!trimmedKey) {
+			continue
+		}
+
+		// For duplicates, the last one in the array wins.
+		// This matches how HTTP headers work in general.
+		result[trimmedKey] = value.trim()
+	}
+
+	return result
+}