fix(agents): replace deprecated getAgentListDisplayName with getAgentDisplayName (fixes #3281)#3407
fix(agents): replace deprecated getAgentListDisplayName with getAgentDisplayName (fixes #3281)#3407MoerAI wants to merge 4 commits intocode-yeongyu:devfrom
Conversation
|
I have read the CLA Document and I hereby sign the CLA |
There was a problem hiding this comment.
3 issues found across 14 files
Confidence score: 2/5
- There is a high-confidence merge risk:
src/plugin-handlers/config-handler.test.tshas a duplicate import specifier, which makes the test file invalid TypeScript and is likely to fail checks immediately. src/plugin-handlers/agent-config-handler.test.tsremovesPluginComponentswhile it is still referenced as a type, creating an unresolved type error that breaks source validity.src/plugin-handlers/agent-key-remapper.test.tsalso appears out of sync with the new display-name sorting behavior, so test expectations should be updated to avoid regression in test outcomes.- Pay close attention to
src/plugin-handlers/config-handler.test.ts,src/plugin-handlers/agent-config-handler.test.ts, andsrc/plugin-handlers/agent-key-remapper.test.ts- compile-time/type validity and test expectation mismatches are likely to block a clean merge.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="src/plugin-handlers/agent-config-handler.test.ts">
<violation number="1" location="src/plugin-handlers/agent-config-handler.test.ts:14">
P1: Custom agent: **Opencode Compatibility**
Removing the `PluginComponents` import leaves an unresolved type reference in this test file, so the change breaks source validity.</violation>
</file>
<file name="src/plugin-handlers/config-handler.test.ts">
<violation number="1" location="src/plugin-handlers/config-handler.test.ts:7">
P0: Custom agent: **Opencode Compatibility**
Duplicate import specifier makes the test file invalid TypeScript.</violation>
</file>
<file name="src/plugin-handlers/agent-key-remapper.test.ts">
<violation number="1" location="src/plugin-handlers/agent-key-remapper.test.ts:99">
P2: Update the expected sorted order to match the clean display names, or this test will fail after removing the ZWSP prefixes.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| getAgentDisplayName("sisyphus"), | ||
| getAgentDisplayName("hephaestus"), | ||
| getAgentDisplayName("prometheus"), | ||
| getAgentDisplayName("atlas"), |
There was a problem hiding this comment.
P2: Update the expected sorted order to match the clean display names, or this test will fail after removing the ZWSP prefixes.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At src/plugin-handlers/agent-key-remapper.test.ts, line 99:
<comment>Update the expected sorted order to match the clean display names, or this test will fail after removing the ZWSP prefixes.</comment>
<file context>
@@ -96,10 +96,10 @@ describe("remapAgentKeysToDisplayNames", () => {
- getAgentListDisplayName("hephaestus"),
- getAgentListDisplayName("prometheus"),
- getAgentListDisplayName("atlas"),
+ getAgentDisplayName("sisyphus"),
+ getAgentDisplayName("hephaestus"),
+ getAgentDisplayName("prometheus"),
</file context>
| getAgentDisplayName("sisyphus"), | |
| getAgentDisplayName("hephaestus"), | |
| getAgentDisplayName("prometheus"), | |
| getAgentDisplayName("atlas"), | |
| getAgentDisplayName("atlas"), | |
| getAgentDisplayName("hephaestus"), | |
| getAgentDisplayName("prometheus"), | |
| getAgentDisplayName("sisyphus"), |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8aaa724932
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (params.config as { default_agent?: string }).default_agent = | ||
| getAgentListDisplayName(configuredDefaultAgent); | ||
| getAgentDisplayName(configuredDefaultAgent); |
There was a problem hiding this comment.
Normalize default_agent through config-key parser
default_agent is now canonicalized with getAgentDisplayName(configuredDefaultAgent), which does not strip legacy ZWSP list prefixes. If an existing config still contains an old prefixed value (for example from prior releases), this assignment keeps the prefix while config.agent keys are remapped to unprefixed display names, so the default agent can stop matching any exported agent key after upgrade. Resolve the input with getAgentConfigKey(...) before converting back to a display name.
Useful? React with 👍 / 👎.
| return (agentResult[getAgentDisplayName(key)] ?? agentResult[key]) as | ||
| | AgentWithPermission |
There was a problem hiding this comment.
Preserve legacy prefixed-key lookup in agentByKey
This helper no longer checks the legacy list-display form, so agent permission patching now misses entries keyed with old prefixed names (for example \u200B...Atlas - Plan Executor). Those legacy keys can still appear in upgraded configs, and when they do, blocks like atlas/sisyphus permission hardening are skipped, leading to inconsistent tool permissions. Keep a prefixed-key fallback or normalize agentResult keys before lookups.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ad09192318
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| for (const [key, value] of Object.entries(agents)) { | ||
| const displayName = getAgentListDisplayName(key) | ||
| const displayName = getAgentDisplayName(key) |
There was a problem hiding this comment.
Normalize legacy prefixed keys before remapping agents
Using getAgentDisplayName(key) directly leaves legacy ZWSP-prefixed keys unchanged, while builtin config keys (for example atlas) are remapped to the new clean display name. In upgraded configs that still contain prefixed keys, this now produces two entries for the same core agent (clean and prefixed), and downstream logic keyed by clean names can skip the legacy entry (ordering/permission updates), causing inconsistent runtime behavior.
Useful? React with 👍 / 👎.
Summary
Problem
getAgentListDisplayName()prepends zero-width space (ZWSP) characters to agent display names for TUI list ordering. These invisible characters leak into HTTP headers, config keys, and agent lookup paths, causing:/compactcommand cannot find default agent)Fix
Replaced all
getAgentListDisplayName()calls in config/tool/command handlers withgetAgentDisplayName(), which returns clean display names without ZWSP prefixes. Theagent-key-remapper.tsnow usesgetAgentDisplayNamefor both object keys and thenamefield. Updated all corresponding test expectations.Changes
src/plugin-handlers/agent-key-remapper.tsgetAgentDisplayNameinstead ofgetAgentListDisplayName/getAgentRuntimeNamesrc/plugin-handlers/agent-config-handler.tsgetAgentDisplayNamefordefault_agentresolutionsrc/plugin-handlers/agent-priority-order.tsgetAgentDisplayNamefor priority ordering keyssrc/plugin-handlers/command-config-handler.tsgetAgentDisplayNamefor command agent refssrc/plugin-handlers/tool-config-handler.tsgetAgentDisplayNamefor tool permission keyssrc/shared/agent-display-names.test.tssrc/plugin-handlers/*.test.tssrc/plugin-interface.test.tssrc/plugin/chat-message.test.tsFixes #3281
Also addresses #3337, #3307, #3347 (ZWSP display issues)
Also addresses #3245 (
/compactcommand agent resolution failure)Summary by cubic
Removed zero‑width space (ZWSP) prefixes from agent names and replaced all uses of
getAgentListDisplayNamewithgetAgentDisplayNameso config, routing, and headers use clean names. Fixes #3281 and also unblocks/compactand resolves TUI ZWSP artifacts (#3337, #3307, #3347, #3245).agent-config-handler,command-config-handler,tool-config-handlernow resolvedefault_agent, refs, and permission keys viagetAgentDisplayName.agent-key-remapperandagent-priority-orderuse clean display names for keys and sorting.getAgentListDisplayNameimports, restored missinggetAgentDisplayNameimports, and updated tests to expect clean names.Written for commit ad09192. Summary will update on new commits.