[CSalcedoDataBI]

Closes #758

What

skills-audit gets a new static finding, mcp-tool-name. It catches a skill whose SKILL.md or references/ hardcode an MCP tool name under a prefix the agent does not have. The typical case is mcp__powerbi-modeling-mcp__connection_operations while the server comes from a plugin as mcp__plugin_fabric-skills_powerbi-modeling-mcp__…, or the reverse. The finding gives the name that works on this machine (in a local-only working field; the detail that may be filed names only the install form) and recommends the server-agnostic wording ("the powerbi-modeling-mcp connection_operations tool").

• Get-McpServerNames.ps1 (new) lists the prefixes the agent really sees. It reads ~/.claude.json (user and per-project entries), the project .mcp.json and the enabled plugins' manifests. It skips what is not loaded:

  • disabled plugins (user < project < local settings);
  • project-scope installs of another project;
  • disabledMcpServers;
  • unapproved or rejected .mcp.json servers.

  Names are normalized the way Claude Code builds tool names. It does not call claude mcp list, because that command health-checks every server and does not show the plugin prefix.

• Invoke-SkillAudit.ps1 checks these forms: mcp__<prefix>__<tool>, __* and the bare mcp__<server> used in allowed-tools. It reports only when the server IS available under another prefix: a server that is not installed says nothing. It does not flag:

  • a plugin skill that names its own plugin prefix;
  • a skill that already gives the working name too.

  Copies of a skill are folded into one finding. A discovery failure skips only this check. -McpServers injects the prefix set.

Tests

tests/agentic-board/SkillAudit.McpToolNames.Tests.ps1 has 54 tests, all on fixtures. The audit's own discovery path is driven through a stub copy of the scripts, never the live config. A live run on the maintainer's machine flagged real skills that hardcode mcp__powerbi-modeling-mcp__* where that server comes from a plugin.

Closes #758

🤖 Generated with Claude Code

[claude]

Review (sha 1f06ff3) — check mcp-tool-name en skills-audit + Get-McpServerNames.ps1.

En general está bien: es solo lectura, no expone valores de config (solo nombres de servidores), degrada con Write-Warning si falla la discovery y solo reporta cuando existe un nombre alternativo que funciona. No veo bugs bloqueantes ni problemas de seguridad. Observaciones menores:

1. Requisito de PowerShell 7: ConvertFrom-Json -AsHashtable (Read-Json en Get-McpServerNames.ps1) no existe en Windows PowerShell 5.1. Ahí Read-Json capturaría el error y emitiría un warning por archivo, dejando la lista vacía y desactivando el check casi en silencio. Conviene #requires -Version 7 o un warning explícito.
2. Ambigüedad de plugin_<plugin>_<server>: con -McpServers (strings sueltos) el split se hace en el primer _, así que un plugin con _ en el nombre produce un server incorrecto. Está comentado en el código; un test que fije ese comportamiento evitaría regresiones.
3. Mantenibilidad: el bloque nuevo en Invoke-SkillAudit.ps1 (~75 líneas, con Resolve-RefServer definida dentro de un if) sería más legible extraído a función o script propio, como ya se hizo con Get-McpServerNames.ps1.

Tests: la cobertura nueva (SkillAudit.McpToolNames.Tests.ps1) parece amplia; no la ejecuté en esta revisión.

[claude]

Sin hallazgos bloqueantes. Revisé Get-McpServerNames.ps1, el check mcp-tool-name en Invoke-SkillAudit.ps1, la doc de SKILL.md y los tests. No ejecuté los tests de Pester.

• El descubrimiento es de solo lectura. No hay inyección ni secretos, porque solo lee JSON de configuración y nunca ejecuta su contenido.
• Si falla el descubrimiento, solo se omite este check y se emite un warning.
• La resolución de plugin_<plugin>_<server> con _ en el nombre del plugin y el filtro de plugins/servidores no cargados están bien cubiertos por los tests.
• Observación menor, no bloqueante: -AsHashtable requiere PowerShell 7+. Conviene confirmar que Invoke-SkillAudit.ps1 ya exige esa versión; si no, falla con un mensaje poco claro.

[claude]

Review (sha 0ffda07)

Sin hallazgos bloqueantes. El check mcp-tool-name y Get-McpServerNames.ps1 son solo lectura, no ejecutan nada externo y manejan bien los fallos (la discovery falla aislada, JSON ilegible se omite con warning). Los tests cubren los casos principales.

Observaciones menores, no bloqueantes:

• Con -McpServers los prefijos de plugin se separan en el primer _. Un plugin con _ en el nombre se resuelve mal, aunque la discovery real no tiene ese problema. Conviene dejarlo documentado.
• SKILL.md y el comentario de summary quedan claros. Considera un test para mcpCheck = 'discovery-failed' si todavía no existe.

[claude]

Revisé el diff (Get-McpServerNames.ps1, el check mcp-tool-name en Invoke-SkillAudit.ps1, SKILL.md y los tests Pester): sin hallazgos bloqueantes. Es solo lectura, sin secretos ni ejecución de entrada externa, y los tests usan fixtures sin tocar la config real. Cubren los casos límite principales: prefijo de plugin, placeholders, README ignorado, allowed-tools y fallo de discovery. No ejecuté los tests Pester en esta revisión.

[claude]

Review (sha a7fd0f5): sin hallazgos bloqueantes.

La lógica de descubrimiento y de detección (mcp-tool-name) es coherente: solo lectura, sin shell-out, sin secretos ni inyección, y los errores de config core fallan de forma explícita. Los tests nuevos (~440 líneas) cubren los casos principales.

Notas menores, no bloqueantes:

• ConvertFrom-Json -AsHashtable exige PowerShell 7+. En 5.1 la lectura de un archivo requerido lanza, y el check se salta con discovery-failed. Conviene documentar el requisito o agregar un test para ese caso.
• Resolve-RefServer asume plugins en kebab-case (primer _ después de plugin_). Es una limitación aceptada y documentada en el comentario, pero un plugin con _ en el nombre en una referencia hardcodeada no se detectará.

[claude]

Sin hallazgos bloqueantes. Revisé el diff leyéndolo, sin ejecutar los Pester (el entorno no me dejó correr pwsh).

• Seguridad: Get-McpServerNames.ps1 es de solo lectura y emite únicamente nombres de servidores, no valores de config. ~/.claude.json puede contener secretos, pero no se vuelcan. Los mensajes de error solo incluyen la ruta y el mensaje de la excepción.
• Lógica: el manejo de -AsHashtable (claves que difieren solo en mayúsculas), la precedencia user < project < local y el fallo ruidoso en config core coinciden con lo que dicen los comentarios. -AsHashtable ya se usa en otros scripts del repo.
• Tests: la cobertura de fixtures es amplia (468 líneas, discovery y audit por separado).

Nit, no bloqueante: Resolve-RefServer parte el prefijo de plugin en el primer _, así que un plugin con _ en el nombre produce un falso negativo (no un falso positivo). Esto queda documentado en el comentario, así que basta con dejarlo como está.

[claude]

Revisé el diff (Get-McpServerNames.ps1, el chequeo mcp-tool-name en Invoke-SkillAudit.ps1, SKILL.md y los tests). No encontré bugs ni problemas de seguridad que bloqueen el merge.

• Los errores de lectura de config fallan en voz alta y el chequeo se omite sin romper el resto de la auditoría. summary.mcpCheck lo deja visible.
• El detail no incluye nombres de plugins locales; van solo en working.
• Los tests usan fixtures y no tocan la config real.
• Nit (opcional): -AsHashtable requiere PowerShell 7+. Conviene confirmar que el resto de los scripts ya exige pwsh 7 y, si no, declararlo con #Requires -Version 7.
• Nit (opcional): $findings[$findings.Count - 1] asume que Add-Finding siempre agrega un elemento. Si algún día deduplica, working se adjuntaría al finding equivocado. Se podría hacer que Add-Finding devuelva el objeto.

[claude]

Sin hallazgos bloqueantes. Revisé Get-McpServerNames.ps1 y el check mcp-tool-name en Invoke-SkillAudit.ps1: solo lectura, sin ejecución de comandos ni secretos, los fallos de discovery degradan solo este check (con mcpCheck = discovery-failed), y el detalle del finding no filtra nombres de plugins locales. Los tests cubren bien los casos límite (case exacto, placeholders, plugin propio, scopes).

Nota menor, no bloqueante: ConvertFrom-Json -AsHashtable exige PowerShell 7+; si el script debe correr en Windows PowerShell 5.1, fallará (en el audit queda como discovery-failed, no rompe el resto).

[CSalcedoDataBI]

External review - claude-review (Claude API, clean context)

Reviewed commit: da0e962dd39de87108476aa8fcaf32eab5004ce8

12 rounds with claude-review (Claude API, Opus 5.5, xhigh, clean context; same model family as the implementer, an independent context, not another vendor). R1-4 on the whole diff, R5-12 per file with tests separate. Every finding verified in the source; about 45 fixed with tests, and 6 rejected with a reason: inventory paths are already forward-slash; existing fixtures have no mcp refs; strict:false marketplace mcpServers are out of scope; the local-plugin-names finding was rejected 3 times, then adopted in R10 (local-only 'working' field). Final state: Get-McpServerNames reviewed clean in R9 and unchanged since. The tests file was 'patch is correct' in R12, and its 3 findings are fixed in da0e962. Invoke-SkillAudit.ps1 was last reviewed in full in R11; its R11 fixes (installed server spelling, placeholder covers plugin refs) were NOT re-reviewed by the API, because R12 hit max_tokens twice (mechanical; stopped per coordinator rule). They are covered by new exact-case tests. 54 fixture tests in SkillAudit.McpToolNames.Tests.ps1; 62 green with Invoke-SkillAudit.Tests.ps1.

[claude]

Revisión del diff (Get-McpServerNames.ps1, Invoke-SkillAudit.ps1, SKILL.md, tests): no encontré bugs bloqueantes ni problemas de seguridad. La lectura de config es de solo lectura, el detalle que podría subirse upstream no incluye nombres de plugins locales, y un fallo de discovery solo omite este check.

Observaciones menores, no bloqueantes:

• Invoke-SkillAudit.ps1: el bloque de MCP (~140 líneas, regex y reglas de exclusión muy densas) vive inline. Extraerlo a una función o script propio, como Get-McpServerNames.ps1, facilitaría testearlo y mantenerlo.
• Get-McpServerNames.ps1: settings.json se lee con -Required, así que un settings con comentarios o comas finales hace fallar el discovery y se salta el check, con warning. Es aceptable, pero conviene tenerlo presente si aparecen reportes de "check skipped".
• Get-McpServerNames.ps1: [IO.Path]::Combine($base, $ms) con un mcpServers de plugin.json que sea ruta absoluta o con .. lee fuera del plugin. Solo se leen claves JSON, así que el riesgo es bajo, pero se podría validar que la ruta quede bajo $base.
• Los tests no se ejecutaron en esta revisión.

[CSalcedoDataBI]

External review - claude-review (Claude API, clean context)

Reviewed commit: ac64c2fb759bdaf82f18a21c784f897018a86380

12 rounds with claude-review (Claude API, Opus 5.5, xhigh, clean context; same model family as the implementer, an independent context, not another vendor). R1-4 on the whole diff, R5-12 per file with tests separate. Every finding verified in the source; about 45 fixed with tests, and 6 rejected with a reason (inventory paths are already forward-slash; existing fixtures have no mcp refs; strict:false marketplace mcpServers are out of scope); the local-plugin-names finding was adopted in R10 (local-only 'working' field). Get-McpServerNames reviewed clean in R9 and unchanged since. The tests file was 'patch is correct' in R12; its 3 findings are fixed, plus a Linux dot-file fix found by CI. Invoke-SkillAudit.ps1 was last reviewed in full in R11; its R11 fixes (installed server spelling, placeholder covers plugin refs) were NOT re-reviewed by the API, because R12 hit max_tokens twice (mechanical; stopped per coordinator rule). They are covered by exact-case tests. 54 fixture tests, 62 green locally with Invoke-SkillAudit.Tests.ps1.

[claude]

Sin hallazgos bloqueantes. Revisé Get-McpServerNames.ps1 y el nuevo chequeo mcp-tool-name en Invoke-SkillAudit.ps1. Lo que vi:

• Es de solo lectura y no ejecuta comandos externos.
• El detalle que puede archivarse no incluye nombres de plugins locales; esos van en working.
• Si falla el descubrimiento, solo se omite este chequeo y queda indicado en mcpCheck.

No ejecuté los tests (los 54 de SkillAudit.McpToolNames.Tests.ps1); este review es solo de lectura del diff.

Nit opcional: -AsHashtable exige PowerShell 7+. Si el repo soporta 5.1, Get-McpServerNames fallaría y el chequeo se omitiría con el warning de descubrimiento.

[CSalcedoDataBI]

On hold: waiting for the Anthropic plugin directory review. Do not merge yet.

The agentic-board plugin is in its first review in the Anthropic plugin directory, and the directory tracks main. Every merge to main becomes a new version that replaces the one under review and restarts the queue. This PR stays on its branch until the plugin leaves review, or until the owner decides to merge the whole batch at once.

Batch waiting together: #799, #800, #801, #802, #803, #804.

• cleanup sessions: squash-merged work is classified as "park", so the plan opens duplicate draft PRs #799 goes first: it fixes the two Pester (ubuntu) failures that block the gate on the other PRs. After it merges, re-run CI and Board-ReviewGate.ps1 on each of the others.
• Each PR's external review (claude-review) is recorded on its head commit. A rebase or a new push invalidates it, so it must be reviewed again after rebasing.

[claude]

Review (solo lectura del diff; no ejecuté los tests):

Sin bugs bloqueantes. La lógica es coherente: lectura de config de solo lectura, sin ejecución de comandos externos ni input sin validar, y los fallos de descubrimiento degradan solo este check (mcpCheck = discovery-failed). Los tests cubren bien los casos límite (case, plugins, scopes, aprobación de .mcp.json).

Observaciones menores, no bloqueantes:

• ConvertFrom-Json -AsHashtable exige PowerShell 7+; en Windows PowerShell 5.1 el script falla. Si 5.1 está soportado, el audit lo reportará como discovery-failed. Conviene documentar o comprobar $PSVersionTable de forma explícita.
• Invoke-SkillAudit.ps1 suma ~130 líneas de lógica inline en el script principal. Extraer el check a una función o script propio, como Get-McpServerNames.ps1, facilitaría mantenerlo.