From 40c71bd0d8c6803456a81ca4225f34c1706ecf0f Mon Sep 17 00:00:00 2001 From: Gustavo Madeira Santana Date: Sun, 19 Apr 2026 17:52:56 -0400 Subject: [PATCH] fix: stabilize subagent outcome timing equality --- .../subagent-registry-completion.test.ts | 8 ++++- src/agents/subagent-registry-completion.ts | 11 ++++++ .../subagent-registry-lifecycle.test.ts | 34 ++++++++++++++++--- src/agents/subagent-registry-lifecycle.ts | 7 ++-- src/agents/subagent-registry.test.ts | 7 +++- 5 files changed, 58 insertions(+), 9 deletions(-) diff --git a/src/agents/subagent-registry-completion.test.ts b/src/agents/subagent-registry-completion.test.ts index 3c4d01ed9a9..54685caf88f 100644 --- a/src/agents/subagent-registry-completion.test.ts +++ b/src/agents/subagent-registry-completion.test.ts @@ -49,7 +49,7 @@ describe("emitSubagentEndedHookOnce", () => { lifecycleMocks.runSubagentEnded.mockClear(); }); - it("treats timing differences as different run outcomes", () => { + it("treats timing differences as different only after both outcomes have timing", () => { expect( mod.runOutcomesEqual( { status: "timeout", startedAt: 1_000, endedAt: 2_000, elapsedMs: 1_000 }, @@ -62,6 +62,12 @@ describe("emitSubagentEndedHookOnce", () => { { status: "error", error: "boom", startedAt: 1_000, endedAt: 2_000, elapsedMs: 1_000 }, ), ).toBe(true); + expect( + mod.runOutcomesEqual( + { status: "ok", startedAt: 1_000, endedAt: 2_000, elapsedMs: 1_000 }, + { status: "ok" }, + ), + ).toBe(true); }); it("records ended hook marker even when no subagent_ended hooks are registered", async () => { diff --git a/src/agents/subagent-registry-completion.ts b/src/agents/subagent-registry-completion.ts index e78bda27b1f..3c887e93bab 100644 --- a/src/agents/subagent-registry-completion.ts +++ b/src/agents/subagent-registry-completion.ts @@ -28,9 +28,20 @@ export function runOutcomesEqual( return false; } } + if (!runOutcomeHasTiming(a) || !runOutcomeHasTiming(b)) { + return true; + } return a.startedAt === b.startedAt && a.endedAt === b.endedAt && a.elapsedMs === b.elapsedMs; } +export function runOutcomeHasTiming(outcome: SubagentRunOutcome | undefined): boolean { + return ( + Number.isFinite(outcome?.startedAt) || + Number.isFinite(outcome?.endedAt) || + Number.isFinite(outcome?.elapsedMs) + ); +} + export function resolveLifecycleOutcomeFromRunOutcome( outcome: SubagentRunOutcome | undefined, ): SubagentLifecycleEndedOutcome { diff --git a/src/agents/subagent-registry-lifecycle.test.ts b/src/agents/subagent-registry-lifecycle.test.ts index d1abc3eb5c1..617ab6e7de6 100644 --- a/src/agents/subagent-registry-lifecycle.test.ts +++ b/src/agents/subagent-registry-lifecycle.test.ts @@ -62,11 +62,6 @@ vi.mock("./subagent-registry-cleanup.js", () => ({ resolveDeferredCleanupDecision: () => ({ kind: "give-up", reason: "retry-limit" }), })); -vi.mock("./subagent-registry-completion.js", () => ({ - runOutcomesEqual: (left: unknown, right: unknown) => - JSON.stringify(left) === JSON.stringify(right), -})); - vi.mock("./subagent-registry-helpers.js", () => ({ ANNOUNCE_COMPLETION_HARD_EXPIRY_MS: 30 * 60_000, ANNOUNCE_EXPIRY_MS: 5 * 60_000, @@ -280,6 +275,35 @@ describe("subagent registry lifecycle hardening", () => { expect(persist).toHaveBeenCalled(); }); + it("persists timing when a preexisting outcome matches without timing", async () => { + const persist = vi.fn(); + const entry = createRunEntry({ + startedAt: 2_000, + outcome: { status: "ok" }, + expectsCompletionMessage: false, + }); + + const controller = createLifecycleController({ entry, persist }); + + await expect( + controller.completeSubagentRun({ + runId: entry.runId, + endedAt: 4_250, + outcome: { status: "ok" }, + reason: SUBAGENT_ENDED_REASON_COMPLETE, + triggerCleanup: false, + }), + ).resolves.toBeUndefined(); + + expect(entry.outcome).toEqual({ + status: "ok", + startedAt: 2_000, + endedAt: 4_250, + elapsedMs: 2_250, + }); + expect(persist).toHaveBeenCalled(); + }); + it("does not wait for a completion reply when the run does not expect one", async () => { const entry = createRunEntry({ expectsCompletionMessage: false, diff --git a/src/agents/subagent-registry-lifecycle.ts b/src/agents/subagent-registry-lifecycle.ts index c278d62a86d..2caa54831c7 100644 --- a/src/agents/subagent-registry-lifecycle.ts +++ b/src/agents/subagent-registry-lifecycle.ts @@ -23,7 +23,7 @@ import { resolveCleanupCompletionReason, resolveDeferredCleanupDecision, } from "./subagent-registry-cleanup.js"; -import { runOutcomesEqual } from "./subagent-registry-completion.js"; +import { runOutcomeHasTiming, runOutcomesEqual } from "./subagent-registry-completion.js"; import { ANNOUNCE_COMPLETION_HARD_EXPIRY_MS, ANNOUNCE_EXPIRY_MS, @@ -594,7 +594,10 @@ export function createSubagentRegistryLifecycleController(params: { startedAt: entry.startedAt, endedAt, }); - if (!runOutcomesEqual(entry.outcome, outcome)) { + const shouldPersistOutcome = + !runOutcomesEqual(entry.outcome, outcome) || + (!runOutcomeHasTiming(entry.outcome) && runOutcomeHasTiming(outcome)); + if (shouldPersistOutcome) { entry.outcome = outcome; mutated = true; } diff --git a/src/agents/subagent-registry.test.ts b/src/agents/subagent-registry.test.ts index 2c8b2de16f0..8c872cd3b2b 100644 --- a/src/agents/subagent-registry.test.ts +++ b/src/agents/subagent-registry.test.ts @@ -191,7 +191,12 @@ describe("subagent registry seam flow", () => { task: "finish the task", cleanup: "delete", roundOneReply: "final completion reply", - outcome: { status: "ok" }, + outcome: { + status: "ok", + startedAt: 111, + endedAt: 222, + elapsedMs: 111, + }, }), );