fix(agents): log warnings instead of swallowing subagent errors (#82943)

SebTardif · steipete · web-flow · commit 907bc0371c3a · 2026-05-25T00:57:29.000+01:00
* fix: log subagent swallowed errors in hook emission and restore paths

Wire createSubsystemLogger into the two silent catch blocks that
discard errors during subagent lifecycle:

1. emitSubagentEndedHookOnce (subagent-registry-completion.ts):
   catch { return false } -&gt; catch (err) { log.warn(...); return false }

2. restoreSubagentRunsOnce (subagent-registry.ts):
   catch { /* ignore */ } -&gt; catch (err) { log.warn(...) }

Both paths now log the error message before continuing, providing
a diagnostic trail when hook emission or disk restore fails silently.

Signed-off-by: Sebastien Tardif &lt;sebtardif@ncf.ca&gt;

* test(agents): keep provider test mocks current

---------

Signed-off-by: Sebastien Tardif &lt;sebtardif@ncf.ca&gt;
Co-authored-by: Peter Steinberger &lt;steipete@gmail.com&gt;
diff --git a/src/agents/model-fallback.probe.test.ts b/src/agents/model-fallback.probe.test.ts
@@ -28,8 +28,42 @@ vi.mock("./provider-model-normalization.runtime.js", () => ({
 }));
 
 const emptyPluginMetadataSnapshot = vi.hoisted(() => ({
+  policyHash: "model-fallback-probe-test-empty-plugin-policy",
   configFingerprint: "model-fallback-probe-test-empty-plugin-metadata",
+  index: {
+    hostContractVersion: "test",
+    compatRegistryVersion: "test",
+    migrationVersion: 1,
+    policyHash: "model-fallback-probe-test-empty-plugin-policy",
+    generatedAtMs: 0,
+    installRecords: {},
+    plugins: [],
+    diagnostics: [],
+  },
+  registryDiagnostics: [],
+  manifestRegistry: { plugins: [], diagnostics: [] },
   plugins: [],
+  diagnostics: [],
+  byPluginId: new Map(),
+  normalizePluginId: (pluginId: string) => pluginId,
+  owners: {
+    channels: new Map(),
+    channelConfigs: new Map(),
+    providers: new Map(),
+    modelCatalogProviders: new Map(),
+    cliBackends: new Map(),
+    setupProviders: new Map(),
+    commandAliases: new Map(),
+    contracts: new Map(),
+  },
+  metrics: {
+    registrySnapshotMs: 0,
+    manifestRegistryMs: 0,
+    ownerMapsMs: 0,
+    totalMs: 0,
+    indexPluginCount: 0,
+    manifestPluginCount: 0,
+  },
 }));
 
 vi.mock("../plugins/current-plugin-metadata-snapshot.js", () => ({
diff --git a/src/agents/pi-embedded-runner-extraparams.test.ts b/src/agents/pi-embedded-runner-extraparams.test.ts
@@ -5,6 +5,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
 import { testing as extraParamsTesting } from "./pi-embedded-runner/extra-params.js";
 
 vi.mock("../plugins/provider-hook-runtime.js", () => ({
+  clearProviderRuntimePluginCacheForTest: vi.fn(),
   testing: {
     buildHookProviderCacheKey: () => "test-provider-hook-cache-key",
   },
diff --git a/src/agents/pi-embedded-runner.sanitize-session-history.test-harness.ts b/src/agents/pi-embedded-runner.sanitize-session-history.test-harness.ts
@@ -88,6 +88,7 @@ export function createSanitizeSessionHistoryProviderHookRuntimeMock(
   extra: Record<string, unknown> = {},
 ) {
   return {
+    clearProviderRuntimePluginCacheForTest: vi.fn(),
     resolveProviderRuntimePlugin: vi.fn(() => undefined),
     resolveProviderHookPlugin: vi.fn(() => undefined),
     resolveProviderPluginsForHooks: vi.fn(() => []),
diff --git a/src/agents/pi-embedded-runner.sanitize-session-history.test.ts b/src/agents/pi-embedded-runner.sanitize-session-history.test.ts
@@ -29,6 +29,7 @@ vi.mock("./pi-embedded-helpers.js", async () => ({
 }));
 
 vi.mock("../plugins/provider-hook-runtime.js", async () => ({
+  clearProviderRuntimePluginCacheForTest: vi.fn(),
   testing: {},
   prepareProviderExtraParams: vi.fn(() => undefined),
   resolveProviderHookPlugin: vi.fn(() => undefined),
diff --git a/src/agents/subagent-registry-completion.ts b/src/agents/subagent-registry-completion.ts
@@ -1,3 +1,4 @@
+import { createSubsystemLogger } from "../logging/subsystem.js";
 import { getGlobalHookRunner } from "../plugins/hook-runner-global.js";
 import type { SubagentRunOutcome } from "./subagent-announce-output.js";
 import {
@@ -10,6 +11,8 @@ import {
 } from "./subagent-lifecycle-events.js";
 import type { SubagentRunRecord } from "./subagent-registry.types.js";
 
+const log = createSubsystemLogger("agents/subagent-registry-completion");
+
 export function runOutcomesEqual(
   a: SubagentRunOutcome | undefined,
   b: SubagentRunOutcome | undefined,
@@ -113,7 +116,10 @@ export async function emitSubagentEndedHookOnce(params: {
     params.entry.endedHookEmittedAt = Date.now();
     params.persist();
     return true;
-  } catch {
+  } catch (err) {
+    log.warn(
+      `failed to emit subagent_ended hook for run ${runId}: ${err instanceof Error ? err.message : String(err)}`,
+    );
     return false;
   } finally {
     params.inFlightRunIds.delete(runId);
diff --git a/src/agents/subagent-registry.ts b/src/agents/subagent-registry.ts
@@ -646,8 +646,10 @@ function restoreSubagentRunsOnce() {
     // Cold-start restore path: queue the same recovery pass that restart
     // startup also uses so resumed children are handled through one seam.
     scheduleSubagentOrphanRecovery();
-  } catch {
-    // ignore restore failures
+  } catch (err) {
+    log.warn(
+      `failed to restore subagent runs from disk: ${err instanceof Error ? err.message : String(err)}`,
+    );
   }
 }
 

Original file line number	Diff line number	Diff line change
`@@ -646,8 +646,10 @@ function restoreSubagentRunsOnce() {`
`646`	`646`	`// Cold-start restore path: queue the same recovery pass that restart`
`647`	`647`	`// startup also uses so resumed children are handled through one seam.`
`648`	`648`	`scheduleSubagentOrphanRecovery();`
`649`		`- } catch {`
`650`		`- // ignore restore failures`
	`649`	`+ } catch (err) {`
	`650`	`+ log.warn(`
	`651`	+ `failed to restore subagent runs from disk: ${err instanceof Error ? err.message : String(err)}`,
	`652`	`+ );`
`651`	`653`	`}`
`652`	`654`	`}`
`653`	`655`