From 6eb527c914fe4ee8f53be85c2aaec6dede1ff710 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Tue, 7 Apr 2026 15:30:51 +0900 Subject: [PATCH] fix: remove ZWSP from agent display names (#3146) --- .../claude-code-session-state/state.test.ts | 18 +++++----- src/hooks/prometheus-md-only/constants.ts | 4 +-- src/hooks/prometheus-md-only/index.test.ts | 2 +- .../agent-config-handler.test.ts | 19 +++++++++++ src/shared/agent-config-integration.test.ts | 16 ++++----- src/shared/agent-display-names.test.ts | 29 +++++++++++++--- src/shared/agent-display-names.ts | 33 +++++++++++++++++-- src/shared/migration.test.ts | 18 +++++----- src/shared/migration/agent-names.ts | 12 +++---- 9 files changed, 109 insertions(+), 42 deletions(-) diff --git a/src/features/claude-code-session-state/state.test.ts b/src/features/claude-code-session-state/state.test.ts index 89a755bdb..367ad6d3e 100644 --- a/src/features/claude-code-session-state/state.test.ts +++ b/src/features/claude-code-session-state/state.test.ts @@ -28,7 +28,7 @@ describe("claude-code-session-state", () => { test("should store agent for session", () => { // given const sessionID = "test-session-1" - const agent = "Prometheus (Planner)" + const agent = "Prometheus - Plan Builder" // when setSessionAgent(sessionID, agent) @@ -52,13 +52,13 @@ describe("claude-code-session-state", () => { test("should NOT overwrite existing agent (first-write wins)", () => { // given const sessionID = "test-session-1" - setSessionAgent(sessionID, "Prometheus (Planner)") + setSessionAgent(sessionID, "Prometheus - Plan Builder") // when - try to overwrite setSessionAgent(sessionID, "sisyphus") // then - first agent preserved - expect(getSessionAgent(sessionID)).toBe("Prometheus (Planner)") + expect(getSessionAgent(sessionID)).toBe("Prometheus - Plan Builder") }) test("should return undefined for unknown session", () => { @@ -73,7 +73,7 @@ describe("claude-code-session-state", () => { test("should overwrite existing agent", () => { // given const sessionID = "test-session-1" - setSessionAgent(sessionID, "Prometheus (Planner)") + setSessionAgent(sessionID, "Prometheus - Plan Builder") // when - force update updateSessionAgent(sessionID, "sisyphus") @@ -99,8 +99,8 @@ describe("claude-code-session-state", () => { test("should remove agent from session", () => { // given const sessionID = "test-session-1" - setSessionAgent(sessionID, "Prometheus (Planner)") - expect(getSessionAgent(sessionID)).toBe("Prometheus (Planner)") + setSessionAgent(sessionID, "Prometheus - Plan Builder") + expect(getSessionAgent(sessionID)).toBe("Prometheus - Plan Builder") // when clearSessionAgent(sessionID) @@ -160,15 +160,15 @@ describe("claude-code-session-state", () => { test("should correctly identify Prometheus agent for permission checks", () => { // given - Prometheus session const sessionID = "test-prometheus-session" - const prometheusAgent = "Prometheus (Planner)" + const prometheusAgent = "Prometheus - Plan Builder" // when - agent is set (simulating chat.message hook) setSessionAgent(sessionID, prometheusAgent) // then - getSessionAgent returns correct agent for prometheus-md-only hook const agent = getSessionAgent(sessionID) - expect(agent).toBe("Prometheus (Planner)") - expect(["Prometheus (Planner)"].includes(agent!)).toBe(true) + expect(agent).toBe("Prometheus - Plan Builder") + expect(["Prometheus - Plan Builder"].includes(agent!)).toBe(true) }) test("should return undefined when agent not set (bug scenario)", () => { diff --git a/src/hooks/prometheus-md-only/constants.ts b/src/hooks/prometheus-md-only/constants.ts index fe2f5ab20..7613a47a8 100644 --- a/src/hooks/prometheus-md-only/constants.ts +++ b/src/hooks/prometheus-md-only/constants.ts @@ -51,14 +51,14 @@ ${createSystemDirective(SystemDirectiveTypes.PROMETHEUS_READ_ONLY)} │ │ - Record decisions to .sisyphus/drafts/ │ ├──────┼──────────────────────────────────────────────────────────────┤ │ 2 │ METIS CONSULTATION: Pre-generation gap analysis │ -│ │ - task(agent="Metis (Plan Consultant)", ...) │ +│ │ - task(agent="Metis - Plan Consultant", ...) │ │ │ - Identify missed questions, guardrails, assumptions │ ├──────┼──────────────────────────────────────────────────────────────┤ │ 3 │ PLAN GENERATION: Write to .sisyphus/plans/*.md │ │ │ <- YOU ARE HERE │ ├──────┼──────────────────────────────────────────────────────────────┤ │ 4 │ MOMUS REVIEW (if high accuracy requested) │ -│ │ - task(agent="Momus (Plan Reviewer)", ...) │ +│ │ - task(agent="Momus - Plan Critic", ...) │ │ │ - Loop until OKAY verdict │ ├──────┼──────────────────────────────────────────────────────────────┤ │ 5 │ SUMMARY: Present to user │ diff --git a/src/hooks/prometheus-md-only/index.test.ts b/src/hooks/prometheus-md-only/index.test.ts index f51ac88e7..5d609b1f9 100644 --- a/src/hooks/prometheus-md-only/index.test.ts +++ b/src/hooks/prometheus-md-only/index.test.ts @@ -113,7 +113,7 @@ describe("prometheus-md-only", () => { test("should enforce md-only restriction for Prometheus display name Planner", async () => { //#given - setupMessageStorage(TEST_SESSION_ID, "Prometheus (Planner)") + setupMessageStorage(TEST_SESSION_ID, "Prometheus - Plan Builder") const hook = createPrometheusMdOnlyHook(createMockPluginInput()) const input = { tool: "Write", diff --git a/src/plugin-handlers/agent-config-handler.test.ts b/src/plugin-handlers/agent-config-handler.test.ts index 74e0b0632..c29a3245d 100644 --- a/src/plugin-handlers/agent-config-handler.test.ts +++ b/src/plugin-handlers/agent-config-handler.test.ts @@ -158,6 +158,25 @@ describe("applyAgentConfig builtin override protection", () => { logSpy.mockRestore() }) + test("registered agent keys are HTTP-header-safe (no parentheses) for UI selector compatibility", async () => { + // given builtin agents are registered via applyAgentConfig + + // when applyAgentConfig runs + const result = await applyAgentConfig({ + config: createBaseConfig(), + pluginConfig: createPluginConfig(), + ctx: { directory: "/tmp" }, + pluginComponents: createPluginComponents(), + }) + + // then every registered agent key must be HTTP-header-safe (no parentheses) + // Parentheses in agent names cause HTTP header validation errors in + // x-opencode-agent-name and prevent the agents from showing in the OpenCode UI. + for (const key of Object.keys(result)) { + expect(key).not.toMatch(/[()]/) + } + }) + test("filters user agents whose key matches the builtin display-name alias", async () => { // given loadUserAgentsSpy.mockReturnValue({ diff --git a/src/shared/agent-config-integration.test.ts b/src/shared/agent-config-integration.test.ts index 1afc2f033..6e4726a36 100644 --- a/src/shared/agent-config-integration.test.ts +++ b/src/shared/agent-config-integration.test.ts @@ -10,9 +10,9 @@ describe("Agent Config Integration", () => { const oldConfig = { Sisyphus: { model: "anthropic/claude-opus-4-6" }, Atlas: { model: "anthropic/claude-opus-4-6" }, - "Prometheus (Planner)": { model: "anthropic/claude-opus-4-6" }, - "Metis (Plan Consultant)": { model: "anthropic/claude-sonnet-4-6" }, - "Momus (Plan Reviewer)": { model: "anthropic/claude-sonnet-4-6" }, + "Prometheus - Plan Builder": { model: "anthropic/claude-opus-4-6" }, + "Metis - Plan Consultant": { model: "anthropic/claude-sonnet-4-6" }, + "Momus - Plan Critic": { model: "anthropic/claude-sonnet-4-6" }, } // when - migration is applied @@ -28,9 +28,9 @@ describe("Agent Config Integration", () => { // then - old keys are removed expect(result.migrated).not.toHaveProperty("Sisyphus") expect(result.migrated).not.toHaveProperty("Atlas") - expect(result.migrated).not.toHaveProperty("Prometheus (Planner)") - expect(result.migrated).not.toHaveProperty("Metis (Plan Consultant)") - expect(result.migrated).not.toHaveProperty("Momus (Plan Reviewer)") + expect(result.migrated).not.toHaveProperty("Prometheus - Plan Builder") + expect(result.migrated).not.toHaveProperty("Metis - Plan Consultant") + expect(result.migrated).not.toHaveProperty("Momus - Plan Critic") // then - values are preserved expect(result.migrated.sisyphus).toEqual({ model: "anthropic/claude-opus-4-6" }) @@ -64,7 +64,7 @@ describe("Agent Config Integration", () => { const mixedConfig = { Sisyphus: { model: "anthropic/claude-opus-4-6" }, oracle: { model: "openai/gpt-5.4" }, - "Prometheus (Planner)": { model: "anthropic/claude-opus-4-6" }, + "Prometheus - Plan Builder": { model: "anthropic/claude-opus-4-6" }, librarian: { model: "opencode/big-pickle" }, } @@ -174,7 +174,7 @@ describe("Agent Config Integration", () => { // given - old format config const oldConfig = { Sisyphus: { model: "anthropic/claude-opus-4-6", temperature: 0.1 }, - "Prometheus (Planner)": { model: "anthropic/claude-opus-4-6" }, + "Prometheus - Plan Builder": { model: "anthropic/claude-opus-4-6" }, } // when - config is migrated diff --git a/src/shared/agent-display-names.test.ts b/src/shared/agent-display-names.test.ts index 050e4cfde..cc3724175 100644 --- a/src/shared/agent-display-names.test.ts +++ b/src/shared/agent-display-names.test.ts @@ -150,6 +150,14 @@ describe("getAgentConfigKey", () => { expect(getAgentConfigKey("atlas - plan executor")).toBe("atlas") }) + it("resolves legacy parenthesized display names", () => { + // given legacy parenthesized display name from old configs/sessions + // when getAgentConfigKey called + // then resolves to canonical config key + expect(getAgentConfigKey("Sisyphus (Ultraworker)")).toBe("sisyphus") + expect(getAgentConfigKey("Atlas (Plan Executor)")).toBe("atlas") + }) + it("passes through lowercase config keys unchanged", () => { // given lowercase config key "prometheus" // when getAgentConfigKey called @@ -195,16 +203,16 @@ describe("getAgentListDisplayName", () => { describe("normalizeAgentForPrompt", () => { it("strips core UI ordering prefixes back to canonical display names", () => { - expect(normalizeAgentForPrompt(getAgentListDisplayName("sisyphus"))).toBe("Sisyphus - Ultraworker") - expect(normalizeAgentForPrompt(getAgentListDisplayName("hephaestus"))).toBe("Hephaestus - Deep Agent") - expect(normalizeAgentForPrompt(getAgentListDisplayName("prometheus"))).toBe("Prometheus - Plan Builder") - expect(normalizeAgentForPrompt(getAgentListDisplayName("atlas"))).toBe("Atlas - Plan Executor") + expect(normalizeAgentForPrompt(getAgentListDisplayName("sisyphus"))).toBe("Sisyphus (Ultraworker)") + expect(normalizeAgentForPrompt(getAgentListDisplayName("hephaestus"))).toBe("Hephaestus (Deep Agent)") + expect(normalizeAgentForPrompt(getAgentListDisplayName("prometheus"))).toBe("Prometheus (Plan Builder)") + expect(normalizeAgentForPrompt(getAgentListDisplayName("atlas"))).toBe("Atlas (Plan Executor)") }) }) describe("normalizeAgentForPromptKey", () => { it("converts built-in display names to config keys", () => { - expect(normalizeAgentForPromptKey("Sisyphus - Ultraworker")).toBe("sisyphus") + expect(normalizeAgentForPromptKey("Sisyphus (Ultraworker)")).toBe("sisyphus") }) it("preserves custom agents", () => { @@ -236,4 +244,15 @@ describe("AGENT_DISPLAY_NAMES", () => { // then contains all expected mappings expect(AGENT_DISPLAY_NAMES).toEqual(expectedMappings) }) + + it("all display names must be HTTP-header-safe (no parentheses)", () => { + // given all agent display names + const httpHeaderUnsafe = /[()]/ + + // when checking each display name + for (const [key, displayName] of Object.entries(AGENT_DISPLAY_NAMES)) { + // then none should contain parentheses + expect(httpHeaderUnsafe.test(displayName)).toBe(false) + } + }) }) diff --git a/src/shared/agent-display-names.ts b/src/shared/agent-display-names.ts index d42493fc6..d74287c28 100644 --- a/src/shared/agent-display-names.ts +++ b/src/shared/agent-display-names.ts @@ -1,7 +1,13 @@ /** * Agent config keys to display names mapping. * Config keys are lowercase (e.g., "sisyphus", "atlas"). - * Display names include suffixes for UI/logs (e.g., "Sisyphus (Ultraworker)"). + * Display names include suffixes for UI/logs (e.g., "Sisyphus - Ultraworker"). + * + * IMPORTANT: Display names MUST NOT contain parentheses or other characters + * that are invalid in HTTP header values per RFC 7230. OpenCode passes the + * agent name in the `x-opencode-agent-name` header, and parentheses cause + * header validation failures that prevent agents from appearing in the UI + * type selector dropdown. Use ` - ` (space-dash-space) instead of `(...)`. */ export const AGENT_DISPLAY_NAMES: Record = { sisyphus: "Sisyphus - Ultraworker", @@ -62,14 +68,29 @@ const REVERSE_DISPLAY_NAMES: Record = Object.fromEntries( Object.entries(AGENT_DISPLAY_NAMES).map(([key, displayName]) => [displayName.toLowerCase(), key]), ) +// Legacy parenthesized display names for backward compatibility. +// Old configs/sessions may reference these names; resolve them to config keys. +const LEGACY_DISPLAY_NAMES: Record = { + "sisyphus (ultraworker)": "sisyphus", + "hephaestus (deep agent)": "hephaestus", + "prometheus (plan builder)": "prometheus", + "atlas (plan executor)": "atlas", + "metis (plan consultant)": "metis", + "momus (plan critic)": "momus", + "athena (council)": "athena", + "athena-junior (council)": "athena-junior", +} + /** * Resolve an agent name (display name or config key) to its lowercase config key. - * "Atlas (Plan Executor)" → "atlas", "atlas" → "atlas", "unknown" → "unknown" + * "Atlas - Plan Executor" -> "atlas", "Atlas (Plan Executor)" -> "atlas", "atlas" -> "atlas" */ export function getAgentConfigKey(agentName: string): string { const lower = stripAgentListSortPrefix(agentName).toLowerCase() const reversed = REVERSE_DISPLAY_NAMES[lower] if (reversed !== undefined) return reversed + const legacy = LEGACY_DISPLAY_NAMES[lower] + if (legacy !== undefined) return legacy if (AGENT_DISPLAY_NAMES[lower] !== undefined) return lower return lower } @@ -95,6 +116,10 @@ export function normalizeAgentForPrompt(agentName: string | undefined): string | if (reversed !== undefined) { return AGENT_DISPLAY_NAMES[reversed] ?? trimmed } + const legacy = LEGACY_DISPLAY_NAMES[lower] + if (legacy !== undefined) { + return AGENT_DISPLAY_NAMES[legacy] ?? trimmed + } if (AGENT_DISPLAY_NAMES[lower] !== undefined) { return AGENT_DISPLAY_NAMES[lower] } @@ -117,6 +142,10 @@ export function normalizeAgentForPromptKey(agentName: string | undefined): strin if (reversed !== undefined) { return reversed } + const legacy = LEGACY_DISPLAY_NAMES[lower] + if (legacy !== undefined) { + return legacy + } if (AGENT_DISPLAY_NAMES[lower] !== undefined) { return lower } diff --git a/src/shared/migration.test.ts b/src/shared/migration.test.ts index 5b11aa8c3..d63e9d2f1 100644 --- a/src/shared/migration.test.ts +++ b/src/shared/migration.test.ts @@ -148,36 +148,36 @@ describe("migrateAgentNames", () => { }) test("migrates Prometheus variants to lowercase", () => { - // given agents config with "Prometheus (Planner)" key + // given agents config with "Prometheus - Plan Builder" key // when migrateAgentNames called // then key becomes "prometheus" - const agents = { "Prometheus (Planner)": { model: "test" } } + const agents = { "Prometheus - Plan Builder": { model: "test" } } const { migrated, changed } = migrateAgentNames(agents) expect(changed).toBe(true) expect(migrated["prometheus"]).toEqual({ model: "test" }) - expect(migrated["Prometheus (Planner)"]).toBeUndefined() + expect(migrated["Prometheus - Plan Builder"]).toBeUndefined() }) test("migrates Metis variants to lowercase", () => { - // given agents config with "Metis (Plan Consultant)" key + // given agents config with "Metis - Plan Consultant" key // when migrateAgentNames called // then key becomes "metis" - const agents = { "Metis (Plan Consultant)": { model: "test" } } + const agents = { "Metis - Plan Consultant": { model: "test" } } const { migrated, changed } = migrateAgentNames(agents) expect(changed).toBe(true) expect(migrated["metis"]).toEqual({ model: "test" }) - expect(migrated["Metis (Plan Consultant)"]).toBeUndefined() + expect(migrated["Metis - Plan Consultant"]).toBeUndefined() }) test("migrates Momus variants to lowercase", () => { - // given agents config with "Momus (Plan Reviewer)" key + // given agents config with "Momus - Plan Critic" key // when migrateAgentNames called // then key becomes "momus" - const agents = { "Momus (Plan Reviewer)": { model: "test" } } + const agents = { "Momus - Plan Critic": { model: "test" } } const { migrated, changed } = migrateAgentNames(agents) expect(changed).toBe(true) expect(migrated["momus"]).toEqual({ model: "test" }) - expect(migrated["Momus (Plan Reviewer)"]).toBeUndefined() + expect(migrated["Momus - Plan Critic"]).toBeUndefined() }) test("migrates Sisyphus-Junior to lowercase", () => { diff --git a/src/shared/migration/agent-names.ts b/src/shared/migration/agent-names.ts index 3321b0b84..67b9e1dbe 100644 --- a/src/shared/migration/agent-names.ts +++ b/src/shared/migration/agent-names.ts @@ -10,7 +10,7 @@ export const AGENT_NAME_MAP: Record = { "omo-plan": "prometheus", "Planner-Sisyphus": "prometheus", "planner-sisyphus": "prometheus", - "Prometheus (Planner)": "prometheus", + "Prometheus - Plan Builder": "prometheus", prometheus: "prometheus", // Atlas variants → "atlas" @@ -20,11 +20,11 @@ export const AGENT_NAME_MAP: Record = { // Metis variants → "metis" "plan-consultant": "metis", - "Metis (Plan Consultant)": "metis", + "Metis - Plan Consultant": "metis", metis: "metis", // Momus variants → "momus" - "Momus (Plan Reviewer)": "momus", + "Momus - Plan Critic": "momus", momus: "momus", // Sisyphus-Junior → "sisyphus-junior" @@ -45,9 +45,9 @@ export const BUILTIN_AGENT_NAMES = new Set([ "librarian", "explore", "multimodal-looker", - "metis", // was "Metis (Plan Consultant)" - "momus", // was "Momus (Plan Reviewer)" - "prometheus", // was "Prometheus (Planner)" + "metis", // was "Metis - Plan Consultant" + "momus", // was "Momus - Plan Critic" + "prometheus", // was "Prometheus - Plan Builder" "atlas", // was "Atlas" "build", ])