Skip to content

Commit 79896a2

Browse files
clawsweeper[bot]harjothkharaclaudeTakhoffman
authored
fix(outbound): keep channel send durable when transcript mirror fails (#89626) (#89812)
Summary: - The PR wraps outbound post-delivery transcript mirroring in warning-only error handling and adds regression tests for thrown and not-ok mirror append failures. - PR surface: Source +16, Tests +61. Total +77 across 2 files. - Reproducibility: yes. A high-confidence source reproduction is to make appendAssistantMessageToSessionTransc ... a/outbound/deliver.ts:1970 and the caller retry path treats that exception as a failed direct announcement. Automerge notes: - PR branch already contained follow-up commit before automerge: fix(outbound): keep channel send durable when transcript mirror fails… Validation: - ClawSweeper review passed for head dfe0fd7. - Required merge gates passed before the squash merge. Prepared head SHA: dfe0fd7 Review: #89812 (comment) Co-authored-by: harjoth <harjoth.khara@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com> Co-authored-by: clawsweeper[bot] <274271284+clawsweeper[bot]@users.noreply.github.com> Approved-by: takhoffman Co-authored-by: takhoffman <781889+takhoffman@users.noreply.github.com>
1 parent a7d5ae1 commit 79896a2

2 files changed

Lines changed: 86 additions & 9 deletions

File tree

‎src/infra/outbound/deliver.test.ts‎

Lines changed: 62 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { chunkText } from "../../auto-reply/chunk.js";
44
import { createMessageReceiptFromOutboundResults } from "../../channels/message/receipt.js";
55
import type { ChannelOutboundAdapter } from "../../channels/plugins/types.js";
66
import type { OpenClawConfig } from "../../config/config.js";
7+
import type { SessionTranscriptAppendResult } from "../../config/sessions/transcript.js";
78
import * as mediaCapabilityModule from "../../media/read-capability.js";
89
import { createHookRunner } from "../../plugins/hooks.js";
910
import { addTestHook } from "../../plugins/hooks.test-helpers.js";
@@ -28,7 +29,9 @@ import {
2829
import { resolvePreferredOpenClawTmpDir } from "../tmp-openclaw-dir.js";
2930

3031
const mocks = vi.hoisted(() => ({
31-
appendAssistantMessageToSessionTranscript: vi.fn(async () => ({ ok: true, sessionFile: "x" })),
32+
appendAssistantMessageToSessionTranscript: vi.fn<() => Promise<SessionTranscriptAppendResult>>(
33+
async () => ({ ok: true, sessionFile: "x", messageId: "m" }),
34+
),
3235
}));
3336
const hookMocks = vi.hoisted(() => ({
3437
runner: {
@@ -3070,6 +3073,64 @@ describe("deliverOutboundPayloads", () => {
30703073
expect(appendOptions?.config).toBe(cfg);
30713074
});
30723075

3076+
it("does not fail the channel send when the post-delivery transcript mirror throws", async () => {
3077+
const sendMatrix = vi.fn().mockResolvedValue({ messageId: "m1", roomId: "!room:example" });
3078+
mocks.appendAssistantMessageToSessionTranscript.mockClear();
3079+
mocks.appendAssistantMessageToSessionTranscript.mockRejectedValueOnce(
3080+
new Error("session file changed while embedded prompt lock was released"),
3081+
);
3082+
3083+
const results = await deliverOutboundPayloads({
3084+
cfg: {},
3085+
channel: "matrix",
3086+
to: "!room:example",
3087+
payloads: [{ text: "done" }],
3088+
deps: { matrix: sendMatrix },
3089+
mirror: {
3090+
sessionKey: "agent:main:main",
3091+
text: "done",
3092+
idempotencyKey: "idem-89626",
3093+
},
3094+
});
3095+
3096+
expect(sendMatrix).toHaveBeenCalledTimes(1);
3097+
expect(results).toHaveLength(1);
3098+
const warnCall = requireMockCall(logMocks.warn, "warn");
3099+
expect(warnCall[0]).toContain(
3100+
"failed to mirror outbound delivery into session transcript; channel send already succeeded",
3101+
);
3102+
expect(warnCall[1]).toMatchObject({ channel: "matrix", sessionKey: "agent:main:main" });
3103+
});
3104+
3105+
it("does not fail the channel send when the transcript mirror reports not-ok", async () => {
3106+
const sendMatrix = vi.fn().mockResolvedValue({ messageId: "m1", roomId: "!room:example" });
3107+
mocks.appendAssistantMessageToSessionTranscript.mockClear();
3108+
mocks.appendAssistantMessageToSessionTranscript.mockResolvedValueOnce({
3109+
ok: false,
3110+
reason: "session locked",
3111+
});
3112+
3113+
const results = await deliverOutboundPayloads({
3114+
cfg: {},
3115+
channel: "matrix",
3116+
to: "!room:example",
3117+
payloads: [{ text: "done" }],
3118+
deps: { matrix: sendMatrix },
3119+
mirror: {
3120+
sessionKey: "agent:main:main",
3121+
text: "done",
3122+
idempotencyKey: "idem-89626-b",
3123+
},
3124+
});
3125+
3126+
expect(sendMatrix).toHaveBeenCalledTimes(1);
3127+
expect(results).toHaveLength(1);
3128+
const warnCall = requireMockCall(logMocks.warn, "warn");
3129+
expect(warnCall[0]).toContain(
3130+
"failed to mirror outbound delivery into session transcript; channel send already succeeded",
3131+
);
3132+
});
3133+
30733134
it("emits message_sent success for text-only deliveries", async () => {
30743135
hookMocks.runner.hasHooks.mockReturnValue(true);
30753136
const sendMatrix = vi.fn().mockResolvedValue({ messageId: "m1", roomId: "!room:example" });

‎src/infra/outbound/deliver.ts‎

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1967,14 +1967,30 @@ async function deliverOutboundPayloadsCore(
19671967
mediaUrls: deliveredMirror.mediaUrls,
19681968
});
19691969
if (mirrorText) {
1970-
const { appendAssistantMessageToSessionTranscript } = await loadTranscriptRuntime();
1971-
await appendAssistantMessageToSessionTranscript({
1972-
agentId: params.mirror.agentId,
1973-
sessionKey: params.mirror.sessionKey,
1974-
text: mirrorText,
1975-
idempotencyKey: params.mirror.idempotencyKey,
1976-
config: params.cfg,
1977-
});
1970+
// Transcript mirroring is best-effort bookkeeping after platform send.
1971+
// Keep mirror failures non-fatal so callers do not retry an already-sent
1972+
// channel payload and create duplicate delivery.
1973+
try {
1974+
const { appendAssistantMessageToSessionTranscript } = await loadTranscriptRuntime();
1975+
const mirrorResult = await appendAssistantMessageToSessionTranscript({
1976+
agentId: params.mirror.agentId,
1977+
sessionKey: params.mirror.sessionKey,
1978+
text: mirrorText,
1979+
idempotencyKey: params.mirror.idempotencyKey,
1980+
config: params.cfg,
1981+
});
1982+
if (!mirrorResult.ok) {
1983+
log.warn(
1984+
`failed to mirror outbound delivery into session transcript; channel send already succeeded: ${mirrorResult.reason}`,
1985+
{ channel, to, sessionKey: params.mirror.sessionKey },
1986+
);
1987+
}
1988+
} catch (err) {
1989+
log.warn(
1990+
`failed to mirror outbound delivery into session transcript; channel send already succeeded: ${formatErrorMessage(err)}`,
1991+
{ channel, to, sessionKey: params.mirror.sessionKey },
1992+
);
1993+
}
19781994
}
19791995
}
19801996

0 commit comments

Comments
 (0)