luna/oh-my-opencode
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Merge pull request #3189 from code-yeongyu/fix/issue-3146

Browse Source 
fix(plugin-loading): improve error handling for plugin installation failures (#3146)
This commit is contained in:
YeonGyu-Kim
2026-04-07 15:57:53 +09:00
committed by GitHub
parent b269f11522 f27a7f2c95
commit dcecf049a1
9 changed files with 109 additions and 42 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/features/claude-code-session-state/state.test.ts
+9 -9
View File
@@ -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)", () => {
src/hooks/prometheus-md-only/constants.ts
+2 -2
View File
@@ -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 │
src/hooks/prometheus-md-only/index.test.ts
+1 -1
View File
@@ -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",
src/plugin-handlers/agent-config-handler.test.ts
+19
View File
@@ -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({
src/shared/agent-config-integration.test.ts
+8 -8
View File
@@ -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
src/shared/agent-display-names.test.ts
+24 -5
View File
@@ -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)
}
})
})
src/shared/agent-display-names.ts
+31 -2
View File
@@ -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<string, string> = {
sisyphus: "Sisyphus - Ultraworker",
@@ -62,14 +68,29 @@ const REVERSE_DISPLAY_NAMES: Record<string, string> = 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<string, string> = {
"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
}
src/shared/migration.test.ts
+9 -9
View File
@@ -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", () => {
src/shared/migration/agent-names.ts
+6 -6
View File
@@ -10,7 +10,7 @@ export const AGENT_NAME_MAP: Record<string, string> = {
"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<string, string> = {
// 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",
])