Revert "Merge pull request #3657 from code-yeongyu/refactor/replace-zwsp-with-real-spaces"
This reverts commitf1a11f2c92, reversing changes made to62c19ce0ef.
This commit is contained in:
YeonGyu-Kim
@@ -18,16 +18,12 @@ 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 stripSortPrefix(name).toLowerCase()
|
||||
return name.replace(ZERO_WIDTH_CHARACTERS_REGEX, "").toLowerCase()
|
||||
}
|
||||
|
||||
function normalizeStoredAgentName(name: string): string {
|
||||
return stripSortPrefix(name)
|
||||
return name.replace(ZERO_WIDTH_CHARACTERS_REGEX, "")
|
||||
}
|
||||
|
||||
export function registerAgentName(name: string): void {
|
||||
|
||||
@@ -8,45 +8,28 @@ 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 leading-space-prefixed runtime names into the `name` field for OpenCode's `localeCompare` sort
|
||||
2. `agent-key-remapper.ts` injects ZWSP-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 leading ASCII spaces (4-3-2-1 descending) 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 ZWSP prefixes (1-4 chars) so core agents sort before alphabetically-named agents.
|
||||
|
||||
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:
|
||||
ZWSP is intentionally used in the `name` field only. It 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 caused 15+ commits, 8+ PRs, and multiple reverts due to:
|
||||
Agent ordering has 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 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()`)
|
||||
- ZWSP in object keys or display names (only allowed 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 { AGENT_DISPLAY_NAMES, getAgentConfigKey, getAgentRuntimeName } from "../shared/agent-display-names";
|
||||
import { getAgentRuntimeName } from "../shared/agent-display-names";
|
||||
import { AGENT_NAME_MAP } from "../shared/migration";
|
||||
import { registerAgentName } from "../features/claude-code-session-state";
|
||||
import {
|
||||
@@ -189,11 +189,8 @@ export async function applyAgentConfig(params: {
|
||||
|
||||
if (isSisyphusEnabled && builtinAgents.sisyphus) {
|
||||
if (configuredDefaultAgent) {
|
||||
const configKey = getAgentConfigKey(configuredDefaultAgent);
|
||||
const isKnownBuiltin = configKey in AGENT_DISPLAY_NAMES;
|
||||
(params.config as { default_agent?: string }).default_agent = isKnownBuiltin
|
||||
? getAgentRuntimeName(configKey)
|
||||
: configuredDefaultAgent;
|
||||
(params.config as { default_agent?: string }).default_agent =
|
||||
getAgentRuntimeName(configuredDefaultAgent);
|
||||
} else {
|
||||
(params.config as { default_agent?: string }).default_agent =
|
||||
getAgentRuntimeName("sisyphus");
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, it, expect } from "bun:test"
|
||||
import { AGENT_DISPLAY_NAMES, getAgentConfigKey, getAgentDisplayName, getAgentListDisplayName, getAgentRuntimeName, normalizeAgentForPrompt, normalizeAgentForPromptKey } from "./agent-display-names"
|
||||
import { AGENT_DISPLAY_NAMES, getAgentConfigKey, getAgentDisplayName, getAgentListDisplayName, 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("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("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("keeps non-core agents unprefixed for list display", () => {
|
||||
@@ -206,19 +206,6 @@ 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")
|
||||
|
||||
@@ -27,10 +27,10 @@ export const AGENT_DISPLAY_NAMES: Record<string, string> = {
|
||||
}
|
||||
|
||||
const AGENT_LIST_SORT_PREFIXES: Record<string, string> = {
|
||||
sisyphus: " ",
|
||||
hephaestus: " ",
|
||||
prometheus: " ",
|
||||
atlas: " ",
|
||||
sisyphus: "\u200B",
|
||||
hephaestus: "\u200B\u200B",
|
||||
prometheus: "\u200B\u200B\u200B",
|
||||
atlas: "\u200B\u200B\u200B\u200B",
|
||||
}
|
||||
|
||||
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).replace(/^\s+/, "")
|
||||
return stripInvisibleAgentCharacters(agentName)
|
||||
}
|
||||
|
||||
export function getAgentRuntimeName(configKey: string): string {
|
||||
@@ -50,20 +50,31 @@ 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 getAgentDisplayName(configKey)
|
||||
return getAgentRuntimeName(configKey)
|
||||
}
|
||||
|
||||
const REVERSE_DISPLAY_NAMES: Record<string, string> = Object.fromEntries(
|
||||
|
||||
@@ -1,152 +0,0 @@
|
||||
/// <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