feat: add custom agent display names via config

[potb]

Summary

Closes #1715

Adds agent_display_names config property so users can rename agents to custom display names.

{
  "agent_display_names": {
    "sisyphus": "Builder",
    "oracle": "Advisor",
    "prometheus": "Planner"
  }
}

---

Summary by cubic

Adds custom agent display names via agent_display_names and makes renamed agents work end-to-end. All comparisons use canonical names; SDK prompts send registered names.

• New Features

  • Config: agent_display_names in schema and merge; initialized at startup; re-keys config.agent, agentResult, and default_agent; bidirectional toCanonical/toRegistered with collision warnings.
  • Canonicalization: applied to delegate-task (plan family + Sisyphus-Junior), call-omo-agent (validation + toRegistered), look_at (metadata + prompt), background-task titles, tool restrictions, orchestrator checks, skipAgents, multimodal metadata, and disabled_agents.
  • Tests: added for schema, alias round-trip/collisions, re-keying, tool restrictions, orchestrator/compaction filters, disabled_agents, and session-utils (fs mock + cleanup).

• Bug Fixes

  • CI: exclude src/shared/session-utils.test.ts from batch to avoid fs mock pollution; fixed workflow YAML indentation and split mock-heavy tests into isolated steps.
  • Stability: reset global alias/display-name state across tests.

^{Written for commit 803c1d0. Summary will update on new commits.}

[cubic-dev-ai]

1 issue found across 7 files

Confidence score: 4/5

• Test cleanup in src/shared/agent-config-integration.test.ts can be skipped on assertion failure, which risks shared state leaking into later tests and causing flaky behavior.
• Overall risk is low because the issue is limited to test isolation rather than production code paths.
• Pay close attention to src/shared/agent-config-integration.test.ts - ensure cleanup runs via afterEach or try/finally.

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/shared/agent-config-integration.test.ts">

<violation number="1" location="src/shared/agent-config-integration.test.ts:244">
P2: Cleanup relies on the final line of the test body, so any failed assertion prevents reset and can leak shared state into subsequent tests. Use afterEach/try/finally to ensure reset always runs.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[potb]

@cubic-dev-ai recheck

[cubic-dev-ai]

@cubic-dev-ai recheck

@potb I have started the AI code review. It will take a few minutes to complete.

[cubic-dev-ai]

3 issues found across 34 files

Confidence score: 4/5

• Safe to merge overall; the issues are low-severity and test-only, so risk to production behavior is minimal.
• Most notable risk is order-dependent test failures from global agent alias state not being reset in src/tools/delegate-task/sync-continuation.test.ts and src/tools/delegate-task/sync-prompt-sender.test.ts, which can cause flaky CI.
• Placeholder tests in src/shared/session-utils.test.ts don’t exercise isCallerOrchestrator, so they don’t validate the intended behavior but won’t break runtime logic.
• Pay close attention to src/tools/delegate-task/sync-continuation.test.ts, src/tools/delegate-task/sync-prompt-sender.test.ts, src/shared/session-utils.test.ts - global alias state leakage and missing assertions.

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/tools/delegate-task/sync-continuation.test.ts">

<violation number="1" location="src/tools/delegate-task/sync-continuation.test.ts:371">
P2: Global agent alias state initialized in tests is never reset, which can leak into subsequent tests and make the suite order-dependent.</violation>
</file>

<file name="src/shared/session-utils.test.ts">

<violation number="1" location="src/shared/session-utils.test.ts:17">
P3: These tests are placeholders: they never call `isCallerOrchestrator` or assert its behavior, so they don’t validate the orchestrator alias logic described in the test names.</violation>
</file>

<file name="src/tools/delegate-task/sync-prompt-sender.test.ts">

<violation number="1" location="src/tools/delegate-task/sync-prompt-sender.test.ts:130">
P2: Tests initialize global agent alias state without resetting it, which can leak alias configuration across tests in the same Bun process and cause order-dependent failures. Add resetAgentNameAliases in an afterEach/beforeEach for this file.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[potb]

Tested locally, works properly

[cubic-dev-ai]

2 issues found across 3 files (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/shared/session-utils.test.ts">

<violation number="1" location="src/shared/session-utils.test.ts:11">
P2: `spyOn(sessionUtils, "getMessageDir")` won’t affect `isCallerOrchestrator` because it calls the local `getMessageDir` binding directly. The tests may still hit the real filesystem, making the mock ineffective and the tests unreliable.</violation>

<violation number="2" location="src/shared/session-utils.test.ts:11">
P2: Spies created with `spyOn` are never restored, which can leak mocked implementations across tests in bun:test and cause test pollution. Add an `afterEach` (e.g., `mock.restore()` or `getMessageDirSpy.mockRestore()`) to clean up spies after each test.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[cubic-dev-ai]

1 issue found across 1 file (changes from recent commits).

Prompt for AI agents (all issues)

Check if these issues are valid — if so, understand the root cause of each and fix them.


<file name="src/shared/session-utils.test.ts">

<violation number="1" location="src/shared/session-utils.test.ts:7">
P2: Global fs mock makes existsSync always return true, so getMessageDir never returns null. The test "returns false when no message directory exists" therefore doesn’t exercise the missing-directory case and only passes because findNearestMessageWithFields returns null.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

[malabarbamba]

For some reasons worked for all agent but not for "prometheus"

[jardo5]

@potb @code-yeongyu Actual amazing PR. Love the names but can be confusing at times.

[code-yeongyu]

PR Review: feat: add custom agent display names via config

Summary

This PR adds an agent_display_names config property allowing users to rename agents to custom display names. It introduces:

• Schema: agent_display_names: Record<string, string> in OhMyOpenCodeConfigSchema
• New modules: agent-name-aliases.ts (bidirectional alias mapping) + agent-display-name-rekeyer.ts (config re-keying)
• Canonicalization: toCanonical() / toRegistered() functions applied across ~15 call sites (delegate-task, call-omo-agent, look-at, background-task, session-utils, tool-registry, idle-event)
• Tests: ~1200 lines of new tests across 9 test files

Positive Feedback

1. Architecture is sound. Two separate concerns are cleanly split:

   • agent-display-names.ts → UI display name overrides (user-facing labels)
   • agent-name-aliases.ts → bidirectional canonical↔registered mapping (SDK routing)
     This separation aligns with the existing codebase's SRP patterns.

2. Collision detection is well thought out. initializeAgentNameAliases validates for:

   • Unknown canonical names
   • Duplicate aliases
   • Aliases colliding with existing canonical names
     All returning structured warnings rather than throwing — correct approach for config validation.

3. Test coverage is thorough. BDD-style comments (//#given, //#when, //#then), round-trip tests, edge cases (empty config, case-insensitivity, reset), integration tests for config-handler and idle-event.

4. Files stay under 200 LOC. agent-name-aliases.ts (76 LOC), agent-display-name-rekeyer.ts (43 LOC) — well within limits.

---

Issues Requiring Changes

1. CRITICAL: Dual initialization creates a timing/state inconsistency

initializeAgentNameAliases is called in two separate places:

• src/index.ts:27 → initializeAgentDisplayNames(pluginConfig.agent_display_names) (sets UI overrides)
• src/plugin-handlers/agent-display-name-rekeyer.ts:18 → initializeAgentNameAliases(displayNames, Object.keys(agentResult)) (sets bidirectional mapping)

The problem: initializeAgentNameAliases resets its internal Maps on every call (lines 13-14 of agent-name-aliases.ts). So the first initialization in index.ts calls initializeAgentDisplayNames which does NOT call initializeAgentNameAliases. Then later the rekeyer calls initializeAgentNameAliases during the config hook.

But there's a timing gap: any toCanonical() calls that happen between plugin init and the config hook will operate without aliases. This is fragile. Consider either:

• Unifying initialization into one place (rekeyer), or
• Documenting clearly that toCanonical is not safe to call before config hook completes

2. CRITICAL: Schema field lacks JSDoc comment

Every field in OhMyOpenCodeConfigSchema has a /** ... */ JSDoc comment except the new agent_display_names:

/** Disable specific tools by name (e.g., ["todowrite", "todoread"]) */
disabled_tools: z.array(z.string()).optional(),
agent_display_names: z.record(z.string(), z.string()).optional(),  // <-- no JSDoc

Add: /** Map agent canonical names to custom display names (e.g., { "sisyphus": "Builder" }) */

3. Whitespace-only changes pollute the diff

Multiple files have indentation changes on lines that have no functional modifications (e.g., idle-event.ts, tools.ts, sync-executor.ts, todo-continuation-enforcer.test.ts, config-handler.test.ts). The entire try block in sync-executor.ts lines 30-42 is re-indented without functional changes.

This makes review harder and creates unnecessary merge conflicts. Please revert whitespace-only changes to keep the diff focused on the actual feature.

4. mergeConfigs doesn't follow existing pattern for agent_display_names

In plugin-config.ts, agent_display_names uses deepMerge but agents already uses deepMerge:

agent_display_names: deepMerge(base.agent_display_names ?? {}, override.agent_display_names ?? {}),

For Record<string, string>, deepMerge is overkill — a simple spread { ...(base.agent_display_names ?? {}), ...(override.agent_display_names ?? {}) } is sufficient and more explicit. deepMerge is designed for nested objects with recursion, proto-pollution safety, and MAX_DEPTH=50 — none of which apply to a flat string-to-string map.

5. Missing newline at EOF in two files

Both agent-display-names.ts and agent-display-names.test.ts are missing trailing newlines (visible from \ No newline at end of file in the diff). This is a minor convention issue but should be fixed.

6. toCanonical in call-omo-agent/tools.ts normalizes then casts

const normalizedAgent = toCanonical(args.subagent_type).toLowerCase() as AllowedAgentType

After toCanonical, the value is already lowercased (the function returns aliasToCanonical.get(lower) ?? lower). The .toLowerCase() is redundant but harmless. However, the as AllowedAgentType cast after canonicalization is unsafe — if the alias maps to something not in AllowedAgentType, this silently passes. Consider adding a runtime check.

7. Removed JSDoc comments from agent-display-names.ts

The PR removes the module-level doc comment and function-level JSDoc from agent-display-names.ts:

-/**
- * Agent config keys to display names mapping.
- * Config keys are lowercase (e.g., "sisyphus", "atlas").
- * Display names include suffixes for UI/logs (e.g., "Sisyphus (Ultraworker)").
- */

These were helpful documentation. The new initializeAgentDisplayNames, resetAgentDisplayNames, and getAgentDisplayName functions also lack JSDoc. Please preserve or update the existing documentation.

---

Minor Observations (Non-blocking)

• The test comment style mixes // given and //#given — both are used in the codebase but this PR uses both in the same new test files. Consider consistency within each file.
• agent-display-name-rekeyer.test.ts uses spyOn(shared, "log" as any) — the as any is technically an anti-pattern per project rules but is pragmatic for mocking.

Verdict: Request Changes

The core architecture is good and the test coverage is impressive. The main issues are:

1. The dual initialization timing gap (critical for correctness)
2. Missing JSDoc on schema field
3. Whitespace pollution in the diff

Fix these and this is ready to merge.