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.
This commit is contained in:
@@ -18,12 +18,16 @@ const registeredAgentAliases = new Map<string, string>()
|
||||
|
||||
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 {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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");
|
||||
|
||||
@@ -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", () => {
|
||||
|
||||
@@ -27,10 +27,10 @@ export const AGENT_DISPLAY_NAMES: Record<string, string> = {
|
||||
}
|
||||
|
||||
const AGENT_LIST_SORT_PREFIXES: Record<string, string> = {
|
||||
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 {
|
||||
|
||||
@@ -0,0 +1,152 @@
|
||||
/// <reference types="bun-types" />
|
||||
|
||||
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)
|
||||
}
|
||||
})
|
||||
})
|
||||
})
|
||||
Reference in New Issue
Block a user