mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-25 20:05:46 -06:00
fix(ui): keep native tool runs in one history group (#128645)
Persist authoritative run ownership on native assistant and tool-result transcript rows so Control UI history can group each run consistently. Co-authored-by: Ayaan Zaidi <hi@obviy.us>
This commit is contained in:
@@ -1,5 +1,6 @@
|
||||
// Verifies guarded session managers emit transcript update events with stable sequence ids.
|
||||
import path from "node:path";
|
||||
import { asNullableRecord } from "@openclaw/normalization-core/record-coerce";
|
||||
import type { AgentMessage } from "openclaw/plugin-sdk/agent-core";
|
||||
import { SessionManager } from "openclaw/plugin-sdk/agent-sessions";
|
||||
import { upsertSessionEntry } from "openclaw/plugin-sdk/session-store-runtime";
|
||||
@@ -380,8 +381,27 @@ describe("guardSessionManager transcript updates", () => {
|
||||
timestamp: Date.now(),
|
||||
} as AgentMessage);
|
||||
|
||||
expect(
|
||||
sm
|
||||
.getEntries()
|
||||
.filter((entry) => entry.type === "message")
|
||||
.map((entry) => ({
|
||||
role: entry.message.role,
|
||||
runId: asNullableRecord(asNullableRecord(entry.message)?.["__openclaw"])?.runId,
|
||||
})),
|
||||
).toEqual([
|
||||
{ role: "user", runId: undefined },
|
||||
{ role: "assistant", runId: "run-owning-final" },
|
||||
{ role: "toolResult", runId: "run-owning-final" },
|
||||
{ role: "assistant", runId: "run-owning-final" },
|
||||
]);
|
||||
expect(getBranchSpy).toHaveBeenCalledTimes(1);
|
||||
expect(updates.map((update) => update.messageSeq)).toEqual([2, 4]);
|
||||
expect(
|
||||
updates.map(
|
||||
(update) => asNullableRecord(asNullableRecord(update.message)?.["__openclaw"])?.runId,
|
||||
),
|
||||
).toEqual(["run-owning-final", "run-owning-final"]);
|
||||
expect(updates.map((update) => update.runId)).toEqual([undefined, "run-owning-final"]);
|
||||
getBranchSpy.mockRestore();
|
||||
});
|
||||
|
||||
@@ -24,7 +24,10 @@ import type {
|
||||
PluginHookBeforeMessageWriteEvent,
|
||||
PluginHookBeforeMessageWriteResult,
|
||||
} from "../plugins/types.js";
|
||||
import { resolveTerminalAssistantTranscriptRunId } from "../sessions/transcript-events.js";
|
||||
import {
|
||||
attachSessionTranscriptRunId,
|
||||
resolveTerminalAssistantTranscriptRunId,
|
||||
} from "../sessions/transcript-events.js";
|
||||
import { isTranscriptOnlyOpenClawAssistantModel } from "../shared/transcript-only-openclaw-assistant.js";
|
||||
import { formatContextLimitTruncationNotice } from "./embedded-agent-runner/context-truncation-notice.js";
|
||||
import {
|
||||
@@ -678,22 +681,28 @@ export function installSessionToolResultGuard(
|
||||
): {
|
||||
anchor?: TranscriptEntryAnchor;
|
||||
entryId: string;
|
||||
message: AgentMessage;
|
||||
messageSeq?: number;
|
||||
sessionTarget?: ReturnType<SessionManager["getSessionTarget"]>;
|
||||
} => {
|
||||
const runOwnedMessage = attachSessionTranscriptRunId(message, transcriptRunId);
|
||||
const parentEntryId = sessionManager.getLeafId();
|
||||
const appendParentEntryId = sessionManager.getAppendParentId();
|
||||
const { entryId, anchor } = originalAppendWithTranscriptAnchor(message as never, options);
|
||||
const { entryId, anchor } = originalAppendWithTranscriptAnchor(
|
||||
runOwnedMessage as never,
|
||||
options,
|
||||
);
|
||||
if (sessionManager.getAppendParentId() === appendParentEntryId) {
|
||||
return { entryId, ...(anchor ? { anchor } : {}) };
|
||||
return { entryId, message: runOwnedMessage, ...(anchor ? { anchor } : {}) };
|
||||
}
|
||||
void opts?.onMessagePersisted?.(message);
|
||||
void opts?.onMessagePersisted?.(runOwnedMessage);
|
||||
const sessionTarget = sessionManager.getSessionTarget();
|
||||
if (!sessionTarget) {
|
||||
return { entryId, ...(anchor ? { anchor } : {}) };
|
||||
return { entryId, message: runOwnedMessage, ...(anchor ? { anchor } : {}) };
|
||||
}
|
||||
return {
|
||||
entryId,
|
||||
message: runOwnedMessage,
|
||||
...(anchor ? { anchor } : {}),
|
||||
sessionTarget,
|
||||
messageSeq: resolveAppendedMessageSeq({
|
||||
@@ -907,6 +916,7 @@ export function installSessionToolResultGuard(
|
||||
const {
|
||||
anchor,
|
||||
entryId: result,
|
||||
message: persistedMessage,
|
||||
messageSeq,
|
||||
sessionTarget,
|
||||
} = appendMessageAndCacheTranscriptSeq(finalMessage, {
|
||||
@@ -914,9 +924,9 @@ export function installSessionToolResultGuard(
|
||||
callerInvalidatesCache || transformedMessage !== nextMessage || finalWrite.changed,
|
||||
});
|
||||
if (sessionTarget) {
|
||||
const runId = resolveTerminalAssistantTranscriptRunId(finalMessage, transcriptRunId);
|
||||
const runId = resolveTerminalAssistantTranscriptRunId(persistedMessage, transcriptRunId);
|
||||
void publishTranscriptUpdate(sessionTarget, {
|
||||
message: finalMessage,
|
||||
message: persistedMessage,
|
||||
messageId: typeof result === "string" ? result : undefined,
|
||||
...(messageSeq !== undefined ? { messageSeq } : {}),
|
||||
...(runId ? { runId } : {}),
|
||||
|
||||
@@ -3357,6 +3357,7 @@ describe("session accessor seam", () => {
|
||||
content: "second committed message",
|
||||
idempotencyKey: "ordered-turn-second",
|
||||
timestamp: 3,
|
||||
__openclaw: { runId: "run-ordered-turn" },
|
||||
},
|
||||
messageId: result.messages[1]?.messageId,
|
||||
messageSeq: 3,
|
||||
|
||||
@@ -5,7 +5,10 @@ import {
|
||||
normalizeAgentId,
|
||||
parseAgentSessionKey,
|
||||
} from "../../routing/session-key.js";
|
||||
import { resolveTerminalAssistantTranscriptRunId } from "../../sessions/transcript-events.js";
|
||||
import {
|
||||
attachSessionTranscriptRunId,
|
||||
resolveTerminalAssistantTranscriptRunId,
|
||||
} from "../../sessions/transcript-events.js";
|
||||
import { getRuntimeConfig } from "../io.js";
|
||||
import { tryResolveLegacyCompatibilityAgentId } from "../legacy.default-agent-owner.js";
|
||||
import type { OpenClawConfig } from "../types.openclaw.js";
|
||||
@@ -232,6 +235,7 @@ async function appendTranscriptTurnMessages(
|
||||
},
|
||||
{
|
||||
...appendOptions,
|
||||
message: attachSessionTranscriptRunId(appendOptions.message, options.runId),
|
||||
...((append.cwd ?? options.cwd) ? { cwd: append.cwd ?? options.cwd } : {}),
|
||||
...((append.config ?? options.config) ? { config: append.config ?? options.config } : {}),
|
||||
},
|
||||
@@ -348,7 +352,10 @@ async function persistExpectedSessionTranscriptTurn(
|
||||
expectedSessionState: options.expectedSessionState,
|
||||
expectedSessionId,
|
||||
atomicGroup: options.atomicGroup,
|
||||
messages: options.messages,
|
||||
messages: options.messages.map((append) => ({
|
||||
...append,
|
||||
message: attachSessionTranscriptRunId(append.message, options.runId),
|
||||
})),
|
||||
sessionLifecyclePatch: options.sessionLifecyclePatch,
|
||||
sessionFile: target.sessionKey!,
|
||||
touchSessionEntry: options.touchSessionEntry,
|
||||
|
||||
@@ -403,7 +403,7 @@ export type SessionTranscriptTurnPersistOptions = {
|
||||
sessionLifecyclePatch?: SessionTranscriptTurnLifecyclePatch;
|
||||
/** Message rows to append under one transcript write lock. */
|
||||
messages: readonly SessionTranscriptTurnMessageAppend[];
|
||||
/** Exact run provenance emitted only for terminal assistant message updates. */
|
||||
/** Exact run provenance persisted on output rows and emitted on terminal assistant updates. */
|
||||
runId?: string;
|
||||
/** Publish each appended message inline, one file-only invalidation, or nothing. */
|
||||
updateMode?: SessionTranscriptTurnUpdateMode;
|
||||
|
||||
@@ -555,9 +555,10 @@ vi.mock("../../plugins/hook-runner-global.js", () => {
|
||||
});
|
||||
|
||||
vi.mock("../../sessions/transcript-events.js", async (importOriginal) => {
|
||||
const { resolveTerminalAssistantTranscriptRunId } =
|
||||
const { attachSessionTranscriptRunId, resolveTerminalAssistantTranscriptRunId } =
|
||||
await importOriginal<typeof import("../../sessions/transcript-events.js")>();
|
||||
return {
|
||||
attachSessionTranscriptRunId,
|
||||
resolveTerminalAssistantTranscriptRunId,
|
||||
emitSessionTranscriptUpdate: vi.fn(
|
||||
(update: {
|
||||
|
||||
@@ -30,9 +30,10 @@ vi.mock("../config/config.js", () => ({
|
||||
}));
|
||||
|
||||
vi.mock("../sessions/transcript-events.js", async (importOriginal) => {
|
||||
const { resolveTerminalAssistantTranscriptRunId } =
|
||||
const { attachSessionTranscriptRunId, resolveTerminalAssistantTranscriptRunId } =
|
||||
await importOriginal<typeof import("../sessions/transcript-events.js")>();
|
||||
return {
|
||||
attachSessionTranscriptRunId,
|
||||
resolveTerminalAssistantTranscriptRunId,
|
||||
onInternalSessionTranscriptUpdate: (cb: typeof transcriptUpdateHandler) => {
|
||||
transcriptUpdateHandler = cb;
|
||||
|
||||
@@ -722,7 +722,7 @@ describe("worker transcript commit application", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("advances sequential commits and assigns run ownership only to the terminal assistant", async () => {
|
||||
it("persists run ownership on worker output while only the terminal envelope completes it", async () => {
|
||||
const updates: Parameters<Parameters<typeof onSessionTranscriptUpdate>[0]>[0][] = [];
|
||||
unsubscribe = onSessionTranscriptUpdate((update) => updates.push(update));
|
||||
const first = await committer.commit({ identity: IDENTITY, request: createRequest() });
|
||||
@@ -757,6 +757,17 @@ describe("worker transcript commit application", () => {
|
||||
expect(second.result.newLeafId).toBe(second.result.entryIds[0]);
|
||||
expect(second.result.newLeafId).not.toBe(first.result.newLeafId);
|
||||
const reopened = SessionManager.open(sessionTarget);
|
||||
expect(
|
||||
reopened
|
||||
.getEntries()
|
||||
.filter((entry) => entry.type === "message")
|
||||
.map((entry) => entry.message),
|
||||
).toMatchObject([
|
||||
{ role: "user" },
|
||||
{ role: "assistant", __openclaw: { runId: IDENTITY.runId } },
|
||||
{ role: "toolResult", __openclaw: { runId: IDENTITY.runId } },
|
||||
{ role: "assistant", __openclaw: { runId: IDENTITY.runId } },
|
||||
]);
|
||||
expect(reopened.getEntries().at(-1)).toMatchObject({
|
||||
id: second.result.newLeafId,
|
||||
parentId: first.result.newLeafId,
|
||||
|
||||
@@ -15,7 +15,10 @@ import {
|
||||
} from "../../config/sessions/session-accessor.js";
|
||||
import type { OpenClawConfig } from "../../config/types.openclaw.js";
|
||||
import { KeyedAsyncQueue } from "../../plugin-sdk/keyed-async-queue.js";
|
||||
import { resolveTerminalAssistantTranscriptRunId } from "../../sessions/transcript-events.js";
|
||||
import {
|
||||
attachSessionTranscriptRunId,
|
||||
resolveTerminalAssistantTranscriptRunId,
|
||||
} from "../../sessions/transcript-events.js";
|
||||
import type { WorkerConnectionIdentity } from "./connection-identity.js";
|
||||
import { resolveWorkerSessionTarget, type ResolvedWorkerSessionTarget } from "./session-target.js";
|
||||
import {
|
||||
@@ -321,8 +324,11 @@ async function applyWorkerTranscriptCommit(params: {
|
||||
sessionId: string;
|
||||
target: ResolvedWorkerSessionTarget;
|
||||
}): Promise<ApplyTranscriptCommitResult> {
|
||||
const redactedMessages = params.messages.map(
|
||||
(message) => redactTranscriptMessage(message, params.config) as CommittedAgentMessage,
|
||||
const redactedMessages = params.messages.map((message) =>
|
||||
attachSessionTranscriptRunId(
|
||||
redactTranscriptMessage(message, params.config) as CommittedAgentMessage,
|
||||
params.runId,
|
||||
),
|
||||
);
|
||||
const expectedState = {
|
||||
sessionId: params.sessionId,
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
// Transcript event tests cover transcript event parsing and compaction.
|
||||
import { afterEach, describe, expect, it, vi } from "vitest";
|
||||
import {
|
||||
attachSessionTranscriptRunId,
|
||||
emitSessionTranscriptUpdate,
|
||||
onInternalSessionTranscriptUpdate,
|
||||
onSessionTranscriptUpdate,
|
||||
@@ -16,6 +17,22 @@ afterEach(() => {
|
||||
});
|
||||
|
||||
describe("transcript events", () => {
|
||||
it.each(["assistant", "toolResult"])("persists normalized run ownership on %s rows", (role) => {
|
||||
const message = { role, content: [], __openclaw: { seq: 2 } };
|
||||
|
||||
expect(attachSessionTranscriptRunId(message, " run-owned ")).toEqual({
|
||||
...message,
|
||||
__openclaw: { seq: 2, runId: "run-owned" },
|
||||
});
|
||||
expect(attachSessionTranscriptRunId(message, " ")).toBe(message);
|
||||
});
|
||||
|
||||
it("does not assign output run ownership to user rows", () => {
|
||||
const message = { role: "user", content: "prompt" };
|
||||
|
||||
expect(attachSessionTranscriptRunId(message, "run-owned")).toBe(message);
|
||||
});
|
||||
|
||||
it("emits trimmed archive file updates only to internal listeners", () => {
|
||||
const listener = vi.fn();
|
||||
cleanup.push(onInternalSessionTranscriptUpdate(listener));
|
||||
|
||||
@@ -38,6 +38,26 @@ export type SessionTranscriptUpdate = Omit<
|
||||
/** Internal transcript update that may identify a transcript without a file path. */
|
||||
export type InternalSessionTranscriptUpdate = SessionTranscriptUpdateFields;
|
||||
|
||||
/** Persists authoritative run ownership on assistant and tool-result rows. */
|
||||
export function attachSessionTranscriptRunId<T>(message: T, runId: string | null | undefined): T {
|
||||
const normalizedRunId = normalizeOptionalString(runId);
|
||||
if (
|
||||
!normalizedRunId ||
|
||||
!isRecord(message) ||
|
||||
(message.role !== "assistant" && message.role !== "toolResult")
|
||||
) {
|
||||
return message;
|
||||
}
|
||||
const metadata = isRecord(message["__openclaw"]) ? message["__openclaw"] : {};
|
||||
if (metadata.runId === normalizedRunId) {
|
||||
return message;
|
||||
}
|
||||
return {
|
||||
...message,
|
||||
__openclaw: { ...metadata, runId: normalizedRunId },
|
||||
};
|
||||
}
|
||||
|
||||
/** Correlates only terminal assistant rows with the run that actually produced them. */
|
||||
export function resolveTerminalAssistantTranscriptRunId(
|
||||
message: unknown,
|
||||
|
||||
@@ -20,7 +20,7 @@ function transcriptMessage(
|
||||
role,
|
||||
content,
|
||||
timestamp: Date.UTC(2026, 7, 19, 12, 0, seq),
|
||||
__openclaw: { id, idempotencyKey: runId, seq },
|
||||
__openclaw: role === "user" ? { id, idempotencyKey: runId, seq } : { id, runId, seq },
|
||||
};
|
||||
}
|
||||
|
||||
@@ -68,7 +68,6 @@ suite.define(() => {
|
||||
),
|
||||
toolCallId: "call-read",
|
||||
toolName: "read",
|
||||
runId: firstRunId,
|
||||
},
|
||||
transcriptMessage(
|
||||
"assistant",
|
||||
@@ -103,7 +102,6 @@ suite.define(() => {
|
||||
),
|
||||
toolCallId: "call-render",
|
||||
toolName: "exec",
|
||||
runId: firstRunId,
|
||||
},
|
||||
transcriptMessage(
|
||||
"assistant",
|
||||
@@ -131,7 +129,6 @@ suite.define(() => {
|
||||
),
|
||||
toolCallId: "call-tool-only",
|
||||
toolName: "read",
|
||||
runId: toolOnlyRunId,
|
||||
},
|
||||
transcriptMessage(
|
||||
"user",
|
||||
@@ -157,7 +154,6 @@ suite.define(() => {
|
||||
),
|
||||
toolCallId: "call-commentary-tool-only",
|
||||
toolName: "read",
|
||||
runId: commentaryToolRunId,
|
||||
},
|
||||
],
|
||||
});
|
||||
@@ -166,21 +162,17 @@ suite.define(() => {
|
||||
const transcript = page.locator(".chat-thread-inner");
|
||||
await transcript.getByText("Caption ready for the second run.", { exact: true }).waitFor();
|
||||
|
||||
const artifactDir = process.env.OPENCLAW_CONTROL_UI_E2E_ARTIFACT_DIR?.trim();
|
||||
if (artifactDir) {
|
||||
await fs.mkdir(artifactDir, { recursive: true });
|
||||
await page.screenshot({
|
||||
path: path.join(artifactDir, "agent-run-transcript.png"),
|
||||
fullPage: true,
|
||||
});
|
||||
}
|
||||
|
||||
const assistantGroups = page.locator(".chat-group.assistant");
|
||||
expect(await assistantGroups.count()).toBe(4);
|
||||
const firstRun = assistantGroups.filter({
|
||||
hasText: "I’ll create the launch card and check the existing style first.",
|
||||
});
|
||||
expect(await firstRun.count()).toBe(1);
|
||||
const artifactDir = process.env.OPENCLAW_CONTROL_UI_E2E_ARTIFACT_DIR?.trim();
|
||||
if (artifactDir) {
|
||||
await fs.mkdir(artifactDir, { recursive: true });
|
||||
await firstRun.screenshot({ path: path.join(artifactDir, "agent-run-transcript.png") });
|
||||
}
|
||||
expect(await firstRun.locator(".chat-sender-name").count()).toBe(1);
|
||||
expect(await firstRun.locator(".chat-group-footer-actions").count()).toBe(1);
|
||||
expect(await firstRun.locator(".chat-message-actions-row").count()).toBe(0);
|
||||
|
||||
Reference in New Issue
Block a user