Fix bug with empty prompt components

Author: mrubens

src/core/prompts/__tests__/get-prompt-component.spec.ts

@@ -0,0 +1,88 @@
+import { describe, it, expect } from "vitest"
+import { getPromptComponent } from "../system"
+import type { CustomModePrompts } from "@roo-code/types"
+
+describe("getPromptComponent", () => {
+	it("should return undefined for empty objects", () => {
+		const customModePrompts: CustomModePrompts = {
+			architect: {},
+		}
+
+		const result = getPromptComponent(customModePrompts, "architect")
+		expect(result).toBeUndefined()
+	})
+
+	it("should return the component for objects with any properties", () => {
+		const customModePrompts: CustomModePrompts = {
+			architect: {
+				foo: "bar",
+				baz: 123,
+			} as any,
+		}
+
+		const result = getPromptComponent(customModePrompts, "architect")
+		expect(result).toEqual({ foo: "bar", baz: 123 })
+	})
+
+	it("should return undefined for missing mode", () => {
+		const customModePrompts: CustomModePrompts = {}
+
+		const result = getPromptComponent(customModePrompts, "architect")
+		expect(result).toBeUndefined()
+	})
+
+	it("should return undefined when customModePrompts is undefined", () => {
+		const result = getPromptComponent(undefined, "architect")
+		expect(result).toBeUndefined()
+	})
+
+	it.each([
+		["roleDefinition", { roleDefinition: "Test role" }],
+		["customInstructions", { customInstructions: "Test instructions" }],
+		["whenToUse", { whenToUse: "Test when to use" }],
+		["description", { description: "Test description" }],
+	])("should return the component when it has %s", (property, component) => {
+		const customModePrompts: CustomModePrompts = {
+			architect: component,
+		}
+
+		const result = getPromptComponent(customModePrompts, "architect")
+		expect(result).toEqual(component)
+	})
+
+	it("should return the component when it has multiple properties", () => {
+		const customModePrompts: CustomModePrompts = {
+			architect: {
+				roleDefinition: "Test role",
+				customInstructions: "Test instructions",
+				whenToUse: "Test when to use",
+				description: "Test description",
+			},
+		}
+
+		const result = getPromptComponent(customModePrompts, "architect")
+		expect(result).toEqual({
+			roleDefinition: "Test role",
+			customInstructions: "Test instructions",
+			whenToUse: "Test when to use",
+			description: "Test description",
+		})
+	})
+
+	it("should return the component when it has both relevant and irrelevant properties", () => {
+		const customModePrompts: CustomModePrompts = {
+			architect: {
+				roleDefinition: "Test role",
+				foo: "bar",
+				baz: 123,
+			} as any,
+		}
+
+		const result = getPromptComponent(customModePrompts, "architect")
+		expect(result).toEqual({
+			roleDefinition: "Test role",
+			foo: "bar",
+			baz: 123,
+		})
+	})
+})

src/core/prompts/system.ts

@@ -6,6 +6,7 @@ import type { ModeConfig, PromptComponent, CustomModePrompts, TodoItem } from "@
 import { Mode, modes, defaultModeSlug, getModeBySlug, getGroupName, getModeSelection } from "../../shared/modes"
 import { DiffStrategy } from "../../shared/tools"
 import { formatLanguage } from "../../shared/language"
+import { isEmpty } from "../../utils/object"
 
 import { McpHub } from "../../services/mcp/McpHub"
 import { CodeIndexManager } from "../../services/code-index/manager"
@@ -26,6 +27,19 @@ import {
 	markdownFormattingSection,
 } from "./sections"
 
+// Helper function to get prompt component, filtering out empty objects
+export function getPromptComponent(
+	customModePrompts: CustomModePrompts | undefined,
+	mode: string,
+): PromptComponent | undefined {
+	const component = customModePrompts?.[mode]
+	// Return undefined if component is empty
+	if (isEmpty(component)) {
+		return undefined
+	}
+	return component
+}
+
 async function generatePrompt(
 	context: vscode.ExtensionContext,
 	cwd: string,
@@ -129,13 +143,6 @@ export const SYSTEM_PROMPT = async (
 		throw new Error("Extension context is required for generating system prompt")
 	}
 
-	const getPromptComponent = (value: unknown) => {
-		if (typeof value === "object" && value !== null) {
-			return value as PromptComponent
-		}
-		return undefined
-	}
-
 	// Try to load custom system prompt from file
 	const variablesForPrompt: PromptVariables = {
 		workspace: cwd,
@@ -147,7 +154,7 @@ export const SYSTEM_PROMPT = async (
 	const fileCustomSystemPrompt = await loadSystemPromptFile(cwd, mode, variablesForPrompt)
 
 	// Check if it's a custom mode
-	const promptComponent = getPromptComponent(customModePrompts?.[mode])
+	const promptComponent = getPromptComponent(customModePrompts, mode)
 
 	// Get full mode config from custom modes or fall back to built-in modes
 	const currentMode = getModeBySlug(mode, customModes) || modes.find((m) => m.slug === mode) || modes[0]

src/shared/__tests__/modes-empty-prompt-component.spec.ts

@@ -0,0 +1,56 @@
+import { describe, it, expect } from "vitest"
+import { getModeSelection, modes } from "../modes"
+import type { PromptComponent } from "@roo-code/types"
+
+describe("getModeSelection with empty promptComponent", () => {
+	it("should use built-in mode instructions when promptComponent is undefined", () => {
+		const architectMode = modes.find((m) => m.slug === "architect")!
+
+		// Test with undefined promptComponent (which is what getPromptComponent returns for empty objects)
+		const result = getModeSelection("architect", undefined, [])
+
+		// Should use built-in mode values
+		expect(result.roleDefinition).toBe(architectMode.roleDefinition)
+		expect(result.baseInstructions).toBe(architectMode.customInstructions)
+		expect(result.baseInstructions).toContain("Do some information gathering")
+	})
+
+	it("should use built-in mode instructions when promptComponent is null", () => {
+		const debugMode = modes.find((m) => m.slug === "debug")!
+
+		// Test with null promptComponent
+		const result = getModeSelection("debug", null as any, [])
+
+		// Should use built-in mode values
+		expect(result.roleDefinition).toBe(debugMode.roleDefinition)
+		expect(result.baseInstructions).toBe(debugMode.customInstructions)
+		expect(result.baseInstructions).toContain("Reflect on 5-7 different possible sources")
+	})
+
+	it("should use promptComponent when it has actual content", () => {
+		// Test with promptComponent that has actual content
+		const validPromptComponent: PromptComponent = {
+			roleDefinition: "Custom role",
+			customInstructions: "Custom instructions",
+		}
+		const result = getModeSelection("architect", validPromptComponent, [])
+
+		// Should use promptComponent values
+		expect(result.roleDefinition).toBe("Custom role")
+		expect(result.baseInstructions).toBe("Custom instructions")
+	})
+
+	it("should use promptComponent when it has partial content", () => {
+		const architectMode = modes.find((m) => m.slug === "architect")!
+
+		// Test with promptComponent that only has customInstructions
+		const partialPromptComponent: PromptComponent = {
+			customInstructions: "Only custom instructions",
+		}
+		const result = getModeSelection("architect", partialPromptComponent, [])
+
+		// Should use promptComponent since it has some content
+		expect(result.roleDefinition).toBe("") // No roleDefinition in promptComponent
+		expect(result.baseInstructions).toBe("Only custom instructions")
+	})
+})

src/utils/__tests__/object.spec.ts

@@ -0,0 +1,38 @@
+import { describe, it, expect } from "vitest"
+import { isEmpty } from "../object"
+
+describe("isEmpty", () => {
+	describe("should return true for empty values", () => {
+		it.each([
+			["empty object", {}],
+			["empty array", []],
+			["null", null],
+			["undefined", undefined],
+			["string", "string"],
+			["number", 123],
+			["boolean true", true],
+			["boolean false", false],
+		])("%s", (_, value) => {
+			expect(isEmpty(value)).toBe(true)
+		})
+	})
+
+	describe("should return false for non-empty values", () => {
+		it.each([
+			["object with properties", { a: 1 }],
+			["object with multiple properties", { a: 1, b: 2 }],
+			["array with one item", [1]],
+			["array with multiple items", [1, 2, 3]],
+		])("%s", (_, value) => {
+			expect(isEmpty(value)).toBe(false)
+		})
+	})
+
+	it("should handle objects with null prototype", () => {
+		const obj = Object.create(null)
+		expect(isEmpty(obj)).toBe(true)
+
+		obj.prop = "value"
+		expect(isEmpty(obj)).toBe(false)
+	})
+})

src/utils/object.ts

@@ -0,0 +1,18 @@
+/**
+ * Check if an object is empty (has no own enumerable properties)
+ * @param obj The object to check
+ * @returns true if the object is empty, false otherwise
+ */
+export function isEmpty(obj: unknown): boolean {
+	if (!obj || typeof obj !== "object") {
+		return true
+	}
+
+	// Check if it's an array
+	if (Array.isArray(obj)) {
+		return obj.length === 0
+	}
+
+	// Check if it's an object with no own properties
+	return Object.keys(obj).length === 0
+}