[CSalcedoDataBI]

Closes #757
Refs #798

What

#757 — /cleanup plugins mcp (Find-DuplicateMcpServers.ps1)

• Reads every installed build (.mcp.json, mcpServers in plugin.json) and lists the servers whose short name repeats across plugins, with each copy's transport (hosted http/sse vs local stdio).
• Recommends which to keep: the hosted one by default, -PreferLocal when the work needs desktop/project files, -Keep <plugin> to decide explicitly. No clear choice → listed and left alone.
• Plan (default) writes nothing and prints a plan id listing every known project lacking plugin:<p>:<s> in disabledMcpServers. -Execute -PlanId <id> applies exactly that plan: refuses a stale id, backs ~/.claude.json up to ~/.agentic-board/backups/ first, writes atomically via System.Text.Json nodes (only disabledMcpServers touched; every other value round-trips), and reads back to verify. Re-running lists projects opened later.

#798 — /cleanup plugins as the single update path (slice)

• Update report: per-plugin cache pressure (builds + MB over -CacheBuildLimit 10 / -CacheMbLimit 500, suggests clean); plugins running from a directory marketplace with the marketplace remove / add <git>#<ref> / install commands; git marketplaces with autoUpdate off (report only, never flipped).
• Release-notice contract: a plugin's deploy script may write <claude home>/plugin-releases/<plugin>.json (version, commit, publishedAt, fixes[]); the existing stale-plugin notice appends Fixes: #a #b. when the file describes the installed build. Fix text is sanitized (short tokens, max 10). Documented in verbs-plugins.md.

Left for #798 (hence Refs, not Closes)

• Offering to turn autoUpdate on (writing known_marketplaces.json) — only reported today.
• Flagging directory-source plugins in /cleanup plugins sessions (only the update report does).
• Investigating whether /mcp reconnect picks up a new build.

Tests

• DuplicateMcp.Tests.ps1 (11) and PluginUpdate.Report.Tests.ps1 (11): fabricated Claude home + fixture config in $TestDrive; never the real ~/.claude.json, plugin cache or claude CLI.
• Locally: the touched suites (PluginUpdate.*, CommandSurface, PluginStructure, the two new ones) — 376 passed, 0 failed. Update-Docs.ps1 -Check clean. Full suite runs in CI.
• The disable step was not run against the real config.

🤖 Generated with Claude Code

[claude]

Review (sha d67090c) — En general bien diseñado: plan/-Execute -PlanId, backup previo, escritura atómica y verificación por relectura. Tiene tests nuevos. Hallazgos:

1. Permisos de ~/.claude.json (Find-DuplicateMcpServers.ps1, Invoke-McpDisable): el config contiene tokens/OAuth. File.Copy al backup y WriteAllText al .tmp + Move crean archivos con el umask por defecto (típicamente 0644 en Linux/macOS), así que tras -Execute el config puede quedar legible por otros usuarios y el backup también. Copia el modo del original (UnixFileMode/chmod 600) en el tmp y en el backup, y crea $BackupDir con 700.
2. Carrera con la app en ejecución: entre Read-ClaudeConfig y el Move Claude Code puede reescribir .claude.json y se perderían sus cambios (la verificación posterior solo comprueba que las entradas estén). Re-lee y compara hash/mtime justo antes del Move, y aborta si cambió.
3. Backups sin límite: cada -Execute deja un claude.json.*.bak (con secretos) que nunca se rota. Considera conservar solo los N últimos.
4. Tests: confirma cobertura de (a) -PlanId obsoleto → rechazo sin escribir, (b) disabledMcpServers que no es array, (c) preservación de claves no tocadas, (d) Get-ReleaseFixes con sha corto (<7) y fixes fuera de la regex.

Nada bloqueante salvo el punto 1 (seguridad), que recomiendo arreglar antes de mergear.

[claude]

Review de 6147b25: el diseño es sólido (plan/ejecución con PlanId, backup previo, escritura atómica, verificación posterior, validación estricta de plugin-releases/*.json y de la ruta de mcpServers). Sin bloqueantes. Dos observaciones:

1. Windows PowerShell 5.1 (menor, pero choca con el objetivo del último commit). El commit 6147b25 hace que el script parsee en 5.1, pero en runtime Find-DuplicateMcpServers.ps1 depende de APIs solo de .NET Core/5+: System.Text.Json.Nodes, SHA256.HashData y File.Move(src, dst, overwrite). En 5.1 Read-ClaudeConfig devolverá unreadable o el trap abortará con un error poco claro. Conviene un guard explícito al inicio (if ($PSVersionTable.PSEdition -ne 'Core') { ...mensaje claro; exit 1 }) o documentar que requiere pwsh 7+.
2. Tests. Hay cobertura buena en DuplicateMcp.Tests.ps1 y PluginUpdate.Report.Tests.ps1; faltan casos para Get-ReleaseFixes (JSON inválido, archivo >64KB, versión/commit que no coincide, tokens con caracteres no permitidos) y para Get-PluginCacheUsage, si no están ya cubiertos en otro archivo. Además, el PlanId solo fingerprinta Missing: si cambia el -Keep entre plan y ejecución con el mismo conjunto de faltantes, el id coincide; es inocuo, pero un test que lo fije documentaría el comportamiento.

[claude]

Review (sha fc3d010): el diseño está sólido (plan sin escritura, plan id que fija lo escrito, backup previo, escritura atómica, verificación posterior, rechazo de enlaces, issue refs filtradas en el notice). Sin bloqueantes. Observaciones menores:

1. Find-DuplicateMcpServers.ps1 (Read-ClaudeConfig / Invoke-McpDisable): usa System.Text.Json.Nodes y File.SetUnixFileMode/Move(...,$true), que solo existen en pwsh 7+/.NET 8. En Windows PowerShell 5.1 el script parsea (por el fix anterior) pero falla en runtime con "unreadable". Mejor detectar $PSVersionTable.PSVersion.Major -lt 7 al inicio y dar un mensaje claro.
2. if (-not $IsWindows): en 5.1 $IsWindows no existe y es $null, así que la rama Unix se ejecutaría en Windows. Está cubierto por el punto 1, pero conviene un guard explícito.
3. Hay una ventana TOCTOU entre leer el config y mover el tmp (la app puede reescribir ~/.claude.json); la verificación posterior la detecta pero el backup ya existe y el cambio de la app se pierde. Está documentado como límite; considera comparar mtime/hash justo antes del Move y abortar si cambió.
4. Los backups de ~/.claude.json (puede contener API keys) se crean en backups/ sin restringir permisos del directorio; en Unix aplica SetUnixFileMode 0700 al directorio o al menos al .bak.
5. El Id (plugin:<plugin>:<server>) no incluye marketplace: dos plugins con el mismo nombre en marketplaces distintos colapsan en el mismo Id y no se detectan como duplicado (aceptable, pero vale una nota).
6. Tests: buena cobertura de plan/apply/plan-id. Faltan casos para config symlink, -Keep ambiguo vía -Execute, y Get-ReleaseFixes con commit corto (<7) o fixes con inyección de texto (verifica que se descarte).

[claude]

Review (sha 442333b): no encontré bugs bloqueantes ni problemas de seguridad. El diseño es sólido: plan/ejecución atados por PlanId, backup previo, escritura atómica, verificación posterior, rechazo de symlinks y -ClaudeHome sin -ConfigPath/-BackupDir. Get-ReleaseFixes valida bien la entrada: lectura acotada a 64 KB, sin links y solo referencias #N. Tampoco hay secrets expuestos.

Observaciones menores, no bloqueantes:

1. Find-DuplicateMcpServers.ps1 (Invoke-McpDisable): el commit 6147b25 lo deja parseable en PS 5.1, pero en ejecución exige PowerShell 7.3+ o .NET 7+. Usa System.Text.Json.Nodes, FileStreamOptions.UnixCreateMode y File.Move(..., true). En 5.1 -Execute fallaría con un error de tipo poco claro. Conviene un guard al inicio, por ejemplo if ($PSVersionTable.PSVersion -lt [version]'7.3') { ...; exit 1 }, con un mensaje explícito.
2. La comprobación "el config cambió" (mtime/tamaño) se hace justo antes del Move, pero sigue habiendo una ventana mínima de TOCTOU. Está reconocida en los comentarios y el riesgo es bajo.
3. Tests: DuplicateMcp.Tests.ps1 y PluginUpdate.Report.Tests.ps1 cubren bien la lógica pura y el reporte. No vi un test del hook Get-ReleaseFixes con archivos hostiles (>64 KB, link, texto libre en fixes). Sería barato añadirlo, porque es la parte que lee datos de terceros.

[claude]

Review: sin hallazgos bloqueantes.

Revisé Find-DuplicateMcpServers.ps1, los cambios en Update-AllPlugins.ps1, PluginStale-NoticeHook.ps1 y PluginState.ps1. No ejecuté los tests.

• Escritura de ~/.claude.json: el flujo plan, -PlanId, backup, escritura atómica, chequeo de stamp y lectura de verificación es sólido. El temp file se crea con el modo del original y rechaza enlaces.
• Notas de release: Get-ReleaseFixes valida nombres, tamaño y tipo de archivo, y solo deja pasar referencias #N, así que el texto externo no llega al modelo.

Nit opcional, sin bloquear: el script depende de System.Text.Json.Nodes y SHA256.HashData (.NET 8 / pwsh 7). En Windows PowerShell 5.1 solo parsea (commit 6147b25), y al ejecutarlo Read-ClaudeConfig devuelve unreadable sin explicar el motivo. Un guard con mensaje claro como "requiere pwsh 7+" ayudaría.

🤖 Generated with Claude Code

[claude]

Revisé Find-DuplicateMcpServers.ps1 (la parte nueva más sensible); no revisé a fondo los cambios en Update-AllPlugins.ps1, PluginStale-NoticeHook.ps1 ni los tests. En ese script no encontré bugs bloqueantes. Hace bien lo difícil: plan y -Execute separados con PlanId, backup antes de escribir, escritura atómica con el modo del archivo original, comprobación de cambios concurrentes, rechazo de enlaces simbólicos y de -Keep con typos, y Test-PathStrictlyInside contra path traversal.

Observaciones menores, no bloqueantes:

• El PlanId solo hace hash de Missing. Dos planes con distinto -Keep pero las mismas entradas por escribir dan el mismo id. Esto es inocuo, porque se escribe exactamente lo que se mostró.
• El script usa $IsWindows y System.Text.Json, así que exige PowerShell 7+. Conviene documentarlo o añadir #requires -Version 7.
• Hay una ventana TOCTOU pequeña entre la comprobación de LastWriteTime/Length y File.Move. Está reconocida en el código y es aceptable.

[claude]

Review (sha d6037a0): no encontré bugs bloqueantes. Revisé a fondo Find-DuplicateMcpServers.ps1 y el diff de Update-AllPlugins.ps1, PluginState.ps1 y PluginStale-NoticeHook.ps1. No leí los tests ni los .md en detalle.

Lo que está bien:

• El flujo plan → -Execute -PlanId está bien atado: el id es un hash de lo que se va a escribir.
• Hace backup antes de escribir y escribe de forma atómica. Recomprueba LastWriteTime/Length justo antes del Move, y el archivo temporal nace con el modo del original, así que no queda legible por otros usuarios.
• Rechaza symlinks.
• Get-ReleaseFixes valida bien la entrada que no es de confianza: regex de nombres, límite de 64 KB, solo referencias #N, máximo 10.

Menores, no bloqueantes:

1. Update-AllPlugins.ps1 (reporte de DirectorySource): $d.Key, $m y $d.Scope se imprimen dentro de comandos claude plugin ... sugeridos sin sanitizar. ProjectPath sí pasa por ConvertTo-PlainText. Los nombres vienen de known_marketplaces.json e installed_plugins.json, así que conviene aplicar el mismo ConvertTo-PlainText o validar con ^[A-Za-z0-9._@-]+$. Si alguien copia y pega esa línea, el riesgo es de inyección.
2. Find-DuplicateMcpServers.ps1:296: $IsWindows y FileStreamOptions.UnixCreateMode exigen PowerShell 7 / .NET 7+. Si el plugin puede correr en 5.1, falla. Conviene declararlo con #requires -Version 7 o comprobarlo al inicio.
3. Test-KeepMatches: -Keep <plugin> aplica a todos los servers de ese plugin. Si un plugin comparte dos nombres de servidor con otro, la elección vale para ambos. Es razonable, pero vale una línea en la doc.

Los tests nuevos (DuplicateMcp, PluginUpdate.Report) cubren los caminos principales. Si se quiere un caso más, sería la carrera de -BeforeReplace con el config reescrito, que ya tiene su gancho de prueba.

[claude]

Sin hallazgos bloqueantes en el sha b391141. Revisé Find-DuplicateMcpServers.ps1, Update-AllPlugins.ps1, PluginState.ps1 y PluginStale-NoticeHook.ps1. No ejecuté los tests.

Lo que comprobé al leer el código:

• El flujo plan y luego -Execute -PlanId coincide con lo que el usuario vio. La escritura es atómica y se hace con backup. Se comprueba la fecha y el tamaño del fichero justo antes del reemplazo. Los enlaces se rechazan. El temporal se crea con el modo del original.
• Get-ReleaseFixes acota la lectura a 64 KB, rechaza enlaces y dispositivos, y solo acepta referencias a issues con regex estricta. No cuela texto libre al modelo.

Sugerencias menores, no bloquean:

• Find-DuplicateMcpServers.ps1 usa System.Text.Json.Nodes, que solo existe en PowerShell 7 o superior. En Windows PowerShell 5.1 el script parsea (commit 6147b25), pero Read-ClaudeConfig cae en unreadable. Conviene decirlo en el mensaje o en la doc, en vez de dar a entender que la config está corrupta.
• El backup de ~/.claude.json puede contener API keys. File.Copy conserva el modo en Unix, pero el directorio de backups se crea con el umask por defecto. Considera un modo 0700 para ese directorio.

[claude]

Review (último commit b23464d + vistazo al diff de release-notice)

Sin bloqueantes. El fix del BOM es correcto (TrimStart([char]0xFEFF) sobre el string decodificado) y tiene test dedicado.

Observaciones menores, no bloqueantes:

• Get-ReleaseFixes usa $fi.LinkTarget (solo .NET 6+/PS 7). En Windows PowerShell 5.1 la propiedad no existe; si hay strict mode lanzaría y el catch devolvería @() silenciosamente, perdiendo los fixes. Si 5.1 está soportado, comprobar ReparsePoint basta.
• La validación del contenido (regex estricta de referencias #N, límite 64 KB, rechazo de links/dispositivos, máx. 10) es sólida frente a inyección de texto en el aviso que lee el modelo.
• No revisé en profundidad Find-DuplicateMcpServers.ps1 (427 líneas); los tests de DuplicateMcp.Tests.ps1 parecen cubrirlo razonablemente.

[claude]

Sin hallazgos bloqueantes. Revisé a fondo Find-DuplicateMcpServers.ps1 y por encima el resto del diff (cambios en Update-AllPlugins, PluginState y el hook, y los tests nuevos). No ejecuté los tests.

Lo que está bien en Find-DuplicateMcpServers.ps1:

• El plan es de solo lectura, y -Execute exige un -PlanId que coincida con el plan recalculado.
• Hace backup antes de escribir y escribe de forma atómica con modo restrictivo en Unix.
• Antes del reemplazo comprueba que la config no cambió, y después relee para verificar.
• Rechaza symlinks.
• Rechaza -Keep que no coincide con ningún plugin.
• Test-PathStrictlyInside protege la ruta de mcpServers en plugin.json.

Nits opcionales:

• Hay una ventana TOCTOU pequeña entre la comprobación de LastWriteTime/Length y el Move. Es inherente a la técnica y está documentada como límite, así que no pide cambio.
• El backup se crea antes de la comprobación final de cambios. Si esa comprobación falla, queda un .bak huérfano. Es inocuo, pero puedes borrarlo en esa rama.

[CSalcedoDataBI]

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

Reviewed commit: 79ba2b0f5ff09b8e58e3bd4b3e83822168fdbd0d

claude-review (Claude API, claude-opus-5-5, --effort xhigh, clean context; same model family, not a second provider) reviewed the full diff in 9 rounds, split by folder after max_tokens. ~40 findings verified against the source and fixed: scope/enabledPlugins filtering (user + project, local overrides shared), skip projects that disable the kept copy, unmatched -Keep refused, plan id never leaked, link / mid-edit / Unix-mode / relative-path / unmapped-drive safety on ~/.claude.json, -ClaudeHome guards; scoped reinstall lines with project folders, freeable-only clean advice, autoUpdate only explicit false; release file keyed plugin@marketplace, bounded regular-file read, BOM, issue-reference-only fixes; tests for each. Final rounds: scripts (Find-DuplicateMcpServers, hook/state/update) and tests+docs returned no findings.

[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 (sha 3aa9e16): sin bloqueantes. Plan/-PlanId/-Execute con backup, escritura atómica, detección de cambio concurrente, rechazo de enlaces y modo 0600 del temporal están bien resueltos; Get-ReleaseFixes valida la entrada (tamaño, regex de refs). Los tests cubren las piezas nuevas.

Observaciones menores (no bloquean):

• Find-DuplicateMcpServers.ps1 (Invoke-McpDisable): si el config cambia durante la edición se aborta, pero el backup ya creado queda huérfano en backups/. Crear el backup justo antes del Move, o borrarlo en ese camino.
• Test-PluginOffInProject solo mira enabledPlugins en .claude/settings*.json; no considera plugins desactivados por scope managed. Caso borde, vale una nota en la doc.
• $IsWindows en el bloque de UnixCreateMode asume PowerShell 7+; en 5.1 queda indefinido y se intentaría GetUnixFileMode. Confirmar que el script exige pwsh 7 (o usar [OperatingSystem]::IsWindows()).