From 5bc80dbe27d991987a5478944ad1c4d28eccbb7f Mon Sep 17 00:00:00 2001 From: Arnab Saha Date: Sun, 31 May 2026 14:15:15 -0700 Subject: [PATCH] fix(diagnostics): carry session UUID on interactive dispatch events Carry the canonical session UUID from the session store into interactive dispatch diagnostic lifecycle events, matching the cron path so downstream diagnostic consumers can join events back to the JSONL transcript id. Guard native command redirects by only attaching the UUID when the lifecycle session key matches the session-store lookup key, avoiding a target UUID under a source conversation key. Verification: - `pnpm test src/auto-reply/reply/dispatch-from-config.test.ts -t "carries the session store UUID|does not stamp a command target"` - `.agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main --prompt ...` - synthetic merge-tree against current `origin/main` Co-authored-by: Claude Opus 4.8 --- .../reply/dispatch-from-config.test.ts | 66 +++++++++++++++++++ src/auto-reply/reply/dispatch-from-config.ts | 15 ++++- 2 files changed, 80 insertions(+), 1 deletion(-) diff --git a/src/auto-reply/reply/dispatch-from-config.test.ts b/src/auto-reply/reply/dispatch-from-config.test.ts index e0fd123f5d75..e2e5ad14c4ec 100644 --- a/src/auto-reply/reply/dispatch-from-config.test.ts +++ b/src/auto-reply/reply/dispatch-from-config.test.ts @@ -5043,6 +5043,72 @@ describe("dispatchReplyFromConfig", () => { expect(processedEvent?.sessionKey).toBe("agent:main:main"); }); + it("carries the session store UUID on interactive diagnostic events", async () => { + setNoAbort(); + sessionStoreMocks.currentEntry = { + sessionId: "test-uuid-1234", + }; + const cfg = { diagnostics: { enabled: true } } as OpenClawConfig; + const dispatcher = createDispatcher(); + const ctx = buildTestCtx({ + Provider: "slack", + Surface: "slack", + SessionKey: "agent:main:main", + MessageSid: "msg-1", + To: "slack:C123", + }); + + const replyResolver = async () => ({ text: "hi" }) satisfies ReplyPayload; + await dispatchReplyFromConfig({ ctx, cfg, dispatcher, replyResolver }); + + expect(diagnosticMocks.logMessageQueued).toHaveBeenCalledTimes(1); + expect(diagnosticMocks.logMessageQueued).toHaveBeenCalledWith( + expect.objectContaining({ + sessionId: "test-uuid-1234", + sessionKey: "agent:main:main", + }), + ); + expect(diagnosticMocks.logSessionStateChange).toHaveBeenCalledWith({ + sessionId: "test-uuid-1234", + sessionKey: "agent:main:main", + state: "processing", + reason: "message_start", + }); + }); + + it("does not stamp a command target's UUID under the source session key", async () => { + // Native command turn: runs in the source conversation but targets a + // different session. resolveSessionStoreLookup is command-target-aware, so + // its entry is the *target's* — while the lifecycle reports the *source* + // key. The UUID must NOT be attached here, to avoid mis-associating a + // session id with the wrong session key (agentweave#187). + setNoAbort(); + sessionStoreMocks.currentEntry = { + sessionId: "target-session-uuid-9999", + }; + const cfg = { diagnostics: { enabled: true } } as OpenClawConfig; + const dispatcher = createDispatcher(); + const ctx = buildTestCtx({ + Provider: "slack", + Surface: "slack", + CommandSource: "native", + SessionKey: "agent:main:source-convo", + CommandTargetSessionKey: "agent:main:target-session", + MessageSid: "msg-1", + To: "slack:C123", + }); + + const replyResolver = async () => ({ text: "hi" }) satisfies ReplyPayload; + await dispatchReplyFromConfig({ ctx, cfg, dispatcher, replyResolver }); + + expect(diagnosticMocks.logMessageQueued).toHaveBeenCalledTimes(1); + const queued = diagnosticMocks.logMessageQueued.mock.calls[0]?.[0] as + | { sessionId?: unknown; sessionKey?: unknown } + | undefined; + expect(queued?.sessionKey).toBe("agent:main:source-convo"); + expect(queued?.sessionId).toBeUndefined(); + }); + it("marks diagnostic progress for real reply events but not reply start callbacks", async () => { setNoAbort(); const cfg = { diagnostics: { enabled: true } } as OpenClawConfig; diff --git a/src/auto-reply/reply/dispatch-from-config.ts b/src/auto-reply/reply/dispatch-from-config.ts index 08a48baae882..740cb0a91a1d 100644 --- a/src/auto-reply/reply/dispatch-from-config.ts +++ b/src/auto-reply/reply/dispatch-from-config.ts @@ -1035,12 +1035,26 @@ export async function dispatchReplyFromConfig( normalizeOptionalString(ctx.SessionKey) ?? normalizeOptionalString(ctx.CommandTargetSessionKey); const startTime = diagnosticsEnabled ? Date.now() : 0; const canTrackSession = diagnosticsEnabled && Boolean(sessionKey); + const initialSessionStoreEntry = resolveSessionStoreLookup(ctx, cfg); + // resolveSessionStoreLookup is command-target-aware (it prefers + // resolveCommandTurnTargetSessionKey), whereas the lifecycle's sessionKey is + // source-first (ctx.SessionKey). On a native command turn that targets a + // different session, the resolved entry can belong to the *target* while the + // lifecycle reports the *source* key — so only carry the UUID when the entry + // is for the same session the lifecycle reports, to avoid mis-associating a + // session id with the wrong session key. When they diverge, emit sessionKey + // only (prior behavior). + const lifecycleSessionId = + initialSessionStoreEntry.sessionKey === sessionKey + ? initialSessionStoreEntry.entry?.sessionId + : undefined; const messageLifecycle = createDiagnosticMessageLifecycle({ enabled: diagnosticsEnabled, channel, chatId, messageId, sessionKey, + sessionId: lifecycleSessionId, source: "dispatch", processingReason: "message_start", startedAtMs: startTime, @@ -1129,7 +1143,6 @@ export async function dispatchReplyFromConfig( inboundDedupeReplayUnsafe = true; }; - const initialSessionStoreEntry = resolveSessionStoreLookup(ctx, cfg); const boundAcpDispatchSessionKey = resolveBoundAcpDispatchSessionKey({ ctx, cfg }); const acpDispatchSessionKey = boundAcpDispatchSessionKey ?? initialSessionStoreEntry.sessionKey ?? sessionKey;