Collect discovery-time marketplace skips, surface profile warnings

Two more gaps in the plugin-diagnostics fix:

scanPluginsDir and loadPluginsFromPaths both call expandPluginPath
with no onSkip, so a marketplace member skipped during discovery (a
bad source under .corbits/plugins/ or a registered pluginPaths entry)
fell through to the raw-stderr default. Unlike the earlier sites, this
one bypassed the diagnostics collector entirely rather than just
misrouting to the log, so the warning was lost outright. Added
expandSkipDiagnosticsHandler in loader.ts and wired it into both call
sites, plus addPath in runner.ts (replacing its duplicate inline
version).

profileDiag, the startup agent-profile resolution, was still logged
only and never reached startupPluginNotices — one of the sites the
previous commit's message claimed to have fixed but did not. Gave it
the same treatment as its discovery and tool-plugin siblings.

Extended the behavioral test with the marketplace-discovery-skip case
(asserting the warning lands in diagnostics, not just that stderr
stays quiet) and the startup agent-profile path.

Author: TheGreatAxios

src/plugins/loader.ts

@@ -267,6 +267,27 @@ function defaultExpandSkip(skip: ExpandPluginPathSkip): void {
   );
 }
 
+function formatExpandSkip(skip: ExpandPluginPathSkip): string {
+  const where = skip.resolved !== undefined ? ` → ${skip.resolved}` : "";
+  return `marketplace source ${JSON.stringify(skip.source)} skipped (${skip.reason})${where}`;
+}
+
+/**
+ * Build an `onSkip` handler that collects into `diagnostics` instead of
+ * `expandPluginPath`'s raw-stderr default. Every discovery path that already
+ * threads a diagnostics collector through must pass this, or a skipped
+ * marketplace member bypasses the collector entirely — not merely misrouted,
+ * dropped, since nothing else observes `defaultExpandSkip`'s stderr write.
+ * Returns undefined (falls back to the default) when no collector is given,
+ * e.g. a direct caller with no batching in play.
+ */
+export function expandSkipDiagnosticsHandler(
+  diagnostics?: PluginLoadDiagnostics,
+): ((skip: ExpandPluginPathSkip) => void) | undefined {
+  if (diagnostics === undefined) return undefined;
+  return (skip) => diagnostics.warnings.push(formatExpandSkip(skip));
+}
+
 /**
  * Containment check with symlink safety. Lexical reject first; when both the
  * candidate and the contain root exist, realpath both and re-check so a symlink
@@ -456,8 +477,11 @@ async function scanPluginsDir(
 
   const results: PluginModule[] = [];
   for (const entry of entries) {
-    // Each entry may itself be a marketplace, so expand before loading.
-    const dirs = await expandPluginPath(join(dir, entry));
+    // Each entry may itself be a marketplace, so expand before loading. A
+    // skipped member routes into `diagnostics` when the caller has one, same
+    // as loadPluginEntry below — otherwise it would bypass the collector.
+    const onSkip = expandSkipDiagnosticsHandler(diagnostics);
+    const dirs = await expandPluginPath(join(dir, entry), onSkip !== undefined ? { onSkip } : {});
     for (const d of dirs) {
       const abs = resolve(d);
       if (originRequiresTrust(origin) && isTrusted !== undefined && !isTrusted(abs)) {
@@ -536,10 +560,13 @@ export async function loadPluginsFromPaths(
     diagnostics?: PluginLoadDiagnostics;
   } = {},
 ): Promise<PluginModule[]> {
+  // A skipped member routes into `diagnostics` when the caller has one, same
+  // reasoning as scanPluginsDir — otherwise it bypasses the collector.
+  const onSkip = expandSkipDiagnosticsHandler(opts.diagnostics);
   const resolved = await Promise.all(
     paths.map(async (p) => {
       const abs = isAbsolute(p) ? p : join(cwd, p);
-      return expandPluginPath(abs);
+      return expandPluginPath(abs, onSkip !== undefined ? { onSkip } : {});
     }),
   );
   // Anything under <cwd>/.corbits/plugins/ is project origin no matter how it

src/tui/plugin-diagnostics-sink.test.ts

@@ -7,7 +7,12 @@ import {
   emitPluginWarningLog,
   formatPluginWarningsSummary,
 } from "../plugins/diagnostics.js";
-import { expandPluginPath, loadPluginEntry, type ExpandPluginPathSkip } from "../plugins/loader.js";
+import {
+  discoverUserPlugins,
+  expandPluginPath,
+  loadPluginEntry,
+  type ExpandPluginPathSkip,
+} from "../plugins/loader.js";
 import { resolveAgentPluginProfiles } from "../plugins/agent-plugins.js";
 import { resolveToolPlugins, type ToolPluginCandidate } from "../plugins/tool-plugins.js";
 import { discoverSessionPlugins } from "../session/runtime-assembly.js";
@@ -132,6 +137,58 @@ describe("interactive plugin diagnostics never hit raw stderr", () => {
     expect(writes).toBe(0);
   });
 
+  test("startup discovery: a marketplace member skipped during discovery is collected, not lost", async () => {
+    // Reproduces the exact shape reported against `scanPluginsDir` /
+    // `loadPluginsFromPaths`: a project-local `.corbits/plugins/` entry that
+    // is itself a marketplace with one bad (absolute) `source`. Both call
+    // `expandPluginPath` internally; without `onSkip` wired to `diagnostics`,
+    // the skip bypasses the collector entirely (not merely misrouted to the
+    // log — genuinely dropped, since nothing else observes the raw write).
+    const cwd = await mkdtemp(join(tmpdir(), "diag-cwd-market-"));
+    const marketDir = join(cwd, ".corbits", "plugins", "market");
+    await mkdir(join(marketDir, ".claude-plugin"), { recursive: true });
+    await writeFile(
+      join(marketDir, ".claude-plugin", "marketplace.json"),
+      JSON.stringify({
+        name: "demo",
+        plugins: [{ name: "bad", source: "/etc/not-a-plugin" }],
+      }),
+    );
+    const { writes } = await withStderrCapture(async () => {
+      const diag = createPluginLoadDiagnostics();
+      await discoverUserPlugins(cwd, { diagnostics: diag });
+      // The skip must land in the collector, not just avoid stderr — a write
+      // that silently vanishes without reaching diagnostics is the same
+      // lost-warning bug reached by a different route.
+      const message = formatPluginWarningsSummary(diag.warnings);
+      expect(message).toBeDefined();
+      expect(diag.warnings.some((w) => w.includes("/etc/not-a-plugin"))).toBe(true);
+    });
+    expect(writes).toBe(0);
+  });
+
+  test("startup agent-profile resolution: a malformed profile stays silent on stderr", async () => {
+    // Same call shape as runner.ts's startup `resolveAgentPluginProfiles`
+    // (over `executablePlugins()` and the full `settings.plugins` config),
+    // distinct from the verify-time call above which targets one plugin id.
+    const mod = {
+      manifest: { id: "startup-agent", name: "Startup Agent", kind: "agent" as const },
+      agentPlugin: { agents: [{ description: "missing the required id field" }] },
+    };
+    const { writes } = await withStderrCapture(async () => {
+      const diag = createPluginLoadDiagnostics();
+      const profiles = await resolveAgentPluginProfiles(
+        [mod],
+        { "startup-agent": { enabled: true } },
+        { diagnostics: diag },
+      );
+      expect(profiles).toEqual([]);
+      const message = formatPluginWarningsSummary(diag.warnings);
+      expect(message).toBeDefined();
+    });
+    expect(writes).toBe(0);
+  });
+
   test("tool-resolve: a throwing tool-plugin factory stays silent on stderr", async () => {
     const candidate: ToolPluginCandidate = {
       id: "throws",

src/tui/runner.ts

@@ -50,8 +50,8 @@ import { refreshCodexInstructions } from "../auth/codex/instructions.js";
 import {
   expandExistingPluginMembers,
   expandPluginPath,
+  expandSkipDiagnosticsHandler,
   loadPluginEntry,
-  type ExpandPluginPathSkip,
   type PluginOrigin,
 } from "../plugins/loader.js";
 import {
@@ -899,14 +899,11 @@ export async function runTUI(initialConfig: Config): Promise<number> {
       // member is trusted (exact-path match on reload). `onSkip` collects into
       // `addDiag` instead of the default stderr write — same reasoning as
       // `loadPluginEntry` above.
-      const members = await expandPluginPath(abs, {
-        onSkip: (skip: ExpandPluginPathSkip) => {
-          const where = skip.resolved !== undefined ? ` → ${skip.resolved}` : "";
-          addDiag.warnings.push(
-            `marketplace source ${JSON.stringify(skip.source)} skipped (${skip.reason})${where}`,
-          );
-        },
-      });
+      const addSkipHandler = expandSkipDiagnosticsHandler(addDiag);
+      const members = await expandPluginPath(
+        abs,
+        addSkipHandler !== undefined ? { onSkip: addSkipHandler } : {},
+      );
       pathTrust = await trustPathPlugins(members.length > 0 ? members : [abs]);
       // Replace any existing descriptor/candidate with the same id so re-adding
       // refreshes rather than duplicates.
@@ -985,6 +982,10 @@ export async function runTUI(initialConfig: Config): Promise<number> {
     { diagnostics: profileDiag },
   );
   emitPluginWarningLog(profileDiag);
+  // Same fire-and-forget reasoning as the discovery/tool-plugin notices above:
+  // this runs before `host` exists, so it is queued rather than dropped.
+  const profileNotice = formatPluginWarningsSummary(profileDiag.warnings);
+  if (profileNotice !== undefined) startupPluginNotices.push(profileNotice);
   const initialProfiles = await loadAgentProfiles(profilesDir, pluginAgentProfiles);
   let liveAgentProfiles = initialProfiles;