From 5c60d27cdcb40bb41e71db985d71d2249c9155da Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 11 Apr 2026 13:06:15 +0900 Subject: [PATCH] fix(agents): enforce permanent canonical agent ordering with regression tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The canonical agent order is: sisyphus → hephaestus → prometheus → atlas Changes: - Export CANONICAL_CORE_AGENT_ORDER as single source of truth - Sort non-core agents alphabetically for deterministic ordering - Add 12 regression tests including 100-permutation stability test This permanently resolves the agent ordering saga that caused 15+ commits, 8+ PRs, and multiple reverts due to ZWSP prefixes, Object.entries() order dependencies, and inconsistent merge sequences. Closes #3281 --- .../agent-priority-order.test.ts | 294 ++++++++++++++---- src/plugin-handlers/agent-priority-order.ts | 67 ++-- 2 files changed, 278 insertions(+), 83 deletions(-) diff --git a/src/plugin-handlers/agent-priority-order.test.ts b/src/plugin-handlers/agent-priority-order.test.ts index 541c2dad5..f9e64fa8f 100644 --- a/src/plugin-handlers/agent-priority-order.test.ts +++ b/src/plugin-handlers/agent-priority-order.test.ts @@ -2,81 +2,253 @@ import { describe, expect, test } from "bun:test" -import { reorderAgentsByPriority } from "./agent-priority-order" +import { + reorderAgentsByPriority, + CANONICAL_CORE_AGENT_ORDER, +} from "./agent-priority-order" import { getAgentDisplayName } from "../shared/agent-display-names" -describe("reorderAgentsByPriority", () => { - test("moves core agents to canonical order and injects runtime order fields", () => { - // given +describe("agent-priority-order", () => { + describe("CANONICAL_CORE_AGENT_ORDER", () => { + // given: The canonical order constant must exist and be correct + + test("exports canonical order as readonly array", () => { + // then + expect(CANONICAL_CORE_AGENT_ORDER).toBeDefined() + expect(Array.isArray(CANONICAL_CORE_AGENT_ORDER)).toBe(true) + }) + + test("canonical order is exactly [sisyphus, hephaestus, prometheus, atlas]", () => { + // then + expect(CANONICAL_CORE_AGENT_ORDER).toEqual([ + "sisyphus", + "hephaestus", + "prometheus", + "atlas", + ]) + }) + + test("canonical order length is exactly 4", () => { + // then + expect(CANONICAL_CORE_AGENT_ORDER).toHaveLength(4) + }) + }) + + describe("reorderAgentsByPriority", () => { + // given: display names for all core agents const sisyphus = getAgentDisplayName("sisyphus") const hephaestus = getAgentDisplayName("hephaestus") const prometheus = getAgentDisplayName("prometheus") const atlas = getAgentDisplayName("atlas") const oracle = getAgentDisplayName("oracle") + const librarian = getAgentDisplayName("librarian") + const explore = getAgentDisplayName("explore") - const agents: Record = { - [oracle]: { name: "oracle", mode: "subagent" }, - [atlas]: { name: "atlas", mode: "primary" }, - [prometheus]: { name: "prometheus", mode: "all" }, - [hephaestus]: { name: "hephaestus", mode: "primary" }, - [sisyphus]: { name: "sisyphus", mode: "primary" }, - } + describe("#given agents in random order", () => { + test("#when all core agents present #then orders as sisyphus→hephaestus→prometheus→atlas", () => { + // given: agents in reverse order + const agents: Record = { + [atlas]: { name: "atlas" }, + [prometheus]: { name: "prometheus" }, + [hephaestus]: { name: "hephaestus" }, + [sisyphus]: { name: "sisyphus" }, + } - // when - const result = reorderAgentsByPriority(agents) + // when + const result = reorderAgentsByPriority(agents) - // then - expect(Object.keys(result)).toEqual([ - sisyphus, - hephaestus, - prometheus, - atlas, - oracle, - ]) - expect(result[sisyphus]).toEqual({ - name: "sisyphus", - mode: "primary", - order: 1, + // then + const keys = Object.keys(result) + expect(keys[0]).toBe(sisyphus) + expect(keys[1]).toBe(hephaestus) + expect(keys[2]).toBe(prometheus) + expect(keys[3]).toBe(atlas) + }) + + test("#when core agents mixed with non-core #then core agents come first in canonical order", () => { + // given: mixed order with non-core agents interleaved + const agents: Record = { + [oracle]: { name: "oracle" }, + [atlas]: { name: "atlas" }, + [librarian]: { name: "librarian" }, + [prometheus]: { name: "prometheus" }, + [explore]: { name: "explore" }, + [hephaestus]: { name: "hephaestus" }, + custom: { name: "custom" }, + [sisyphus]: { name: "sisyphus" }, + } + + // when + const result = reorderAgentsByPriority(agents) + + // then + const keys = Object.keys(result) + expect(keys.slice(0, 4)).toEqual([sisyphus, hephaestus, prometheus, atlas]) + }) }) - expect(result[hephaestus]).toEqual({ - name: "hephaestus", - mode: "primary", - order: 2, - }) - expect(result[prometheus]).toEqual({ - name: "prometheus", - mode: "all", - order: 3, - }) - expect(result[atlas]).toEqual({ - name: "atlas", - mode: "primary", - order: 4, - }) - expect(result[oracle]).toEqual({ - name: "oracle", - mode: "subagent", - }) - }) - test("leaves non-object agent configs untouched while still reordering keys", () => { - // given - const sisyphus = getAgentDisplayName("sisyphus") - const atlas = getAgentDisplayName("atlas") + describe("#given 100 random permutations", () => { + test("#when reordered #then result is ALWAYS identical", () => { + // given: base agent config + const baseAgents = { + [sisyphus]: { name: "sisyphus" }, + [hephaestus]: { name: "hephaestus" }, + [prometheus]: { name: "prometheus" }, + [atlas]: { name: "atlas" }, + [oracle]: { name: "oracle" }, + [librarian]: { name: "librarian" }, + custom1: { name: "custom1" }, + custom2: { name: "custom2" }, + } - const agents: Record = { - [atlas]: "atlas-config", - custom: "custom-config", - [sisyphus]: "sisyphus-config", - } + // given: shuffle function + const shuffle = (array: T[]): T[] => { + const result = [...array] + for (let i = result.length - 1; i > 0; i--) { + const j = Math.floor(Math.random() * (i + 1)) + ;[result[i], result[j]] = [result[j], result[i]] + } + return result + } - // when - const result = reorderAgentsByPriority(agents) + // when: run 100 times with different key orders + const results: string[][] = [] + for (let i = 0; i < 100; i++) { + const shuffledKeys = shuffle(Object.keys(baseAgents)) + const shuffledAgents: Record = {} + for (const key of shuffledKeys) { + shuffledAgents[key] = baseAgents[key] + } + const result = reorderAgentsByPriority(shuffledAgents) + results.push(Object.keys(result)) + } - // then - expect(Object.keys(result)).toEqual([sisyphus, atlas, "custom"]) - expect(result[sisyphus]).toBe("sisyphus-config") - expect(result[atlas]).toBe("atlas-config") - expect(result.custom).toBe("custom-config") + // then: all results should have identical key order + const firstResult = results[0] + for (let i = 1; i < results.length; i++) { + expect(results[i]).toEqual(firstResult) + } + + // then: core agents are always first 4 in canonical order + expect(firstResult.slice(0, 4)).toEqual([ + sisyphus, + hephaestus, + prometheus, + atlas, + ]) + }) + }) + + describe("#given partial core agents", () => { + test("#when only sisyphus and atlas present #then orders as sisyphus→atlas", () => { + // given + const agents: Record = { + [atlas]: { name: "atlas" }, + custom: { name: "custom" }, + [sisyphus]: { name: "sisyphus" }, + } + + // when + const result = reorderAgentsByPriority(agents) + + // then + const keys = Object.keys(result) + const sisyphusIdx = keys.indexOf(sisyphus) + const atlasIdx = keys.indexOf(atlas) + expect(sisyphusIdx).toBeLessThan(atlasIdx) + expect(sisyphusIdx).toBe(0) + }) + + test("#when only hephaestus and prometheus present #then orders as hephaestus→prometheus", () => { + // given + const agents: Record = { + [prometheus]: { name: "prometheus" }, + custom: { name: "custom" }, + [hephaestus]: { name: "hephaestus" }, + } + + // when + const result = reorderAgentsByPriority(agents) + + // then + const keys = Object.keys(result) + const hephaestusIdx = keys.indexOf(hephaestus) + const prometheusIdx = keys.indexOf(prometheus) + expect(hephaestusIdx).toBeLessThan(prometheusIdx) + expect(hephaestusIdx).toBe(0) + }) + }) + + describe("#given order field injection", () => { + test("#when core agent is object #then injects order field", () => { + // given + const agents: Record = { + [sisyphus]: { name: "sisyphus", mode: "primary" }, + [hephaestus]: { name: "hephaestus", mode: "primary" }, + [prometheus]: { name: "prometheus", mode: "all" }, + [atlas]: { name: "atlas", mode: "primary" }, + } + + // when + const result = reorderAgentsByPriority(agents) + + // then + expect(result[sisyphus]).toEqual({ name: "sisyphus", mode: "primary", order: 1 }) + expect(result[hephaestus]).toEqual({ name: "hephaestus", mode: "primary", order: 2 }) + expect(result[prometheus]).toEqual({ name: "prometheus", mode: "all", order: 3 }) + expect(result[atlas]).toEqual({ name: "atlas", mode: "primary", order: 4 }) + }) + + test("#when core agent is non-object #then leaves value unchanged", () => { + // given + const agents: Record = { + [sisyphus]: "string-config", + [atlas]: null, + } + + // when + const result = reorderAgentsByPriority(agents) + + // then + expect(result[sisyphus]).toBe("string-config") + expect(result[atlas]).toBe(null) + }) + + test("#when non-core agent #then does NOT inject order field", () => { + // given + const agents: Record = { + [oracle]: { name: "oracle", mode: "subagent" }, + custom: { name: "custom" }, + } + + // when + const result = reorderAgentsByPriority(agents) + + // then + expect(result[oracle]).toEqual({ name: "oracle", mode: "subagent" }) + expect(result.custom).toEqual({ name: "custom" }) + }) + }) + + describe("#given non-core agent ordering", () => { + test("#when multiple non-core agents #then sorted alphabetically after core agents", () => { + // given: non-core agents in random order + const agents: Record = { + zebra: { name: "zebra" }, + [sisyphus]: { name: "sisyphus" }, + apple: { name: "apple" }, + mango: { name: "mango" }, + [atlas]: { name: "atlas" }, + } + + // when + const result = reorderAgentsByPriority(agents) + + // then: core agents first, then alphabetical + const keys = Object.keys(result) + expect(keys.slice(0, 2)).toEqual([sisyphus, atlas]) + expect(keys.slice(2)).toEqual(["apple", "mango", "zebra"]) + }) + }) }) }) diff --git a/src/plugin-handlers/agent-priority-order.ts b/src/plugin-handlers/agent-priority-order.ts index c315ad76a..003679e25 100644 --- a/src/plugin-handlers/agent-priority-order.ts +++ b/src/plugin-handlers/agent-priority-order.ts @@ -1,40 +1,63 @@ -import { getAgentDisplayName } from "../shared/agent-display-names"; +import { getAgentDisplayName } from "../shared/agent-display-names" -const CORE_AGENT_ORDER: ReadonlyArray<{ displayName: string; order: number }> = [ - { displayName: getAgentDisplayName("sisyphus"), order: 1 }, - { displayName: getAgentDisplayName("hephaestus"), order: 2 }, - { displayName: getAgentDisplayName("prometheus"), order: 3 }, - { displayName: getAgentDisplayName("atlas"), order: 4 }, -]; +/** + * CRITICAL: This is the ONLY source of truth for core agent ordering. + * The order is: sisyphus → hephaestus → prometheus → atlas + * + * DO NOT CHANGE THIS ORDER. Any PR attempting to modify this order + * or introduce alternative ordering mechanisms (ZWSP prefixes, sort + * shims, etc.) will be rejected. + * + * See: src/plugin-handlers/AGENTS.md for architectural context. + */ +export const CANONICAL_CORE_AGENT_ORDER = [ + "sisyphus", + "hephaestus", + "prometheus", + "atlas", +] as const -function injectOrderField( - agentConfig: unknown, - order: number, -): unknown { +type CoreAgentName = (typeof CANONICAL_CORE_AGENT_ORDER)[number] + +const CORE_AGENT_ORDER: ReadonlyArray<{ + configKey: CoreAgentName + displayName: string + order: number +}> = CANONICAL_CORE_AGENT_ORDER.map((configKey, index) => ({ + configKey, + displayName: getAgentDisplayName(configKey), + order: index + 1, +})) + +const CORE_DISPLAY_NAMES = new Set(CORE_AGENT_ORDER.map((a) => a.displayName)) + +function injectOrderField(agentConfig: unknown, order: number): unknown { if (typeof agentConfig === "object" && agentConfig !== null) { - return { ...agentConfig, order }; + return { ...agentConfig, order } } - return agentConfig; + return agentConfig } export function reorderAgentsByPriority( agents: Record, ): Record { - const ordered: Record = {}; - const seen = new Set(); + const ordered: Record = {} + const seen = new Set() for (const { displayName, order } of CORE_AGENT_ORDER) { if (Object.prototype.hasOwnProperty.call(agents, displayName)) { - ordered[displayName] = injectOrderField(agents[displayName], order); - seen.add(displayName); + ordered[displayName] = injectOrderField(agents[displayName], order) + seen.add(displayName) } } - for (const [key, value] of Object.entries(agents)) { - if (!seen.has(key)) { - ordered[key] = value; - } + const nonCoreKeys = Object.keys(agents) + .filter((key) => !seen.has(key)) + .sort((a, b) => a.localeCompare(b)) + + for (const key of nonCoreKeys) { + ordered[key] = agents[key] } - return ordered; + return ordered }