Skip to content

fix(plugin): isolate per-skill reattach failures + surface degraded plugins (#496) - #509

Draft
TYRMars wants to merge 1 commit into
mainfrom
claude/vibrant-dijkstra-sjnf08
Draft

fix(plugin): isolate per-skill reattach failures + surface degraded plugins (#496)#509
TYRMars wants to merge 1 commit into
mainfrom
claude/vibrant-dijkstra-sjnf08

Conversation

@TYRMars

@TYRMars TYRMars commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes #496.

PluginManager.#reattachOne iterated a plugin's skills with no per-skill error guard, and the MCP-server loop ran after it. On server restart, one unreadable or unparseable SKILL.md threw and aborted the rest of that plugin's reattach — every sibling skill and every MCP server the plugin declares. reattachInstalled then swallowed the rejection with a bare .catch(() => {}), and the package keeps no logger, so the failure was completely silent. Worse, GET /v1/plugins kept reporting the plugin as fully installed, because that response is backed by the ledger, not live state.

Changes

  • Per-skill isolation (packages/plugin/src/manager.ts): each skill's reattach is wrapped in its own try/catch. A bad SKILL.md is recorded and skipped so it can no longer take out sibling skills or the MCP servers below it — matching SkillCatalog.scanRoot and the MCP loop's own best-effort intent.
  • MCP failures recorded, not swallowed: the MCP add loop's failures are collected instead of vanishing into .catch(() => {}).
  • Degradation is surfaced: added optional degraded / last_error fields to the InstalledPlugin ledger entry. They're recomputed on every reattach (cleared on a clean pass), and reattachInstalled also records a manifest read/parse failure onto the entry. Both fields flow through list() / get() to GET /v1/plugins (and the apps/jarvis-web client type), so a partially-reattached plugin stops reporting as fully healthy.

Testing

pnpm --filter @jarvis/plugin test (17/17) and typecheck pass; the changed files lint clean. New tests cover:

  • a broken skill is skipped while its sibling skill and the plugin's MCP server both still reattach, with the entry marked degraded + last_error naming the bad skill;
  • a failed MCP-server connect is surfaced as degraded without aborting the skill reattach;
  • a stale degraded flag is cleared once the plugin becomes healthy again.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EHctPZgnTPnPvzQUHCWNzC


Generated by Claude Code

…lugins (#496)

`#reattachOne` iterated a plugin's skills with no per-skill error guard, and
the MCP-server loop ran after it. On restart, one unreadable/unparseable
SKILL.md threw, aborting the rest of that plugin's reattach — every sibling
skill AND every MCP server the plugin declares. `reattachInstalled` swallowed
the rejection with a bare `.catch(() => {})`, so the failure was completely
silent while `GET /v1/plugins` kept reporting the plugin as fully healthy
(the ledger, not live state, backs that response).

- Wrap each skill's reattach in its own try/catch: a bad skill is recorded and
  skipped, so it can't take out sibling skills or the MCP servers below it
  (matches `SkillCatalog.scanRoot` and the MCP loop's own best-effort intent).
- Record MCP add failures instead of silently swallowing them.
- Add `degraded` / `last_error` to the ledger entry, recomputed on every
  reattach (cleared on a clean pass). `reattachInstalled` now records a
  manifest read/parse failure onto the entry too, so a partially-reattached
  plugin stops reporting as fully installed. Both fields flow through
  `list()`/`get()` to `GET /v1/plugins` and the web client type.
- Tests: broken-skill isolation, failed-MCP degradation, and stale-flag clearing.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EHctPZgnTPnPvzQUHCWNzC
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants