Skip to content

Fix duplicate slash alias crash - #805

Closed
giattijunior wants to merge 1 commit into
fathah:mainfrom
giattijunior:fix/duplicate-compact-alias
Closed

giattijunior wants to merge 1 commit into
fathah:mainfrom
giattijunior:fix/duplicate-compact-alias

Conversation

@giattijunior

Copy link
Copy Markdown

Fix duplicate slash alias crash when agent catalog includes /compact

Summary

The renderer crashed when the backend command catalog exposed /compact both as:

  • a standalone command in pairs
  • an alias mapping compact -> compress in canon

createSlashCatalog() registered the command first, then registerAlias() threw Duplicate slash command alias: /compact, which broke the Chat screen on connection.

Fix

Ignore duplicate alias registrations when the canonical command is already present instead of throwing.

Validation

  • Added a regression test for pairs: /compact, /compress plus canon: compact -> compress
  • Ran:
npm test -- src/renderer/src/screens/Chat/slash/commandCatalog.test.ts

Result: 1 passed, 4 passed

@greptile-apps

greptile-apps Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR fixes a renderer crash that occurred when the backend command catalog exposed the same name (e.g., /compact) as both a standalone command in pairs and as an alias key in canon. The fix changes registerAlias to silently return instead of throwing when the alias key is already registered.

  • commandCatalog.ts: registerAlias now returns early instead of throwing on duplicate alias key, preventing the crash for backend catalogs that emit contradictory pairs/canon entries for the same name.
  • commandCatalog.test.ts: Adds a regression test that faithfully reproduces the /compact+/compress scenario and asserts both commands resolve independently after the fix.

Confidence Score: 4/5

The change is safe to merge; it eliminates a real crash with minimal surface area, and the new test directly reproduces the failure scenario.

The crash fix is correct and well-scoped. Two minor concerns: (1) the silent return applies to both command-vs-alias and alias-vs-alias conflicts, so if two independent alias sources ever map the same key to different targets, the second one loses without any observable signal; (2) agentCommandsFromCatalog still emits the contradictory alias into its output rather than filtering it at the source, so the deduplication logic is split across two functions. Neither is a present defect — just edge-case hardening worth revisiting.

The registerAlias function in commandCatalog.ts is the place to revisit if alias-vs-alias conflicts with different targets become possible.

Important Files Changed

Filename Overview
src/renderer/src/screens/Chat/slash/commandCatalog.ts Changes registerAlias to silently return (instead of throw) when the alias key is already present; fixes the crash but the silent-drop applies to alias-vs-alias conflicts too, not just command-vs-alias ones
src/renderer/src/screens/Chat/slash/commandCatalog.test.ts Adds a regression test that exercises pairs: /compact, /compress + canon: compact → compress; accurately reproduces the crash scenario and verifies both commands resolve correctly after the fix

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant B as Backend Catalog
    participant A as agentCommandsFromCatalog
    participant C as createSlashCatalog
    participant R as registerAlias

    B->>A: "{ pairs: [[/compact,...],[/compress,...]], canon: {compact: compress} }"
    A->>A: "Build seen = {compact, compress}"
    A->>A: "Emit alias compact→compress (seen.has(compress) = true)"
    A-->>C: "{ commands: [compact, compress], aliases: {compact: compress} }"

    C->>R: registerCommand(compact) ✅
    C->>R: registerCommand(compress) ✅
    C->>R: registerAlias(compact, compress)
    Note over R: byName.has(compact) = true
    R-->>C: return (silent, was: throw) ✅ fix

    Note over C: /compact → compact cmd
    Note over C: /compress → compress cmd
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant B as Backend Catalog
    participant A as agentCommandsFromCatalog
    participant C as createSlashCatalog
    participant R as registerAlias

    B->>A: "{ pairs: [[/compact,...],[/compress,...]], canon: {compact: compress} }"
    A->>A: "Build seen = {compact, compress}"
    A->>A: "Emit alias compact→compress (seen.has(compress) = true)"
    A-->>C: "{ commands: [compact, compress], aliases: {compact: compress} }"

    C->>R: registerCommand(compact) ✅
    C->>R: registerCommand(compress) ✅
    C->>R: registerAlias(compact, compress)
    Note over R: byName.has(compact) = true
    R-->>C: return (silent, was: throw) ✅ fix

    Note over C: /compact → compact cmd
    Note over C: /compress → compress cmd
Loading

Comments Outside Diff (1)

  1. src/renderer/src/screens/Chat/slash/commandCatalog.ts, line 120-124 (link)

    P2 agentCommandsFromCatalog does not pre-filter command-shadowed aliases

    When a canon entry's alias key (compact) is already in seen as a canonical command, agentCommandsFromCatalog still emits it into the returned aliases record. The duplicate is only suppressed later inside registerAlias. Adding || seen.has(alias) to the continue-condition on line 123 would prevent the contradictory alias from ever leaving this function, making the intent self-documenting and keeping registerAlias a pure invariant-checker rather than a policy-applier. As-is, the fix works but the deduplication logic is split across two functions that are hard to reason about together.

    Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Reviews (1): Last reviewed commit: "Fix duplicate slash alias crash" | Re-trigger Greptile

Comment on lines 45 to 47
if (byName.has(aliasKey) || aliases.has(aliasKey)) {
throw new Error(`Duplicate slash command alias: /${aliasKey}`);
return;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Silent alias-vs-alias conflict silently drops second mapping

The early return now fires both when the alias key collides with a canonical command (the intended fix) and when it collides with an already-registered alias pointing to a different target. In the second case — e.g., if agent commands and desktop commands each define "/r" → different targets — the second registration silently loses with no observable feedback. The upstream agentCommandsFromCatalog protects against this today because Object.entries de-dupes object keys, but future callers that supply aliases from two independent sources could hit it and get the wrong resolution without any warning. A dev-mode console.warn on the aliases.has(aliasKey) branch would make this detectable.

@fathah

fathah commented Jul 4, 2026

Copy link
Copy Markdown
Owner

Thanks for tracking this down and for the regression test — the diagnosis of the /compact pairs-vs-canon collision was exactly right.

This has since been fixed on main by #808, which resolves the same collision one layer earlier: agentCommandsFromCatalog now drops a canon alias whose name is already a first-class pairs command, so the reconciled catalog is self-consistent before it ever reaches createSlashCatalog. Your repro scenario (pairs: /compact, /compress + canon: compact → compress) is covered by a regression test there.

We deliberately kept the fix out of registerAlias: its throw-on-duplicate is intentional validation for genuine desktop-authoring conflicts (two in-repo commands claiming the same name), and silently returning would mask those at startup. The design rule we've settled on — also applied in #818 for the related backend-command-vs-desktop-alias case — is that backend catalog data is untrusted runtime input and gets sanitized at the reconciliation layer, while createSlashCatalog's throw stays strict for in-repo authoring mistakes.

Closing as superseded by #808. Thanks again for the report and the clean repro — it helped confirm the fix covers this exact case.

@fathah fathah closed this Jul 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants