refactor(plugins-mcp): dedupe collectors, guards, and register loops

src/mcp/tool-name.ts

@@ -113,19 +113,14 @@ const MCP_READ_ONLY_TOOL_PREFIXES = [
   "fetch_",
 ] as const;
 
-function mcpToolSegmentMatchesPrefix(
-  segment: string,
-  prefixes: readonly string[],
-): boolean {
-  return prefixes.some((prefix) => segment.startsWith(prefix));
-}
-
 // Name-prefix fallback when a server omits ToolAnnotations on tools/list.
 export function isReadOnlyMcpTool(name: string): boolean {
   const parsed = parseMcpToolName(name);
   if (parsed === null) return false;
   const segment = parsed.tool;
-  if (mcpToolSegmentMatchesPrefix(segment, MCP_MUTATING_TOOL_PREFIXES))
+  if (MCP_MUTATING_TOOL_PREFIXES.some((prefix) => segment.startsWith(prefix)))
     return false;
-  return mcpToolSegmentMatchesPrefix(segment, MCP_READ_ONLY_TOOL_PREFIXES);
+  return MCP_READ_ONLY_TOOL_PREFIXES.some((prefix) =>
+    segment.startsWith(prefix),
+  );
 }

src/plugins/agent-plugins.ts

@@ -7,7 +7,7 @@ import type { PluginModule } from "./loader.js";
 import type { PluginConfig } from "../config/settings.js";
 import { isPluginModuleEnabled } from "./register.js";
 import {
-  pluginWarningSink,
+  resolvePluginWarningHandler,
   type PluginLoadDiagnostics,
 } from "./diagnostics.js";
 import { type } from "arktype";
@@ -27,8 +27,9 @@ function resolveAgentProfileWarningHandler(
 ): (msg: string) => void {
   if (typeof opts === "function") return opts;
   if (opts.diagnostics !== undefined)
-    return pluginWarningSink(opts.diagnostics);
-  if (opts.onWarning !== undefined) return opts.onWarning;
+    return resolvePluginWarningHandler({ diagnostics: opts.diagnostics });
+  if (opts.onWarning !== undefined)
+    return resolvePluginWarningHandler({ onWarning: opts.onWarning });
   return () => undefined;
 }
 

src/plugins/data-only-commands.ts

@@ -7,7 +7,7 @@ import {
   type CommandResult,
   type SubcommandDefinition,
 } from "../tui/commands/registry.js";
-import { splitFrontmatter } from "./frontmatter.js";
+import { COMMAND_NAME_PATTERN, splitFrontmatter } from "./frontmatter.js";
 
 // A data-only command plugin declares its slash commands as markdown files, the
 // same convention Claude Code (`.claude/commands/`), OpenCode
@@ -25,8 +25,6 @@ import { splitFrontmatter } from "./frontmatter.js";
 // replaced with the args passed to that command level (all args for a flat
 // command; the args after the subcommand for a namespaced one).
 
-const COMMAND_NAME_PATTERN = /^[a-z0-9]+(-[a-z0-9]+)*$/;
-
 interface LoadedBody {
   description: string;
   body: string;

src/plugins/data-only.ts

@@ -26,7 +26,7 @@ export interface DataOnlyPlugin {
   commandPlugin?: CommandPlugin;
 }
 
-function isENOENT(err: unknown): boolean {
+export function isENOENT(err: unknown): boolean {
   return (
     typeof err === "object" &&
     err !== null &&

src/plugins/frontmatter.ts

@@ -9,6 +9,11 @@ export interface ParsedMarkdown {
   body: string;
 }
 
+// Slash-command names are kebab-case identifiers. Data-only command files and
+// skill commands validate the same shape, so the pattern lives here once —
+// both modules already import this file for splitFrontmatter.
+export const COMMAND_NAME_PATTERN = /^[a-z0-9]+(-[a-z0-9]+)*$/;
+
 // Strip a leading `---\n...\n---` YAML block. Returns { frontmatter, body }.
 // No frontmatter block -> empty-object frontmatter (an agent/command can
 // legitimately have none). A present-but-malformed block -> null frontmatter

src/plugins/loader.ts

@@ -12,12 +12,14 @@ import { pathIsInsideOrEqual } from "../util/path-contain.js";
 import {
   parsePluginManifest,
   PluginManifestSchema,
+  type PluginCredentialField,
+  type PluginKind,
   type PluginManifest,
 } from "./manifest.js";
 import { NOOP_TELEMETRY, type Telemetry } from "../telemetry/index.js";
 import type { PluginLoadReporter } from "../telemetry/product-events.js";
 import { runtimePluginLoadReporter } from "../telemetry/singleton.js";
-import { loadDataOnlyPlugin } from "./data-only.js";
+import { isENOENT, loadDataOnlyPlugin } from "./data-only.js";
 
 import {
   resolvePluginWarningHandler,
@@ -76,19 +78,46 @@ export interface PluginModule {
   shadowedRepoDefaultEnabled?: boolean;
 }
 
-function isENOENT(err: unknown): boolean {
-  return (
-    typeof err === "object" &&
-    err !== null &&
-    "code" in err &&
-    (err as { code?: unknown }).code === "ENOENT"
-  );
-}
-
 function errorText(err: unknown): string {
   return err instanceof Error ? err.message : String(err);
 }
 
+// Flat candidate behind the tool-plugin and web-provider collectors: both
+// select modules by manifest kind + factory key and project the same shape,
+// so the per-kind entry points stay one-line typed wrappers.
+export interface CollectedPluginCandidate<TFactory> {
+  id: string;
+  name: string;
+  description?: string;
+  credentials: PluginCredentialField[];
+  factory: (options: unknown) => TFactory | Promise<TFactory>;
+}
+
+export function collectPluginCandidates<TFactory>(
+  modules: PluginModule[],
+  opts: {
+    kind: PluginKind;
+    factoryKey: "createToolPlugin" | "createWebProvider";
+  },
+): CollectedPluginCandidate<TFactory>[] {
+  const out: CollectedPluginCandidate<TFactory>[] = [];
+  for (const mod of modules) {
+    if (mod.manifest?.kind !== opts.kind) continue;
+    const factory = mod[opts.factoryKey];
+    if (typeof factory !== "function") continue;
+    out.push({
+      id: mod.manifest.id,
+      name: mod.manifest.name,
+      ...(mod.manifest.description !== undefined
+        ? { description: mod.manifest.description }
+        : {}),
+      credentials: mod.manifest.credentials ?? [],
+      factory: factory as CollectedPluginCandidate<TFactory>["factory"],
+    });
+  }
+  return out;
+}
+
 // Read and validate a manifest.json beside the module. Plugins may declare
 // their manifest as a JS export (mod.manifest) or a sibling manifest.json file;
 // this covers the JSON path so plugins that are pure data + commands work too.
@@ -427,23 +456,16 @@ export function expandSkipDiagnosticsHandler(
   return (skip) => diagnostics.warnings.push(formatExpandSkip(skip));
 }
 
-/**
- * `onSkip` when no diagnostics collector is in play: one explicit stderr
- * line, module-private and only reached by an internal caller's own
- * deliberate choice (see `resolveExpandSkip` below) — never `expandPluginPath`
- * falling back to it on its own.
- */
-function stderrExpandSkip(skip: ExpandPluginPathSkip): void {
-  process.stderr.write(`plugins: ${formatExpandSkip(skip)}\n`);
-}
-
 /** Diagnostics when given, else the explicit stderr line — no silent option. */
 function resolveExpandSkip(
   diagnostics?: PluginLoadDiagnostics,
 ): (skip: ExpandPluginPathSkip) => void {
-  return diagnostics !== undefined
-    ? expandSkipDiagnosticsHandler(diagnostics)
-    : stderrExpandSkip;
+  const onWarning = resolvePluginWarningHandler(
+    diagnostics !== undefined
+      ? { diagnostics }
+      : { onWarning: stderrPluginWarning },
+  );
+  return (skip) => onWarning(formatExpandSkip(skip));
 }
 
 /**
@@ -928,11 +950,11 @@ export async function discoverClaudeInstalledPlugins(
     parsed = JSON.parse(raw);
   } catch {
     // Prefer collector when present; else stderrPluginWarning.
-    resolvePluginWarningHandler(
-      opts.diagnostics !== undefined
-        ? { diagnostics: opts.diagnostics }
-        : { onWarning: stderrPluginWarning },
-    )(`failed to parse ${registryPath}`);
+    (opts.diagnostics !== undefined
+      ? resolvePluginWarningHandler({ diagnostics: opts.diagnostics })
+      : resolvePluginWarningHandler({ onWarning: stderrPluginWarning }))(
+      `failed to parse ${registryPath}`,
+    );
     return [];
   }
   if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) {

src/plugins/register.ts

@@ -89,28 +89,40 @@ export function registerCommandPlugins(
   config: Record<string, PluginConfig> | (() => Record<string, PluginConfig>),
 ): string[] {
   const getConfig = typeof config === "function" ? config : () => config;
-  const registered: string[] = [];
-  for (const mod of modules) {
+  return collectEnabledModules(modules, (mod) => {
     const id = mod.manifest?.id;
     if (id === undefined || !registerCommandPluginModule(mod, getConfig))
-      continue;
-    if (isPluginModuleEnabled(mod, getConfig())) registered.push(id);
-  }
-  return registered;
+      return undefined;
+    return isPluginModuleEnabled(mod, getConfig()) ? id : undefined;
+  });
 }
 
 export function registerWorkflowPlugins(
   modules: PluginModule[],
   config: Record<string, PluginConfig>,
 ): string[] {
-  const registered: string[] = [];
-  for (const mod of modules) {
-    if (!isEnabledWorkflowPlugin(mod, config)) continue;
+  return collectEnabledModules(modules, (mod) => {
+    if (!isEnabledWorkflowPlugin(mod, config)) return undefined;
     const workflowPlugin = mod.workflowPlugin;
     const id = mod.manifest?.id;
-    if (workflowPlugin === undefined || id === undefined) continue;
+    if (workflowPlugin === undefined || id === undefined) return undefined;
     registerWorkflowPlugin(workflowPlugin);
-    registered.push(id);
+    return id;
+  });
+}
+
+// Shared register loop: each module is offered to `collect`, which performs
+// that kind's registration side effects and returns the manifest id exactly
+// when the module counts as registered, or undefined to skip it. One pass in
+// module order; per-kind predicates stay at the call sites above.
+function collectEnabledModules(
+  modules: PluginModule[],
+  collect: (mod: PluginModule) => string | undefined,
+): string[] {
+  const registered: string[] = [];
+  for (const mod of modules) {
+    const id = collect(mod);
+    if (id !== undefined) registered.push(id);
   }
   return registered;
 }

src/plugins/skill-commands.ts

@@ -5,7 +5,7 @@ import {
   type CommandDefinition,
   type CommandResult,
 } from "../tui/commands/registry.js";
-import { splitFrontmatter } from "./frontmatter.js";
+import { COMMAND_NAME_PATTERN, splitFrontmatter } from "./frontmatter.js";
 
 // Slash is the operator action surface: `/<skill-name> [args]` sends the skill
 // body (plus args) to the agent. Convention/internal skills opt out with
@@ -15,8 +15,6 @@ import { splitFrontmatter } from "./frontmatter.js";
 // skips the skill from `discoverSkills` lazy listing. Explicit `use_skill` /
 // `resolveSkillBody` still loads the body by name.
 
-const COMMAND_NAME_PATTERN = /^[a-z0-9]+(-[a-z0-9]+)*$/;
-
 // `$ARGUMENTS` (Claude Code convention) interpolates inline when the author used
 // it; otherwise args append after the body so the skill instructions run against
 // the user's target (e.g. an issue id).

src/plugins/tool-plugins.ts

@@ -1,5 +1,5 @@
 import type { ToolPlugin } from "@intx/tools-posix";
-import type { PluginModule } from "./loader.js";
+import { collectPluginCandidates, type PluginModule } from "./loader.js";
 import type { PluginConfig } from "../config/settings.js";
 import type { PluginCredentialField } from "./manifest.js";
 import { scrubSecrets } from "../web/secret-scrub.js";
@@ -22,21 +22,10 @@ export interface ToolPluginCandidate {
 export function collectToolPlugins(
   modules: PluginModule[],
 ): ToolPluginCandidate[] {
-  const out: ToolPluginCandidate[] = [];
-  for (const mod of modules) {
-    if (mod.manifest?.kind !== "tool") continue;
-    if (typeof mod.createToolPlugin !== "function") continue;
-    out.push({
-      id: mod.manifest.id,
-      name: mod.manifest.name,
-      ...(mod.manifest.description !== undefined
-        ? { description: mod.manifest.description }
-        : {}),
-      credentials: mod.manifest.credentials ?? [],
-      factory: mod.createToolPlugin as ToolPluginCandidate["factory"],
-    });
-  }
-  return out;
+  return collectPluginCandidates<ToolPlugin>(modules, {
+    kind: "tool",
+    factoryKey: "createToolPlugin",
+  });
 }
 
 // A tool plugin adds in-process agent capabilities, so it is wired in only when

src/web/plugin-provider.ts

@@ -1,6 +1,7 @@
 import type { WebProvider } from "./types.js";
 import { scrubSecrets } from "./secret-scrub.js";
 import type { PluginModule } from "../plugins/loader.js";
+import { collectPluginCandidates } from "../plugins/loader.js";
 import type { PluginConfig } from "../config/settings.js";
 import type { PluginCredentialField } from "../plugins/manifest.js";
 
@@ -18,21 +19,10 @@ export interface WebPluginCandidate {
 export function collectWebPlugins(
   modules: PluginModule[],
 ): WebPluginCandidate[] {
-  const out: WebPluginCandidate[] = [];
-  for (const mod of modules) {
-    if (mod.manifest?.kind !== "web") continue;
-    if (typeof mod.createWebProvider !== "function") continue;
-    out.push({
-      id: mod.manifest.id,
-      name: mod.manifest.name,
-      ...(mod.manifest.description !== undefined
-        ? { description: mod.manifest.description }
-        : {}),
-      credentials: mod.manifest.credentials ?? [],
-      factory: mod.createWebProvider as WebPluginCandidate["factory"],
-    });
-  }
-  return out;
+  return collectPluginCandidates<WebProvider>(modules, {
+    kind: "web",
+    factoryKey: "createWebProvider",
+  });
 }
 
 // Pick the active web plugin: an explicit `web` override wins; otherwise the