From 907bc0371c3abda0964bfa455d8ccea54f93cd26 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Sun, 24 May 2026 16:57:29 -0700 Subject: [PATCH] fix(agents): log warnings instead of swallowing subagent errors (#82943) * 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 } -> catch (err) { log.warn(...); return false } 2. restoreSubagentRunsOnce (subagent-registry.ts): catch { /* ignore */ } -> 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 * test(agents): keep provider test mocks current --------- Signed-off-by: Sebastien Tardif Co-authored-by: Peter Steinberger --- src/agents/model-fallback.probe.test.ts | 34 +++++++++++++++++++ .../pi-embedded-runner-extraparams.test.ts | 1 + ...r.sanitize-session-history.test-harness.ts | 1 + ...ed-runner.sanitize-session-history.test.ts | 1 + src/agents/subagent-registry-completion.ts | 8 ++++- src/agents/subagent-registry.ts | 6 ++-- 6 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/agents/model-fallback.probe.test.ts b/src/agents/model-fallback.probe.test.ts index ea843eb14889..3a1dfb1cc2e3 100644 --- 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 index 2da5c92a2e50..327f2807735c 100644 --- 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 index 62ec65356540..c611e9cda5c5 100644 --- 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 = {}, ) { 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 index 8c6cdef9ac7d..d46c85567b5a 100644 --- 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 index da3ad3999b93..ddfbbc770218 100644 --- 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 index a1eb95323f1e..3a8b8fb3017a 100644 --- 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)}`, + ); } }