Skip to content

refactor: consolidate CLIENT_MAPPINGS + USER_CLIENT_MAPPINGS into a single agent-host registry #377

@christso

Description

@christso

Priority: medium · Pure refactor. Marked `good first issue`. Drift between the two maps already exists in the current code.

Problem

`src/models/client-mapping.ts` defines two parallel records for the same set of clients:

  • `CLIENT_MAPPINGS` — project-scope paths (e.g., `windsurf.skillsPath = '.windsurf/skills/'`).
  • `USER_CLIENT_MAPPINGS` — user-scope paths (e.g., `windsurf.skillsPath = '.codeium/windsurf/skills/'`).

Each is ~130 lines of nearly-identical client entries. Drift is inevitable and already present — `windsurf` has subtly different project and user paths and it's hard to tell at a glance whether that's intentional or accidental. Compare `gh skill`'s registry (`cli/cli` `internal/skills/registry/registry.go`), which uses one slice of `AgentHost` structs, each with paired `ProjectDir` and `UserDir` fields plus a single `InstallDir(scope, gitRoot, homeDir)` resolver.

Current behavior

```bash

Two near-duplicate records with subtle deltas that are hard to review:

$ awk '/^export const (CLIENT_MAPPINGS|USER_CLIENT_MAPPINGS)/,/^};/' src/models/client-mapping.ts | wc -l
260+

Subtle drift example (project vs user windsurf path):

$ grep -A2 'windsurf:' src/models/client-mapping.ts
windsurf: { skillsPath: '.windsurf/skills/', ... } # project
windsurf: { skillsPath: '.codeium/windsurf/skills/', ... } # user
```

Expected behavior

```ts
// Single source of truth, one entry per host:
export interface AgentHost {
id: ClientType;
name: string; // "Windsurf"
project: ClientMapping;
user: ClientMapping;
}

export const AGENT_HOSTS: readonly AgentHost[] = [
{
id: 'windsurf',
name: 'Windsurf',
project: { skillsPath: '.windsurf/skills/', agentFile: 'AGENTS.md' },
user: { skillsPath: '.codeium/windsurf/skills/', agentFile: 'AGENTS.md' },
},
// ...
];

// Plus helpers that everything else uses:
export function findHostById(id: ClientType): AgentHost;
export function getMapping(id: ClientType, scope: 'project' | 'user'): ClientMapping;
export function uniqueProjectSkillsPaths(): string[]; // for the .agents/skills/ dedup logic
export function agentHelpList(): string; // for help output
```

`CLIENT_MAPPINGS` and `USER_CLIENT_MAPPINGS` either become thin views over `AGENT_HOSTS` (preferred — keeps existing call sites working during migration) or are removed entirely (cleaner — do this if every call site is migrated in the same PR).

Verification gate (must pass before closing)

```bash
set -euo pipefail
bun run build
bun run typecheck

(1) Single source of truth: every client in ClientTypeSchema has exactly one entry in AGENT_HOSTS

allagents --help >/dev/null # sanity that build works
node -e "
const { AGENT_HOSTS } = require('./dist/index.js');
const { ClientTypeSchema } = require('./dist/index.js');
const ids = new Set(AGENT_HOSTS.map(h => h.id));
const expected = new Set(ClientTypeSchema.options);
if (ids.size !== expected.size) { console.error('size mismatch'); process.exit(1); }
for (const e of expected) if (!ids.has(e)) { console.error('missing', e); process.exit(1); }
for (const i of ids) if (!expected.has(i)) { console.error('extra', i); process.exit(1); }
// No duplicate ids:
if (ids.size !== AGENT_HOSTS.length) { console.error('duplicate ids'); process.exit(1); }
" 2>/dev/null || true # (skip if internals aren't exported; in that case use a unit test instead)

(2) All existing tests still pass — behavior is unchanged

bun test

(3) getMapping returns identical content for every (client, scope) pair as the old maps

This belongs in a unit test that imports both the new registry and the legacy maps (if kept):

for client of ClientTypeSchema.options:

expect(getMapping(client, 'project')).toEqual(CLIENT_MAPPINGS[client])

expect(getMapping(client, 'user')).toEqual(USER_CLIENT_MAPPINGS[client])

bun test src/models/tests/client-mapping.test.ts

(4) The intentional project/user delta for windsurf (and any others) is preserved

node -e "
const r = require('./dist/index.js');
const w = r.findHostById('windsurf');
if (w.project.skillsPath !== '.windsurf/skills/') process.exit(1);
if (w.user.skillsPath !== '.codeium/windsurf/skills/') process.exit(1);
" 2>/dev/null
```

All four checks must pass. If `dist` doesn't export the internals, replace the inline `node -e` checks with equivalent unit tests under `src/models/tests/`.

Implementation notes

  • The refactor can be staged:
    1. Introduce `AGENT_HOSTS` and helpers alongside the existing maps.
    2. Rewrite `CLIENT_MAPPINGS` and `USER_CLIENT_MAPPINGS` as derived constants (`Object.fromEntries(AGENT_HOSTS.map(...))`). This catches drift at compile time.
    3. (Optional follow-up PR) Migrate call sites from the legacy maps to `getMapping(client, scope)` and delete the derived constants.
  • Add a deliberate test that walks every `ClientTypeSchema` option and asserts `findHostById` succeeds — this catches the case where someone adds a new client to the schema but forgets the registry.
  • Keep `resolveClientMappings` (the vscode→copilot inheritance) but apply it post-lookup; the inheritance rule is orthogonal to the registry shape.
  • The `agentHelpList()` helper has no equivalent today; pattern it after `cli/cli`'s `AgentHelpList()` and use it in the singular-rename issue's `--help` text.

Refs

  • Reference impl: `cli/cli` `internal/skills/registry/registry.go` (`AgentHost` struct, ~40 entries, helpers `FindByID`, `AgentIDs`, `AgentHelpList`, `UniqueProjectDirs`, `InstallDir`).
  • Companion wiki page: `concepts/allagents-vs-gh-skill.md` § "Agent host registry".

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    Status

    Done

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions