Skip to content

Commit 310084b

Browse files
Kaspreclaude
Kaspre
and
claude
committed
fix(delivery): track and log silent delivery failures
Add structured deliveryStatus return and diagnostic [delivery] log line to deliverAgentCommandResult, making five previously-invisible failure paths visible: no channel, internal channel, no target, thrown error, and empty results. deliveryStatus contract: - succeeded: boolean (true = at least one payload delivered) - hadPartialFailure: true when onError fired but some payloads succeeded - error: true when delivery threw (bestEffort) or preflight check failed - Early return (no payloads) includes deliveryStatus when deliver=true - JSON output (--json --deliver) includes deliveryStatus in the envelope Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent b3b62ed commit 310084b

2 files changed

Lines changed: 499 additions & 23 deletions

File tree

‎src/agents/command/delivery.test.ts‎

Lines changed: 373 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,18 @@
1-
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
1+
import { afterAll, afterEach, beforeEach, describe, expect, it, vi } from "vitest";
22
import type { ReplyPayload } from "../../auto-reply/reply-payload.js";
3+
import * as channelPluginsModule from "../../channels/plugins/index.js";
34
import type { ChannelOutboundAdapter } from "../../channels/plugins/types.js";
45
import type { CliDeps } from "../../cli/outbound-send-deps.js";
56
import type { OpenClawConfig } from "../../config/config.js";
7+
import type { SessionEntry } from "../../config/sessions.js";
8+
import * as agentDeliveryModule from "../../infra/outbound/agent-delivery.js";
9+
import type { AgentDeliveryPlan } from "../../infra/outbound/agent-delivery.js";
10+
import * as deliverModule from "../../infra/outbound/deliver.js";
11+
import type { OutboundDeliveryResult } from "../../infra/outbound/deliver.js";
612
import { setActivePluginRegistry } from "../../plugins/runtime.js";
13+
import type { RuntimeEnv } from "../../runtime.js";
714
import { createOutboundTestPlugin, createTestRegistry } from "../../test-utils/channel-plugins.js";
15+
import * as messageChannelModule from "../../utils/message-channel.js";
816
import { deliverAgentCommandResult, normalizeAgentCommandReplyPayloads } from "./delivery.js";
917
import type { AgentCommandOpts } from "./types.js";
1018

@@ -264,3 +272,367 @@ describe("normalizeAgentCommandReplyPayloads", () => {
264272
]);
265273
});
266274
});
275+
276+
// ---------------------------------------------------------------------------
277+
// deliveryStatus tracking tests (PR #53961)
278+
// Uses spyOn approach for delivery mocking — separate from upstream normalize tests.
279+
// ---------------------------------------------------------------------------
280+
281+
// ---------------------------------------------------------------------------
282+
// Spies (vi.mock has module-resolution issues in forks pool when transitive
283+
// dependencies are pre-loaded by test/setup.ts — vi.spyOn is reliable).
284+
// ---------------------------------------------------------------------------
285+
286+
const deliverSpy = vi.spyOn(deliverModule, "deliverOutboundPayloads");
287+
const deliveryPlanSpy = vi.spyOn(agentDeliveryModule, "resolveAgentDeliveryPlan");
288+
const outboundTargetSpy = vi.spyOn(agentDeliveryModule, "resolveAgentOutboundTarget");
289+
const channelPluginSpy = vi.spyOn(channelPluginsModule, "getChannelPlugin");
290+
const isInternalSpy = vi.spyOn(messageChannelModule, "isInternalMessageChannel");
291+
292+
afterAll(() => {
293+
vi.restoreAllMocks();
294+
});
295+
296+
// ---------------------------------------------------------------------------
297+
// Helpers
298+
// ---------------------------------------------------------------------------
299+
300+
function createRuntime(): RuntimeEnv & {
301+
log: ReturnType<typeof vi.fn>;
302+
error: ReturnType<typeof vi.fn>;
303+
} {
304+
return { log: vi.fn(), error: vi.fn() } as unknown as RuntimeEnv & {
305+
log: ReturnType<typeof vi.fn>;
306+
error: ReturnType<typeof vi.fn>;
307+
};
308+
}
309+
310+
/** Set up spies for a standard successful Discord delivery. */
311+
function setupSuccessfulDelivery() {
312+
deliverSpy.mockResolvedValue([
313+
{ channel: "discord", messageId: "msg-1" } as OutboundDeliveryResult,
314+
]);
315+
deliveryPlanSpy.mockReturnValue({
316+
baseDelivery: {} as unknown as AgentDeliveryPlan["baseDelivery"],
317+
resolvedChannel: "discord",
318+
resolvedTo: "channel:123456",
319+
resolvedAccountId: "bot-1",
320+
});
321+
outboundTargetSpy.mockReturnValue({
322+
resolvedTarget: {
323+
ok: true as const,
324+
to: "channel:123456",
325+
} as ReturnType<typeof agentDeliveryModule.resolveAgentOutboundTarget>["resolvedTarget"],
326+
resolvedTo: "channel:123456",
327+
targetMode: "explicit" as const,
328+
});
329+
channelPluginSpy.mockReturnValue({ name: "discord" } as unknown as ReturnType<
330+
typeof channelPluginsModule.getChannelPlugin
331+
>);
332+
isInternalSpy.mockReturnValue(false);
333+
}
334+
335+
async function runDelivery(
336+
opts: Record<string, unknown>,
337+
overrides?: { runtime?: ReturnType<typeof createRuntime> },
338+
) {
339+
const runtime = overrides?.runtime ?? createRuntime();
340+
const result = await deliverAgentCommandResult({
341+
cfg: {} as unknown as OpenClawConfig,
342+
deps: {} as unknown as CliDeps,
343+
runtime,
344+
opts: opts as unknown as AgentCommandOpts,
345+
outboundSession: { key: "agent:main:discord:direct:12345" },
346+
sessionEntry: {
347+
lastChannel: "discord",
348+
lastTo: "channel:123456",
349+
} as unknown as SessionEntry,
350+
result: { payloads: [{ text: "hello" }], meta: { durationMs: 1 } },
351+
payloads: [{ text: "hello" }],
352+
});
353+
return { runtime, result };
354+
}
355+
356+
function logMessages(runtime: ReturnType<typeof createRuntime>): string[] {
357+
return runtime.log.mock.calls.map((c: unknown[]) => String(c[0]));
358+
}
359+
360+
// ---------------------------------------------------------------------------
361+
// Tests
362+
// ---------------------------------------------------------------------------
363+
364+
describe("deliverAgentCommandResult — delivery status tracking", () => {
365+
beforeEach(() => {
366+
// Clear call counts/results but keep spies attached (vi.restoreAllMocks
367+
// would disconnect them from the module exports).
368+
deliverSpy.mockReset();
369+
deliveryPlanSpy.mockReset();
370+
outboundTargetSpy.mockReset();
371+
channelPluginSpy.mockReset();
372+
isInternalSpy.mockReset();
373+
setupSuccessfulDelivery();
374+
});
375+
376+
it("returns deliveryStatus.succeeded=true on successful delivery", async () => {
377+
const { result, runtime } = await runDelivery({
378+
message: "hello",
379+
deliver: true,
380+
channel: "discord",
381+
to: "channel:123456",
382+
});
383+
384+
expect(deliverSpy).toHaveBeenCalledOnce();
385+
expect(result.deliveryStatus).toEqual({
386+
requested: true,
387+
attempted: true,
388+
succeeded: true,
389+
});
390+
// No warning log on success
391+
expect(logMessages(runtime).some((msg) => msg.includes("[delivery]"))).toBe(false);
392+
});
393+
394+
it("returns no deliveryStatus when deliver is false", async () => {
395+
const { result } = await runDelivery({
396+
message: "hello",
397+
deliver: false,
398+
});
399+
400+
expect(deliverSpy).not.toHaveBeenCalled();
401+
expect(result.deliveryStatus).toBeUndefined();
402+
});
403+
404+
it("logs warning and returns succeeded=false when delivery target is missing", async () => {
405+
outboundTargetSpy.mockReturnValue({
406+
resolvedTarget: null,
407+
resolvedTo: undefined,
408+
targetMode: "implicit" as const,
409+
});
410+
411+
const { result, runtime } = await runDelivery({
412+
message: "hello",
413+
deliver: true,
414+
channel: "discord",
415+
});
416+
417+
expect(deliverSpy).not.toHaveBeenCalled();
418+
expect(result.deliveryStatus).toEqual({
419+
requested: true,
420+
attempted: false,
421+
succeeded: false,
422+
});
423+
expect(
424+
logMessages(runtime).some((msg) =>
425+
msg.includes("[delivery] delivery requested but not completed"),
426+
),
427+
).toBe(true);
428+
});
429+
430+
it("logs warning and returns succeeded=false when deliverOutboundPayloads returns empty", async () => {
431+
deliverSpy.mockResolvedValue([]);
432+
433+
const { result, runtime } = await runDelivery({
434+
message: "hello",
435+
deliver: true,
436+
channel: "discord",
437+
to: "channel:123456",
438+
});
439+
440+
expect(deliverSpy).toHaveBeenCalledOnce();
441+
expect(result.deliveryStatus).toEqual({
442+
requested: true,
443+
attempted: true,
444+
succeeded: false,
445+
});
446+
expect(logMessages(runtime).some((msg) => msg.includes("delivery returned zero results"))).toBe(
447+
true,
448+
);
449+
});
450+
451+
it("catches thrown error in bestEffort mode without re-throwing", async () => {
452+
deliverSpy.mockRejectedValue(new Error("Discord API timeout"));
453+
454+
const { result, runtime } = await runDelivery({
455+
message: "hello",
456+
deliver: true,
457+
bestEffortDeliver: true,
458+
channel: "discord",
459+
to: "channel:123456",
460+
});
461+
462+
expect(result.deliveryStatus).toEqual({
463+
requested: true,
464+
attempted: true,
465+
succeeded: false,
466+
error: true,
467+
});
468+
// Error should be logged via logDeliveryError -> runtime.error or runtime.log
469+
const allOutput = [...runtime.error.mock.calls, ...runtime.log.mock.calls].map((c) =>
470+
String(c[0]),
471+
);
472+
expect(allOutput.some((msg) => msg.includes("Discord API timeout"))).toBe(true);
473+
// Structured log should report "threw an error", not "zero results"
474+
expect(allOutput.some((msg) => msg.includes("delivery threw an error"))).toBe(true);
475+
});
476+
477+
it("re-throws error when bestEffort is false", async () => {
478+
deliverSpy.mockRejectedValue(new Error("Discord API timeout"));
479+
480+
await expect(
481+
runDelivery({
482+
message: "hello",
483+
deliver: true,
484+
bestEffortDeliver: false,
485+
channel: "discord",
486+
to: "channel:123456",
487+
}),
488+
).rejects.toThrow("Discord API timeout");
489+
});
490+
491+
it("returns deliveryStatus on early return when deliver=true but no payloads", async () => {
492+
const runtime = createRuntime();
493+
const result = await deliverAgentCommandResult({
494+
cfg: {} as unknown as OpenClawConfig,
495+
deps: {} as unknown as CliDeps,
496+
runtime,
497+
opts: {
498+
message: "hello",
499+
deliver: true,
500+
channel: "discord",
501+
to: "channel:123456",
502+
} as unknown as AgentCommandOpts,
503+
outboundSession: undefined,
504+
sessionEntry: undefined,
505+
result: { payloads: [], meta: { durationMs: 1 } },
506+
payloads: [],
507+
});
508+
509+
expect(result.deliveryStatus).toEqual({
510+
requested: true,
511+
attempted: false,
512+
succeeded: false,
513+
});
514+
expect(logMessages(runtime)).toContain("No reply from agent.");
515+
});
516+
517+
it("returns no deliveryStatus on early return when deliver=false and no payloads", async () => {
518+
const runtime = createRuntime();
519+
const result = await deliverAgentCommandResult({
520+
cfg: {} as unknown as OpenClawConfig,
521+
deps: {} as unknown as CliDeps,
522+
runtime,
523+
opts: {
524+
message: "hello",
525+
deliver: false,
526+
} as unknown as AgentCommandOpts,
527+
outboundSession: undefined,
528+
sessionEntry: undefined,
529+
result: { payloads: [], meta: { durationMs: 1 } },
530+
payloads: [],
531+
});
532+
533+
expect(result.deliveryStatus).toBeUndefined();
534+
});
535+
536+
it("returns succeeded=true with hadPartialFailure when onError fires but results exist", async () => {
537+
deliverSpy.mockImplementation(async (opts) => {
538+
// Simulate partial failure: onError fires for one payload, but results still returned
539+
opts.onError?.(new Error("Payload 2 failed"), { text: "hello" } as never);
540+
return [{ channel: "discord", messageId: "msg-1" } as OutboundDeliveryResult];
541+
});
542+
543+
const { result, runtime } = await runDelivery({
544+
message: "hello",
545+
deliver: true,
546+
channel: "discord",
547+
to: "channel:123456",
548+
});
549+
550+
expect(result.deliveryStatus).toEqual({
551+
requested: true,
552+
attempted: true,
553+
succeeded: true,
554+
hadPartialFailure: true,
555+
});
556+
// No [delivery] warning — succeeded is true
557+
expect(logMessages(runtime).some((msg) => msg.includes("[delivery]"))).toBe(false);
558+
});
559+
560+
it("logs warning when channel resolves to internal", async () => {
561+
isInternalSpy.mockReturnValue(true);
562+
deliveryPlanSpy.mockReturnValue({
563+
baseDelivery: {} as unknown as AgentDeliveryPlan["baseDelivery"],
564+
resolvedChannel: "__internal__",
565+
resolvedTo: undefined,
566+
});
567+
568+
const { result, runtime } = await runDelivery({
569+
message: "hello",
570+
deliver: true,
571+
bestEffortDeliver: true,
572+
});
573+
574+
expect(deliverSpy).not.toHaveBeenCalled();
575+
expect(result.deliveryStatus).toEqual({
576+
requested: true,
577+
attempted: false,
578+
succeeded: false,
579+
error: true,
580+
});
581+
expect(logMessages(runtime).some((msg) => msg.includes("channel resolved to internal"))).toBe(
582+
true,
583+
);
584+
});
585+
586+
it("includes deliveryStatus in JSON output when deliver=true", async () => {
587+
const { result, runtime } = await runDelivery({
588+
message: "hello",
589+
deliver: true,
590+
json: true,
591+
channel: "discord",
592+
to: "channel:123456",
593+
});
594+
595+
expect(result.deliveryStatus).toEqual({
596+
requested: true,
597+
attempted: true,
598+
succeeded: true,
599+
});
600+
// JSON output should include deliveryStatus
601+
const jsonOutput = logMessages(runtime).find((msg) => msg.startsWith("{"));
602+
expect(jsonOutput).toBeDefined();
603+
const parsed = JSON.parse(jsonOutput!);
604+
expect(parsed.deliveryStatus).toEqual({
605+
requested: true,
606+
attempted: true,
607+
succeeded: true,
608+
});
609+
});
610+
611+
it("omits deliveryStatus from JSON output when deliver=false", async () => {
612+
const runtime = createRuntime();
613+
isInternalSpy.mockReturnValue(false);
614+
channelPluginSpy.mockReturnValue({ name: "discord" } as unknown as ReturnType<
615+
typeof channelPluginsModule.getChannelPlugin
616+
>);
617+
await deliverAgentCommandResult({
618+
cfg: {} as unknown as OpenClawConfig,
619+
deps: {} as unknown as CliDeps,
620+
runtime,
621+
opts: {
622+
message: "hello",
623+
deliver: false,
624+
json: true,
625+
channel: "discord",
626+
} as unknown as AgentCommandOpts,
627+
outboundSession: undefined,
628+
sessionEntry: undefined,
629+
result: { payloads: [{ text: "hello" }], meta: { durationMs: 1 } },
630+
payloads: [{ text: "hello" }],
631+
});
632+
633+
const jsonOutput = logMessages(runtime).find((msg) => msg.startsWith("{"));
634+
expect(jsonOutput).toBeDefined();
635+
const parsed = JSON.parse(jsonOutput!);
636+
expect(parsed.deliveryStatus).toBeUndefined();
637+
});
638+
});

0 commit comments

Comments
 (0)