From 0e9bb5969dcf7a83a587463117dbcca21a4f8934 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 27 Apr 2026 13:48:09 +0900 Subject: [PATCH 1/2] refactor(agents): replace broken ZWSP sort prefixes with leading ASCII spaces The ZWSP-based core agent sort prefixes silently failed to produce the canonical sisyphus -> hephaestus -> prometheus -> atlas order. Empirical testing of OpenCode's Agent.list() sort behavior shows that Unicode collation treats zero-width characters as ignorable at the primary level, so ZWSP-prefixed names sorted alphabetically with non-core agents interleaved (e.g. Sisyphus, athena, Atlas, explore, Hephaestus, ...). This commit replaces the ZWSP prefixes with leading ASCII spaces in descending lengths (sisyphus=4, hephaestus=3, prometheus=2, atlas=1). ASCII spaces sort reliably before alphabetic characters in localeCompare under all locales and render correctly in every terminal. Changes: - AGENT_LIST_SORT_PREFIXES: ZWSP -> leading spaces (4-3-2-1 descending) - stripAgentListSortPrefix: now strips both legacy ZWSP and new leading whitespace, preserving backward compatibility with existing sessions - normalizeStoredAgentName / normalizeRegisteredAgentName: extract a shared stripSortPrefix helper that handles both prefix formats - agent-config-handler: resolve user-provided default_agent display names through getAgentConfigKey before applying the runtime prefix, so configs like default_agent="Hephaestus - Deep Agent" are normalized - agent-runtime-name-sort.test.ts: new regression test simulating OpenCode's exact sortBy logic (default_agent desc + name asc localeCompare) to verify canonical core agent order under randomised input permutations - AGENTS.md: document the empirical finding that ZWSP was broken, why ASCII spaces work, and the descending prefix-length contract Existing strip functions retain ZWSP support so legacy session state and configs continue to resolve correctly without migration. --- .../claude-code-session-state/state.ts | 8 +- src/plugin-handlers/AGENTS.md | 27 +++- src/plugin-handlers/agent-config-handler.ts | 9 +- src/shared/agent-display-names.test.ts | 10 +- src/shared/agent-display-names.ts | 10 +- src/shared/agent-runtime-name-sort.test.ts | 152 ++++++++++++++++++ 6 files changed, 196 insertions(+), 20 deletions(-) create mode 100644 src/shared/agent-runtime-name-sort.test.ts diff --git a/src/features/claude-code-session-state/state.ts b/src/features/claude-code-session-state/state.ts index 049366166..0eccbf9b3 100644 --- a/src/features/claude-code-session-state/state.ts +++ b/src/features/claude-code-session-state/state.ts @@ -18,12 +18,16 @@ const registeredAgentAliases = new Map() const ZERO_WIDTH_CHARACTERS_REGEX = /[\u200B\u200C\u200D\uFEFF]/g +function stripSortPrefix(name: string): string { + return name.replace(ZERO_WIDTH_CHARACTERS_REGEX, "").replace(/^\s+/, "") +} + function normalizeRegisteredAgentName(name: string): string { - return name.replace(ZERO_WIDTH_CHARACTERS_REGEX, "").toLowerCase() + return stripSortPrefix(name).toLowerCase() } function normalizeStoredAgentName(name: string): string { - return name.replace(ZERO_WIDTH_CHARACTERS_REGEX, "") + return stripSortPrefix(name) } export function registerAgentName(name: string): void { diff --git a/src/plugin-handlers/AGENTS.md b/src/plugin-handlers/AGENTS.md index df6c8bf14..8d0154485 100644 --- a/src/plugin-handlers/AGENTS.md +++ b/src/plugin-handlers/AGENTS.md @@ -8,28 +8,45 @@ The canonical agent order is **sisyphus → hephaestus → prometheus → atlas* This order is enforced via two mechanisms working together: 1. `CANONICAL_CORE_AGENT_ORDER` in `agent-priority-order.ts` controls object key insertion order -2. `agent-key-remapper.ts` injects ZWSP-prefixed runtime names into the `name` field for OpenCode's `localeCompare` sort +2. `agent-key-remapper.ts` injects leading-space-prefixed runtime names into the `name` field for OpenCode's `localeCompare` sort ### Why Two Mechanisms -OpenCode's `Agent.list()` sorts agents by `name` field via `localeCompare`. Object key order alone is not enough. The `name` field carries ZWSP prefixes (1-4 chars) so core agents sort before alphabetically-named agents. +OpenCode's `Agent.list()` sorts agents by `name` field via `localeCompare`. Object key order alone is not enough. The `name` field carries leading ASCII spaces (4-3-2-1 descending) so core agents sort before alphabetically-named agents. -ZWSP is intentionally used in the `name` field only. It MUST NOT appear in: +The prefix lengths are intentionally **descending** (sisyphus=4, hephaestus=3, prometheus=2, atlas=1) because `localeCompare` puts strings with more leading whitespace before strings with fewer. Reference: see `agent-runtime-name-sort.test.ts` for empirical verification. + +### Why ASCII Spaces, Not ZWSP + +Earlier versions used ZWSP (`\u200B`) prefixes hoping they would be invisible to users. They silently failed: Unicode collation algorithms treat zero-width characters as ignorable at the primary level, so ZWSP-prefixed names sorted as if the prefix did not exist. The result was alphabetical order interleaving core and non-core agents. + +ASCII space (`\u0020`) is the only character that: +- Sorts before alphabetic characters reliably under all locales +- Renders correctly in every terminal (no glyph substitution) +- Is valid in HTTP header values (RFC 7230) when placed in the `name` field + +The leading-space prefix MUST NOT appear in: - Object keys (used as HTTP header values, causes RFC 7230 violations) - Display names returned by `getAgentDisplayName()` - Config keys +### Backward Compatibility + +`stripAgentListSortPrefix()` strips both the new leading-space prefix AND legacy ZWSP/zero-width characters. Existing sessions and configs from the ZWSP era continue to resolve correctly. + ### History -Agent ordering has caused 15+ commits, 8+ PRs, and multiple reverts due to: +Agent ordering caused 15+ commits, 8+ PRs, and multiple reverts due to: 1. Early ZWSP attempts that leaked into HTTP headers via object keys 2. Object.entries() iteration order depending on merge sequence 3. Multiple code paths assembling agents differently +4. The ZWSP prefix being silently broken in `localeCompare` sort (resolved in this commit by switching to leading ASCII spaces) ### Forbidden Patterns DO NOT introduce: -- ZWSP in object keys or display names (only allowed in `name` field via `getAgentRuntimeName()`) +- ZWSP in any field (broken in `localeCompare`, replaced by leading ASCII spaces) +- Leading whitespace in object keys or display names (allowed only in `name` field via `getAgentRuntimeName()`) - Runtime sort shims or comparators - Alternative ordering constants - Object.entries() order dependencies diff --git a/src/plugin-handlers/agent-config-handler.ts b/src/plugin-handlers/agent-config-handler.ts index 384871114..c8f2a7810 100644 --- a/src/plugin-handlers/agent-config-handler.ts +++ b/src/plugin-handlers/agent-config-handler.ts @@ -2,7 +2,7 @@ import { createBuiltinAgents } from "../agents"; import { createSisyphusJuniorAgentWithOverrides } from "../agents/sisyphus-junior"; import type { OhMyOpenCodeConfig } from "../config"; import { isTaskSystemEnabled, log, migrateAgentConfig } from "../shared"; -import { getAgentRuntimeName } from "../shared/agent-display-names"; +import { AGENT_DISPLAY_NAMES, getAgentConfigKey, getAgentRuntimeName } from "../shared/agent-display-names"; import { AGENT_NAME_MAP } from "../shared/migration"; import { registerAgentName } from "../features/claude-code-session-state"; import { @@ -189,8 +189,11 @@ export async function applyAgentConfig(params: { if (isSisyphusEnabled && builtinAgents.sisyphus) { if (configuredDefaultAgent) { - (params.config as { default_agent?: string }).default_agent = - getAgentRuntimeName(configuredDefaultAgent); + const configKey = getAgentConfigKey(configuredDefaultAgent); + const isKnownBuiltin = configKey in AGENT_DISPLAY_NAMES; + (params.config as { default_agent?: string }).default_agent = isKnownBuiltin + ? getAgentRuntimeName(configKey) + : configuredDefaultAgent; } else { (params.config as { default_agent?: string }).default_agent = getAgentRuntimeName("sisyphus"); diff --git a/src/shared/agent-display-names.test.ts b/src/shared/agent-display-names.test.ts index 2c3d732cd..0fb52ec06 100644 --- a/src/shared/agent-display-names.test.ts +++ b/src/shared/agent-display-names.test.ts @@ -194,11 +194,11 @@ describe("getAgentConfigKey", () => { }) describe("getAgentListDisplayName", () => { - it("applies invisible stable-sort prefixes to the core agent list", () => { - expect(getAgentListDisplayName("sisyphus")).toBe("\u200BSisyphus - Ultraworker") - expect(getAgentListDisplayName("hephaestus")).toBe("\u200B\u200BHephaestus - Deep Agent") - expect(getAgentListDisplayName("prometheus")).toBe("\u200B\u200B\u200BPrometheus - Plan Builder") - expect(getAgentListDisplayName("atlas")).toBe("\u200B\u200B\u200B\u200BAtlas - Plan Executor") + it("applies leading-space stable-sort prefixes so OpenCode localeCompare yields canonical order", () => { + expect(getAgentListDisplayName("sisyphus")).toBe(" Sisyphus - Ultraworker") + expect(getAgentListDisplayName("hephaestus")).toBe(" Hephaestus - Deep Agent") + expect(getAgentListDisplayName("prometheus")).toBe(" Prometheus - Plan Builder") + expect(getAgentListDisplayName("atlas")).toBe(" Atlas - Plan Executor") }) it("keeps non-core agents unprefixed for list display", () => { diff --git a/src/shared/agent-display-names.ts b/src/shared/agent-display-names.ts index 324fac785..081550d7c 100644 --- a/src/shared/agent-display-names.ts +++ b/src/shared/agent-display-names.ts @@ -27,10 +27,10 @@ export const AGENT_DISPLAY_NAMES: Record = { } const AGENT_LIST_SORT_PREFIXES: Record = { - sisyphus: "\u200B", - hephaestus: "\u200B\u200B", - prometheus: "\u200B\u200B\u200B", - atlas: "\u200B\u200B\u200B\u200B", + sisyphus: " ", + hephaestus: " ", + prometheus: " ", + atlas: " ", } const INVISIBLE_AGENT_CHARACTERS_REGEX = /[\u200B\u200C\u200D\uFEFF]/g @@ -40,7 +40,7 @@ export function stripInvisibleAgentCharacters(agentName: string): string { } export function stripAgentListSortPrefix(agentName: string): string { - return stripInvisibleAgentCharacters(agentName) + return stripInvisibleAgentCharacters(agentName).replace(/^\s+/, "") } export function getAgentRuntimeName(configKey: string): string { diff --git a/src/shared/agent-runtime-name-sort.test.ts b/src/shared/agent-runtime-name-sort.test.ts new file mode 100644 index 000000000..c39b4a545 --- /dev/null +++ b/src/shared/agent-runtime-name-sort.test.ts @@ -0,0 +1,152 @@ +/// + +import { describe, expect, it, test } from "bun:test" + +import { + AGENT_DISPLAY_NAMES, + getAgentRuntimeName, + normalizeAgentForPromptKey, +} from "./agent-display-names" + +// OpenCode Agent.list() sorts via remeda sortBy: default_agent desc, then name asc localeCompare. +// Reference: ../opencode/packages/opencode/src/agent/agent.ts:284-293. +// Earlier ZWSP prefixes silently failed: Unicode collation treats zero-width chars as ignorable. +function simulateOpencodeSort(agentNames: string[], defaultName: string): string[] { + return [...agentNames].sort((a, b) => { + const aIsDefault = a === defaultName ? 1 : 0 + const bIsDefault = b === defaultName ? 1 : 0 + if (aIsDefault !== bIsDefault) return bIsDefault - aIsDefault + return a.localeCompare(b) + }) +} + +describe("OpenCode Agent.list() sort with runtime-name prefixes", () => { + describe("#given the four core agents and a mix of non-core agents", () => { + test("#when sorted using opencode-style sortBy #then core agents come first in canonical order", () => { + const sisyphus = getAgentRuntimeName("sisyphus") + const hephaestus = getAgentRuntimeName("hephaestus") + const prometheus = getAgentRuntimeName("prometheus") + const atlas = getAgentRuntimeName("atlas") + + const allAgents = [ + sisyphus, + hephaestus, + prometheus, + atlas, + "athena", + "explore", + "metis", + "oracle", + ] + + const sorted = simulateOpencodeSort(allAgents, sisyphus) + const orderedConfigKeys = sorted.map((name) => normalizeAgentForPromptKey(name)) + + expect(orderedConfigKeys).toEqual([ + "sisyphus", + "hephaestus", + "prometheus", + "atlas", + "athena", + "explore", + "metis", + "oracle", + ]) + }) + + test("#when default_agent is unset #then canonical core order still holds via prefix alone", () => { + const sisyphus = getAgentRuntimeName("sisyphus") + const hephaestus = getAgentRuntimeName("hephaestus") + const prometheus = getAgentRuntimeName("prometheus") + const atlas = getAgentRuntimeName("atlas") + + const allAgents = [hephaestus, prometheus, atlas, sisyphus, "athena", "oracle"] + + const sorted = simulateOpencodeSort(allAgents, "no-such-default-agent") + const orderedConfigKeys = sorted.map((name) => normalizeAgentForPromptKey(name)) + + expect(orderedConfigKeys.slice(0, 4)).toEqual([ + "sisyphus", + "hephaestus", + "prometheus", + "atlas", + ]) + }) + }) + + describe("#given input array in random order", () => { + test("#when sorted with opencode comparator #then result is always canonical", () => { + const sisyphus = getAgentRuntimeName("sisyphus") + const hephaestus = getAgentRuntimeName("hephaestus") + const prometheus = getAgentRuntimeName("prometheus") + const atlas = getAgentRuntimeName("atlas") + const nonCore = ["athena", "explore", "librarian", "metis", "oracle"] + const allAgents = [...nonCore, atlas, prometheus, hephaestus, sisyphus] + + for (let attempt = 0; attempt < 25; attempt += 1) { + const shuffled = [...allAgents] + for (let i = shuffled.length - 1; i > 0; i -= 1) { + const j = Math.floor(Math.random() * (i + 1)) + ;[shuffled[i], shuffled[j]] = [shuffled[j], shuffled[i]] + } + const sorted = simulateOpencodeSort(shuffled, sisyphus) + const orderedConfigKeys = sorted.map((name) => normalizeAgentForPromptKey(name)) + + expect(orderedConfigKeys).toEqual([ + "sisyphus", + "hephaestus", + "prometheus", + "atlas", + "athena", + "explore", + "librarian", + "metis", + "oracle", + ]) + } + }) + }) + + describe("#given runtime names containing only core agents", () => { + test("#when sorted #then sisyphus, hephaestus, prometheus, atlas in that order", () => { + const sisyphus = getAgentRuntimeName("sisyphus") + const hephaestus = getAgentRuntimeName("hephaestus") + const prometheus = getAgentRuntimeName("prometheus") + const atlas = getAgentRuntimeName("atlas") + + const sorted = simulateOpencodeSort([atlas, prometheus, hephaestus, sisyphus], sisyphus) + const orderedConfigKeys = sorted.map((name) => normalizeAgentForPromptKey(name)) + + expect(orderedConfigKeys).toEqual([ + "sisyphus", + "hephaestus", + "prometheus", + "atlas", + ]) + }) + }) + + describe("#given the prefix is meant to render in OpenCode TUI", () => { + it("uses ASCII whitespace so terminals render the prefix without character corruption", () => { + const runtimeNames = Object.keys(AGENT_DISPLAY_NAMES).map(getAgentRuntimeName) + const invisibleCharsRegex = /[\u200B\u200C\u200D\uFEFF]/ + + for (const name of runtimeNames) { + expect(invisibleCharsRegex.test(name)).toBe(false) + } + }) + + it("only adds leading whitespace, never trailing or interior whitespace beyond the display name", () => { + const sisyphus = getAgentRuntimeName("sisyphus") + const hephaestus = getAgentRuntimeName("hephaestus") + const prometheus = getAgentRuntimeName("prometheus") + const atlas = getAgentRuntimeName("atlas") + + for (const name of [sisyphus, hephaestus, prometheus, atlas]) { + const trimmed = name.trimStart() + expect(name.length).toBeGreaterThanOrEqual(trimmed.length) + expect(trimmed.endsWith(" ")).toBe(false) + } + }) + }) +}) From f100a8565b3a10c5a4883e469048e0bf5c71d7c5 Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Mon, 27 Apr 2026 15:30:20 +0900 Subject: [PATCH 2/2] fix(agents): keep object keys clean by separating list display from runtime name Discovered via post-implementation review (Oracle goal verification): the prior commit's prefix swap (ZWSP -> ASCII spaces) inherited a pre-existing architectural bug from the ZWSP era. `getAgentListDisplayName()` was an alias for `getAgentRuntimeName()`, which meant every callsite that used the "list display" name as an OBJECT KEY (config.agent keys, lookup keys, HTTP-header-bound paths) ended up carrying the sort prefix. This worked silently with ZWSP because zero-width characters are visually invisible. With ASCII space prefixes, the same bug becomes user-visible and violates the explicit RFC 7230 constraint documented in AGENTS.md: "ZWSP MUST NOT appear in object keys (used as HTTP header values)." Fix: separate the two concepts that were conflated. - `getAgentListDisplayName(key)` now returns the CLEAN display name (alias of `getAgentDisplayName`). Used for object keys, config keys, and any path where the name will be sent over HTTP. - `getAgentRuntimeName(key)` keeps its prefixed return value. Used ONLY for the `.name` field that OpenCode reads for `localeCompare` sort. `agent-key-remapper.ts` was already correct: it uses `getAgentRuntimeName` for the `.name` field. The bug was that `getAgentListDisplayName` (used as the object key) also returned the prefix. Test updates: - agent-display-names.test.ts splits the assertions: getAgentListDisplayName asserts clean names, new getAgentRuntimeName describe asserts prefixes - All other tests using getAgentListDisplayName as an expected object key continue to pass because they always wanted clean names Verification: - bun test: 5769 pass / 10 pre-existing failures (unchanged) - bun run typecheck: clean - Manual: agent-key-remapper output keys verified RFC 7230 safe (no leading whitespace, no ZWSP); name fields preserve descending-space prefix for canonical core agent ordering --- src/shared/agent-display-names.test.ts | 25 +++++++++++++++++++------ src/shared/agent-display-names.ts | 17 +++-------------- 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/shared/agent-display-names.test.ts b/src/shared/agent-display-names.test.ts index 0fb52ec06..e4abea669 100644 --- a/src/shared/agent-display-names.test.ts +++ b/src/shared/agent-display-names.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from "bun:test" -import { AGENT_DISPLAY_NAMES, getAgentConfigKey, getAgentDisplayName, getAgentListDisplayName, normalizeAgentForPrompt, normalizeAgentForPromptKey } from "./agent-display-names" +import { AGENT_DISPLAY_NAMES, getAgentConfigKey, getAgentDisplayName, getAgentListDisplayName, getAgentRuntimeName, normalizeAgentForPrompt, normalizeAgentForPromptKey } from "./agent-display-names" describe("getAgentDisplayName", () => { it("returns display name for lowercase config key (new format)", () => { @@ -194,11 +194,11 @@ describe("getAgentConfigKey", () => { }) describe("getAgentListDisplayName", () => { - it("applies leading-space stable-sort prefixes so OpenCode localeCompare yields canonical order", () => { - expect(getAgentListDisplayName("sisyphus")).toBe(" Sisyphus - Ultraworker") - expect(getAgentListDisplayName("hephaestus")).toBe(" Hephaestus - Deep Agent") - expect(getAgentListDisplayName("prometheus")).toBe(" Prometheus - Plan Builder") - expect(getAgentListDisplayName("atlas")).toBe(" Atlas - Plan Executor") + it("returns clean display names for object keys (no leading whitespace, RFC 7230 safe)", () => { + expect(getAgentListDisplayName("sisyphus")).toBe("Sisyphus - Ultraworker") + expect(getAgentListDisplayName("hephaestus")).toBe("Hephaestus - Deep Agent") + expect(getAgentListDisplayName("prometheus")).toBe("Prometheus - Plan Builder") + expect(getAgentListDisplayName("atlas")).toBe("Atlas - Plan Executor") }) it("keeps non-core agents unprefixed for list display", () => { @@ -206,6 +206,19 @@ describe("getAgentListDisplayName", () => { }) }) +describe("getAgentRuntimeName", () => { + it("applies leading-space stable-sort prefixes so OpenCode localeCompare yields canonical order", () => { + expect(getAgentRuntimeName("sisyphus")).toBe(" Sisyphus - Ultraworker") + expect(getAgentRuntimeName("hephaestus")).toBe(" Hephaestus - Deep Agent") + expect(getAgentRuntimeName("prometheus")).toBe(" Prometheus - Plan Builder") + expect(getAgentRuntimeName("atlas")).toBe(" Atlas - Plan Executor") + }) + + it("keeps non-core agents unprefixed (no entry in AGENT_LIST_SORT_PREFIXES)", () => { + expect(getAgentRuntimeName("oracle")).toBe("oracle") + }) +}) + describe("normalizeAgentForPrompt", () => { it("strips core UI ordering prefixes back to canonical display names", () => { expect(normalizeAgentForPrompt(getAgentListDisplayName("sisyphus"))).toBe("Sisyphus - Ultraworker") diff --git a/src/shared/agent-display-names.ts b/src/shared/agent-display-names.ts index 081550d7c..10bf91845 100644 --- a/src/shared/agent-display-names.ts +++ b/src/shared/agent-display-names.ts @@ -50,31 +50,20 @@ export function getAgentRuntimeName(configKey: string): string { return prefix ? `${prefix}${displayName}` : displayName } -/** - * Get display name for an agent config key. - * Uses case-insensitive lookup for backward compatibility. - * Returns original key if not found. - */ export function getAgentDisplayName(configKey: string): string { - // Try exact match first const exactMatch = AGENT_DISPLAY_NAMES[configKey] if (exactMatch !== undefined) return exactMatch - - // Fall back to case-insensitive search + const lowerKey = configKey.toLowerCase() for (const [k, v] of Object.entries(AGENT_DISPLAY_NAMES)) { if (k.toLowerCase() === lowerKey) return v } - - // Unknown agent: return original key + return configKey } -/** - * Runtime-facing agent name used for OpenCode list ordering. - */ export function getAgentListDisplayName(configKey: string): string { - return getAgentRuntimeName(configKey) + return getAgentDisplayName(configKey) } const REVERSE_DISPLAY_NAMES: Record = Object.fromEntries(