fix: stabilize subagent outcome timing equality

This commit is contained in:
Gustavo Madeira Santana
2026-04-19 17:52:56 -04:00
parent bd20428d65
commit 40c71bd0d8
5 changed files with 58 additions and 9 deletions

View File

@@ -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 () => {

View File

@@ -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 {

View File

@@ -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,

View File

@@ -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;
}

View File

@@ -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,
},
}),
);