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) + } + }) + }) +})