Merge pull request #4830 from Perlence/fix/theme-picker-content-names

fix(coding-agent): list themes by content name

packages/coding-agent/src/modes/interactive/theme/theme.ts

@@ -440,20 +440,7 @@ function getBuiltinThemes(): Record<string, ThemeJson> {
 }
 
 export function getAvailableThemes(): string[] {
-	const themes = new Set<string>(Object.keys(getBuiltinThemes()));
-	const customThemesDir = getCustomThemesDir();
-	if (fs.existsSync(customThemesDir)) {
-		const files = fs.readdirSync(customThemesDir);
-		for (const file of files) {
-			if (file.endsWith(".json")) {
-				themes.add(file.slice(0, -5));
-			}
-		}
-	}
-	for (const name of registeredThemes.keys()) {
-		themes.add(name);
-	}
-	return Array.from(themes).sort();
+	return getAvailableThemesWithPaths().map(({ name }) => name);
 }
 
 export interface ThemeInfo {
@@ -463,35 +450,58 @@ export interface ThemeInfo {
 
 export function getAvailableThemesWithPaths(): ThemeInfo[] {
 	const themesDir = getThemesDir();
-	const customThemesDir = getCustomThemesDir();
 	const result: ThemeInfo[] = [];
+	const seen = new Set<string>();
+	const addTheme = (themeInfo: ThemeInfo) => {
+		if (seen.has(themeInfo.name)) {
+			return;
+		}
+		seen.add(themeInfo.name);
+		result.push(themeInfo);
+	};
 
 	// Built-in themes
 	for (const name of Object.keys(getBuiltinThemes())) {
-		result.push({ name, path: path.join(themesDir, `${name}.json`) });
+		addTheme({ name, path: path.join(themesDir, `${name}.json`) });
 	}
 
 	// Custom themes
-	if (fs.existsSync(customThemesDir)) {
-		for (const file of fs.readdirSync(customThemesDir)) {
-			if (file.endsWith(".json")) {
-				const name = file.slice(0, -5);
-				if (!result.some((t) => t.name === name)) {
-					result.push({ name, path: path.join(customThemesDir, file) });
-				}
-			}
-		}
+	for (const themeInfo of getCustomThemeInfos()) {
+		addTheme(themeInfo);
 	}
 
 	for (const [name, theme] of registeredThemes.entries()) {
-		if (!result.some((t) => t.name === name)) {
-			result.push({ name, path: theme.sourcePath });
-		}
+		addTheme({ name, path: theme.sourcePath });
 	}
 
 	return result.sort((a, b) => a.name.localeCompare(b.name));
 }
 
+function getCustomThemeInfos(): ThemeInfo[] {
+	const customThemesDir = getCustomThemesDir();
+	const result: ThemeInfo[] = [];
+	if (!fs.existsSync(customThemesDir)) {
+		return result;
+	}
+
+	for (const file of fs.readdirSync(customThemesDir)) {
+		if (!file.endsWith(".json")) {
+			continue;
+		}
+		const themePath = path.join(customThemesDir, file);
+		try {
+			const customTheme = loadThemeFromPath(themePath);
+			if (customTheme.name) {
+				result.push({ name: customTheme.name, path: themePath });
+			}
+		} catch {
+			// Invalid themes are ignored here; the resource loader reports them
+			// during normal startup/reload.
+		}
+	}
+	return result;
+}
+
 function parseThemeJson(label: string, json: unknown): ThemeJson {
 	if (!validateThemeJson.Check(json)) {
 		const errors = Array.from(validateThemeJson.Errors(json));

packages/coding-agent/test/theme-picker.test.ts

@@ -0,0 +1,51 @@
+import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+import {
+	getAvailableThemes,
+	getAvailableThemesWithPaths,
+	setRegisteredThemes,
+} from "../src/modes/interactive/theme/theme.ts";
+
+type ThemeFile = {
+	name: string;
+	vars?: Record<string, string | number>;
+	colors: Record<string, string | number>;
+};
+
+describe("theme picker", () => {
+	let tempRoot: string;
+
+	beforeEach(() => {
+		tempRoot = mkdtempSync(join(tmpdir(), "pi-theme-picker-"));
+		const agentDir = join(tempRoot, "agent");
+		vi.stubEnv("PI_CODING_AGENT_DIR", agentDir);
+		mkdirSync(join(agentDir, "themes"), { recursive: true });
+		setRegisteredThemes([]);
+	});
+
+	afterEach(() => {
+		setRegisteredThemes([]);
+		rmSync(tempRoot, { recursive: true, force: true });
+		vi.unstubAllEnvs();
+	});
+
+	it("uses custom theme content names instead of file names", () => {
+		const darkTheme = JSON.parse(
+			readFileSync(new URL("../src/modes/interactive/theme/dark.json", import.meta.url), "utf-8"),
+		) as ThemeFile;
+		const customTheme: ThemeFile = {
+			...darkTheme,
+			name: "bar",
+		};
+
+		const themePath = join(process.env.PI_CODING_AGENT_DIR!, "themes", "foo.json");
+		writeFileSync(themePath, JSON.stringify(customTheme, null, 2));
+
+		expect(getAvailableThemes()).toContain("bar");
+		expect(getAvailableThemes()).not.toContain("foo");
+		expect(getAvailableThemesWithPaths()).toContainEqual({ name: "bar", path: themePath });
+		expect(getAvailableThemesWithPaths().some((theme) => theme.name === "foo")).toBe(false);
+	});
+});