fix(sessions): keep admin events out of model context (#128346)

This commit is contained in:
Peter Steinberger
2026-08-23 14:09:39 -07:00
committed by GitHub
parent 3a4b3dfecc
commit c48e973c66
24 changed files with 478 additions and 359 deletions
+2 -2
View File
@@ -1285,10 +1285,10 @@ See [Multi-Agent Sandbox & Tools](/tools/multi-agent-sandbox-tools) for preceden
- `defaultSpawnContext`: default native subagent context for thread-bound spawns (`"fork"` or `"isolated"`). Defaults to `"fork"`.
- **`sharing`**: controls which per-session collaboration modes owners and `operator.admin` connections may select. Every flag defaults to `true`; setting one to `false` removes that choice from the Control UI and makes create-time visibility or `session.visibility.set` reject it. New sessions start `shared` unless the Control UI starts one as a draft.
- `readOnly`: allow `read-only`, where non-members can watch but cannot send, steer, abort, approve, or mutate session state.
- `suggest`: allow `suggest`. In this phase it enforces the same admission behavior as `read-only`; the suggestion queue is a later feature.
- `suggest`: allow `suggest`, where viewers can submit suggestions for the session owner or an `operator.admin` connection to send, queue, edit, or dismiss without granting direct access to send or manage the session.
- `drafts`: allow `draft`, which hides the session from non-admin, non-owner session lists and event broadcasts.
Membership and visibility changes are written into the session transcript as system notes. These controls coordinate operators sharing one agent; they are not a security boundary between tenants. Use separate Gateways or agents when work requires isolation.
Session visibility and membership are maintained as canonical sharing state. Structured `session.sharing` and `session.suggestion` change events refresh connected clients without adding administrative narration to conversation transcripts. These controls coordinate operators sharing one agent; they are not a security boundary between tenants. Use separate Gateways or agents when work requires isolation.
</Accordion>
@@ -231,55 +231,71 @@ src/write.ts
expect(preparation.messages.map((message) => message.role)).toEqual(["user", "toolResult"]);
});
it("preserves earlier branch context while excluding private shell output", async () => {
const model = createModel(8192);
const capture = createCapturingStream(model);
const shellMessage: AgentMessage = {
role: "bashExecution",
command: "private command",
output: `private output marker ${"x".repeat(80_000)}`,
exitCode: 0,
cancelled: false,
truncated: false,
timestamp: 2,
excludeFromContext: true,
};
const entries: SessionTreeEntry[] = [
createMessageEntry({ role: "user", content: "important original request", timestamp: 1 }, 0),
createMessageEntry(shellMessage, 1),
createMessageEntry({ role: "user", content: "continue branch", timestamp: 3 }, 2),
];
it.each(["shell", "custom"] as const)(
"preserves earlier branch context while excluding private %s activity",
async (kind) => {
const model = createModel(8192);
const capture = createCapturingStream(model);
const excludedMessage: AgentMessage =
kind === "shell"
? {
role: "bashExecution",
command: "private command",
output: `private output marker ${"x".repeat(80_000)}`,
exitCode: 0,
cancelled: false,
truncated: false,
timestamp: 2,
excludeFromContext: true,
}
: {
role: "custom",
customType: "openclaw.operator-activity",
content: `private output marker ${"x".repeat(80_000)}`,
display: true,
timestamp: 2,
excludeFromContext: true,
};
const entries: SessionTreeEntry[] = [
createMessageEntry(
{ role: "user", content: "important original request", timestamp: 1 },
0,
),
createMessageEntry(excludedMessage, 1),
createMessageEntry({ role: "user", content: "continue branch", timestamp: 3 }, 2),
];
const preparation = prepareBranchEntries(entries, 100);
expect(preparation.messages).toMatchObject([
{ role: "user", content: "important original request" },
{ role: "user", content: "continue branch" },
]);
expect(preparation.totalTokens).toBeLessThan(100);
const preparation = prepareBranchEntries(entries, 100);
expect(preparation.messages).toMatchObject([
{ role: "user", content: "important original request" },
{ role: "user", content: "continue branch" },
]);
expect(preparation.totalTokens).toBeLessThan(100);
const visibleEntries = entries.map((entry, index) =>
index === 1
? createMessageEntry({ ...shellMessage, excludeFromContext: false }, index)
: entry,
);
expect(prepareBranchEntries(visibleEntries, 100).messages).toMatchObject([
{ role: "user", content: "continue branch" },
]);
const visibleEntries = entries.map((entry, index) =>
index === 1
? createMessageEntry({ ...excludedMessage, excludeFromContext: false }, index)
: entry,
);
expect(prepareBranchEntries(visibleEntries, 100).messages).toMatchObject([
{ role: "user", content: "continue branch" },
]);
const result = await generateBranchSummary(entries, {
model,
apiKey: "test-key",
signal: new AbortController().signal,
streamFn: capture.streamFn,
});
const result = await generateBranchSummary(entries, {
model,
apiKey: "test-key",
signal: new AbortController().signal,
streamFn: capture.streamFn,
});
expect(result.ok).toBe(true);
expect(capture.readCapture().prompt).toContain("important original request");
expect(capture.readCapture().prompt).toContain("continue branch");
expect(capture.readCapture().prompt).not.toContain("private command");
expect(capture.readCapture().prompt).not.toContain("private output marker");
expect(JSON.stringify(entries)).toContain("private output marker");
});
expect(result.ok).toBe(true);
expect(capture.readCapture().prompt).toContain("important original request");
expect(capture.readCapture().prompt).toContain("continue branch");
expect(capture.readCapture().prompt).not.toContain("private command");
expect(capture.readCapture().prompt).not.toContain("private output marker");
expect(JSON.stringify(entries)).toContain("private output marker");
},
);
it("summarizes tool failures without exposing private result details", async () => {
const model = createModel(128_000);
@@ -283,24 +283,44 @@ describe("calculateContextTokens", () => {
});
describe("session-entry compaction budgeting", () => {
it("counts visible shell output while ignoring private output after provider usage", () => {
const hidden = createBashMessage("x".repeat(80_000), 2, true);
const visible = createBashMessage("x".repeat(80_000), 2, false);
const assistant = createAssistant("done", createUsage(42), 1);
const latest: AgentMessage = { role: "user", content: "continue", timestamp: 3 };
it.each([
{
kind: "shell",
createMessage: (excludeFromContext: boolean) =>
createBashMessage("x".repeat(80_000), 2, excludeFromContext),
},
{
kind: "custom",
createMessage: (excludeFromContext: boolean): AgentMessage => ({
role: "custom",
customType: "openclaw.operator-activity",
content: "x".repeat(80_004),
display: excludeFromContext,
excludeFromContext,
timestamp: 2,
}),
},
])(
"counts visible $kind activity while ignoring excluded activity after provider usage",
({ createMessage }) => {
const hidden = createMessage(true);
const visible = createMessage(false);
const assistant = createAssistant("done", createUsage(42), 1);
const latest: AgentMessage = { role: "user", content: "continue", timestamp: 3 };
expect(estimateTokens(hidden)).toBe(0);
expect(estimateTokens(visible)).toBeGreaterThan(20_000);
expect(estimateContextTokens([assistant, hidden, latest])).toMatchObject({
tokens: 44,
usageTokens: 42,
trailingTokens: 2,
lastUsageIndex: 0,
});
expect(estimateContextTokens([assistant, visible, latest]).trailingTokens).toBeGreaterThan(
20_000,
);
});
expect(estimateTokens(hidden)).toBe(0);
expect(estimateTokens(visible)).toBeGreaterThan(20_000);
expect(estimateContextTokens([assistant, hidden, latest])).toMatchObject({
tokens: 44,
usageTokens: 42,
trailingTokens: 2,
lastUsageIndex: 0,
});
expect(estimateContextTokens([assistant, visible, latest]).trailingTokens).toBeGreaterThan(
20_000,
);
},
);
it("never rewinds a retained visible turn onto an excluded shell-history row", () => {
const entries: SessionTreeEntry[] = [
@@ -294,6 +294,9 @@ function countContentBlockChars(
/** Estimate token count for one message using a conservative character heuristic. */
export function estimateTokens(message: AgentMessage): number {
if ("excludeFromContext" in message && message.excludeFromContext === true) {
return 0;
}
let chars = 0;
const harnessMessage = message as HarnessMessage;
@@ -334,9 +337,6 @@ export function estimateTokens(message: AgentMessage): number {
return Math.ceil(chars / CHARS_PER_TOKEN_ESTIMATE);
}
case "bashExecution": {
if (harnessMessage.excludeFromContext === true) {
return 0;
}
chars =
estimateStringChars(harnessMessage.command) + estimateStringChars(harnessMessage.output);
return Math.ceil(chars / CHARS_PER_TOKEN_ESTIMATE);
@@ -137,13 +137,57 @@ describe("buildSessionContext", () => {
const activity = {
role: "custom" as const,
customType: "openclaw.context-compaction",
content: "Context compacted",
content: `Context compacted ${"x".repeat(80_000)}`,
display: true,
excludeFromContext: true,
timestamp: Date.parse(timestamp),
};
const runtimeContext = {
role: "custom" as const,
customType: "openclaw.runtime-context",
content: "Model-visible runtime context",
display: false,
details: { runtimeContextCarrier: true },
timestamp: Date.parse(timestamp),
};
const activityEntry: SessionTreeEntry = {
type: "message",
id: "activity",
parentId: "initial",
timestamp,
message: activity,
};
const runtimeContextEntry: SessionTreeEntry = {
type: "message",
id: "runtime-context",
parentId: "activity",
timestamp,
message: runtimeContext,
};
const entries = [
userEntry("initial", null, "original request"),
activityEntry,
runtimeContextEntry,
userEntry("latest", "runtime-context", "continue"),
];
const messages = buildSessionContext(entries).messages;
expect(convertToLlm([activity])).toEqual([]);
expect(projectSessionEntryMessage(activityEntry)).toBeUndefined();
expect(projectSessionEntryMessage(runtimeContextEntry)).toBe(runtimeContext);
expect(messages.map((message) => message.role)).toEqual(["user", "custom", "user"]);
expect(JSON.stringify(messages)).toContain("Model-visible runtime context");
expect(JSON.stringify(messages)).not.toContain("Context compacted");
expect(convertToLlm(messages)).toMatchObject([
{ role: "user", content: "original request" },
{
role: "user",
content: [{ type: "text", text: "Model-visible runtime context" }],
runtimeContextCarrier: true,
},
{ role: "user", content: "continue" },
]);
expect(JSON.stringify(entries)).toContain("Context compacted");
});
it("keeps private shell executions in history without projecting them into context", () => {
@@ -16,8 +16,8 @@ const SESSION_HISTORY_PRELUDE = Symbol.for("openclaw.sessionHistoryPrelude");
export function projectSessionEntryMessage(entry: SessionTreeEntry): AgentMessage | undefined {
switch (entry.type) {
case "message":
// Private shell history stays persisted but never enters replay or summarization.
return entry.message.role === "bashExecution" && entry.message.excludeFromContext === true
// Display-only history stays persisted but never enters replay or summarization.
return "excludeFromContext" in entry.message && entry.message.excludeFromContext === true
? undefined
: entry.message;
case "custom_message":
@@ -40,6 +40,34 @@ describe("compaction real conversation classification", () => {
},
);
it.each([
{ excludeFromContext: true, expected: false },
{ excludeFromContext: false, expected: true },
])(
"classifies custom conversation according to context eligibility ($excludeFromContext)",
({ excludeFromContext, expected }) => {
const custom = {
role: "custom",
customType: "display-note",
content: "A visible administrative event.",
display: true,
excludeFromContext,
timestamp: 1,
} satisfies AgentMessage;
const toolResult = {
role: "toolResult",
toolCallId: "call-1",
toolName: "exec",
content: [{ type: "text", text: "audit output" }],
} as AgentMessage;
const messages = [custom, toolResult];
expect(hasMeaningfulConversationContent(custom)).toBe(expected);
expect(isRealConversationMessage(custom, messages, 0)).toBe(expected);
expect(isRealConversationMessage(toolResult, messages, 1)).toBe(expected);
},
);
it("rejects tool-call-only messages and orphan tool results", () => {
const toolCall = {
role: "assistant",
+3 -4
View File
@@ -30,6 +30,9 @@ function isSummaryRole(role: unknown): boolean {
/** Returns whether a message has content worth preserving as conversation. */
export function hasMeaningfulConversationContent(message: AgentMessage): boolean {
if ("excludeFromContext" in message && message.excludeFromContext === true) {
return false;
}
if ((message as { role?: unknown }).role === "custom") {
const custom = message as { content?: unknown; display?: unknown };
return custom.display !== false && hasMeaningfulMessageContent(custom.content);
@@ -38,11 +41,7 @@ export function hasMeaningfulConversationContent(message: AgentMessage): boolean
const bash = message as {
command?: unknown;
output?: unknown;
excludeFromContext?: unknown;
};
if (bash.excludeFromContext === true) {
return false;
}
const command = typeof bash.command === "string" ? bash.command : "";
const output = typeof bash.output === "string" ? bash.output : "";
return hasMeaningfulText(`${command}\n${output}`);
@@ -417,6 +417,9 @@ describe("runEmbeddedAttemptSettledPhase", () => {
content: expect.stringMatching(/1.*image contents.*unavailable.*resend.*not claim/is),
}),
);
expect(fixture.sessionManager.appendMessage.mock.calls[0]?.[0]).not.toHaveProperty(
"excludeFromContext",
);
expect(mocks.completeResult).toHaveBeenCalledWith(
expect.objectContaining({
state: expect.objectContaining({
@@ -232,6 +232,29 @@ describe("compaction-timeout helpers", () => {
}),
expectedLength: 1,
},
{
name: "excluded custom tail",
tail: castAgentMessage({
role: "custom",
customType: "display-note",
content: "display-only activity",
display: true,
excludeFromContext: true,
timestamp: 2,
}),
expectedLength: 1,
},
{
name: "undisplayed model-visible custom tail",
tail: castAgentMessage({
role: "custom",
customType: "openclaw-runtime-context",
content: "runtime context",
display: false,
timestamp: 2,
}),
expectedLength: 2,
},
{
name: "tool result tail",
tail: castAgentMessage({
@@ -51,15 +51,17 @@ type SnapshotSelection = {
};
export function canContinueFromMessage(message: AgentMessage | undefined): boolean {
switch (message?.role) {
if (!message || ("excludeFromContext" in message && message.excludeFromContext === true)) {
return false;
}
switch (message.role) {
case "user":
case "toolResult":
case "branchSummary":
case "compactionSummary":
case "custom":
return true;
case "bashExecution":
return message.excludeFromContext !== true;
return true;
default:
return false;
}
@@ -61,13 +61,46 @@ describe("preemptive precheck counts bashExecution and summary turns", () => {
expect(precheckTokens).toBeGreaterThanOrEqual(realProviderTokens);
});
it("drops a bash turn excluded from context", () => {
const excluded = {
...(bashExecMessage() as unknown as Record<string, unknown>),
excludeFromContext: true,
} as unknown as AgentMessage;
it.each([
{
role: "bashExecution",
message: { ...bashExecMessage(), excludeFromContext: true } as AgentMessage,
},
{
role: "custom",
message: {
role: "custom",
customType: "display-note",
content: BIG_OUTPUT,
display: true,
excludeFromContext: true,
timestamp: 1,
} satisfies AgentMessage,
},
])("drops excluded $role turns from token pressure", ({ message }) => {
expect(estimateLlmBoundaryTokenPressure({ messages: [message], prompt: "" })).toBeLessThan(50);
expect(
shouldPreemptivelyCompactBeforePrompt({
messages: [message],
prompt: "continue",
contextTokenBudget: 128000,
reserveTokens: 16384,
}).route,
).toBe("fits");
});
expect(estimateLlmBoundaryTokenPressure({ messages: [excluded], prompt: "" })).toBeLessThan(50);
it("counts undisplayed custom context when it remains model-visible", () => {
const runtimeContext = {
role: "custom",
customType: "openclaw-runtime-context",
content: BIG_OUTPUT,
display: false,
timestamp: 1,
} satisfies AgentMessage;
expect(
estimateLlmBoundaryTokenPressure({ messages: [runtimeContext], prompt: "" }),
).toBeGreaterThan(50000);
});
it("routes an oversized bash transcript to compaction", () => {
@@ -156,6 +156,9 @@ function estimateContentTokenPressure(
}
function estimateMessageTokenPressure(message: AgentMessage): number {
if ("excludeFromContext" in message && message.excludeFromContext === true) {
return 0;
}
// Provider replay can carry legacy aliases outside the canonical AgentMessage union.
const legacy: Record<string, unknown> = isRecord(message) ? message : {};
let tokens = MESSAGE_BOUNDARY_OVERHEAD_TOKENS;
@@ -169,9 +172,6 @@ function estimateMessageTokenPressure(message: AgentMessage): number {
}
if (message.role === "bashExecution") {
if (message.excludeFromContext === true) {
return 0;
}
const bashMessage: BashExecutionMessage = message;
tokens += estimateStringTokenPressure(bashExecutionToText(bashMessage));
return tokens;
@@ -98,21 +98,34 @@ describe("tool-result-char-estimator", () => {
expect(chars).toBeGreaterThan(500_000);
});
it("returns 0 for bashExecution with excludeFromContext", () => {
const msg = {
it.each([
{
role: "bashExecution",
command: "npm run build",
output: "huge output ".repeat(50000),
exitCode: 0,
cancelled: false,
truncated: false,
excludeFromContext: true,
timestamp: 1,
} as unknown as AgentMessage;
message: {
role: "bashExecution",
command: "npm run build",
output: "huge output ".repeat(50000),
exitCode: 0,
cancelled: false,
truncated: false,
excludeFromContext: true,
timestamp: 1,
},
},
{
role: "custom",
message: {
role: "custom",
customType: "display-note",
content: "huge output ".repeat(50000),
display: true,
excludeFromContext: true,
timestamp: 1,
},
},
])("returns 0 for excluded $role messages", ({ message }) => {
const cache = createMessageCharEstimateCache();
const chars = estimateMessageCharsCached(msg, cache);
expect(chars).toBe(0);
expect(estimateMessageCharsCached(message as AgentMessage, cache)).toBe(0);
});
it("estimates compactionSummary with prefix/suffix", () => {
@@ -145,13 +158,13 @@ describe("tool-result-char-estimator", () => {
expect(chars).toBeGreaterThan(256);
});
it("estimates custom message with string content", () => {
it.each([true, false])("estimates custom message with display=%s", (display) => {
const text = "custom data ".repeat(5000);
const msg = {
role: "custom",
customType: "test",
content: text,
display: true,
display,
timestamp: 1,
} as unknown as AgentMessage;
@@ -105,7 +105,11 @@ export function getToolResultText(msg: AgentMessage): string {
}
function estimateMessageChars(msg: AgentMessage): number {
if (!msg || typeof msg !== "object") {
if (
!msg ||
typeof msg !== "object" ||
("excludeFromContext" in msg && msg.excludeFromContext === true)
) {
return 0;
}
@@ -161,9 +165,6 @@ function estimateMessageChars(msg: AgentMessage): number {
const role: unknown = Reflect.get(msg, "role");
if (role === "bashExecution") {
if (Reflect.get(msg, "excludeFromContext") === true) {
return 0;
}
return bashExecutionToText(msg as Parameters<typeof bashExecutionToText>[0]).length;
}
+8
View File
@@ -533,6 +533,14 @@ describe("sessions tool", () => {
}),
}),
);
expect(events).not.toContainEqual(
expect.objectContaining({
message: expect.objectContaining({
customType: "openclaw.system-note",
excludeFromContext: expect.anything(),
}),
}),
);
});
});
@@ -1,32 +0,0 @@
import { SessionManager } from "../../agents/sessions/session-manager.js";
import type { SessionEntry } from "../../config/sessions.js";
import type { OpenClawConfig } from "../../config/types.openclaw.js";
export async function appendSessionAudit(params: {
cfg: OpenClawConfig;
target: {
agentId: string;
entry: Pick<SessionEntry, "sessionId">;
sessionKey: string;
storePath: string;
};
text: string;
now: number;
}): Promise<void> {
const identity = {
agentId: params.target.agentId,
sessionId: params.target.entry.sessionId,
storePath: params.target.storePath,
};
SessionManager.appendMessageToTranscript(
{ ...identity, sessionKey: params.target.sessionKey },
{
role: "custom",
customType: "openclaw.system-note",
content: `System note: ${params.text}`,
display: true,
timestamp: params.now,
},
{ config: params.cfg },
);
}
@@ -1,5 +1,4 @@
import { afterEach, describe, expect, it, vi } from "vitest";
import { SessionManager } from "../../agents/sessions/session-manager.js";
import {
loadSessionEntry,
loadTranscriptEvents,
@@ -9,7 +8,6 @@ import {
import {
addSessionMember,
listSessionMembers,
removeSessionMember,
} from "../../config/sessions/session-sharing-store.js";
import {
closeOpenClawAgentDatabasesForTest,
@@ -107,7 +105,11 @@ function context(
}
async function call(
method: "session.visibility.set" | "session.members.list" | "session.members.add",
method:
| "session.visibility.set"
| "session.members.list"
| "session.members.add"
| "session.members.remove",
params: Record<string, unknown>,
requestContext: GatewayRequestContext,
requestClient: GatewayClient = soloClient(),
@@ -709,7 +711,7 @@ describe("session sharing handlers", () => {
});
});
it("persists visibility and membership changes as transcript system notes", async () => {
it("publishes canonical visibility and membership changes without changing the transcript", async () => {
await withOpenClawTestState({ scenario: "minimal" }, async () => {
const sessionKey = "agent:main:main";
await upsertSessionEntryCore(
@@ -718,6 +720,11 @@ describe("session sharing handlers", () => {
);
const broadcast = vi.fn();
const requestContext = context(broadcast);
const transcriptBefore = await loadTranscriptEvents({
agentId: "main",
sessionId: "session-main",
sessionKey,
});
expect(
await call(
@@ -739,32 +746,51 @@ describe("session sharing handlers", () => {
expect.objectContaining({ identityId: "local-operator", addedBy: "local-operator" }),
]);
const events = await loadTranscriptEvents({
agentId: "main",
sessionId: "session-main",
sessionKey,
});
expect(events).toEqual(
expect.arrayContaining([
expect.objectContaining({
message: expect.objectContaining({
customType: "openclaw.system-note",
content: expect.stringContaining("changed session visibility"),
}),
expect(
await call(
"session.members.remove",
{ sessionKey, identityId: "local-operator" },
requestContext,
),
).toEqual([[true, { ok: true, sessionKey, identityId: "local-operator" }, undefined]]);
expect(listSessionMembers({ agentId: "main", sessionKey })).toEqual([]);
expect(
await loadTranscriptEvents({
agentId: "main",
sessionId: "session-main",
sessionKey,
}),
).toEqual(transcriptBefore);
const sharingEvents = broadcast.mock.calls
.filter(([event]) => event === "session.sharing")
.map(([, payload, options]) => ({ payload, options }));
expect(sharingEvents).toEqual([
{
payload: expect.objectContaining({
action: "visibility",
sessionKey,
visibility: "read-only",
}),
expect.objectContaining({
message: expect.objectContaining({
customType: "openclaw.system-note",
content: expect.stringContaining("added local-operator"),
}),
options: { sessionKeys: [sessionKey] },
},
{
payload: expect.objectContaining({
action: "member-added",
sessionKey,
identityId: "local-operator",
}),
]),
);
expect(broadcast).toHaveBeenCalledWith(
"session.sharing",
expect.objectContaining({ sessionKey }),
{ sessionKeys: [sessionKey] },
);
options: { sessionKeys: [sessionKey] },
},
{
payload: expect.objectContaining({
action: "member-removed",
sessionKey,
identityId: "local-operator",
}),
options: { sessionKeys: [sessionKey] },
},
]);
const restrictedKey = "agent:main:restricted";
await upsertSessionEntryCore(
@@ -821,38 +847,6 @@ describe("session sharing handlers", () => {
agentId: "main",
}),
).toBe(false);
await patchSessionEntryCore({ agentId: "main", sessionKey }, () => ({
visibility: "shared",
}));
const append = vi
.spyOn(SessionManager, "appendMessageToTranscript")
.mockImplementationOnce(() => {
throw new Error("audit unavailable");
});
const concurrent = await Promise.allSettled([
call("session.visibility.set", { sessionKey, visibility: "read-only" }, requestContext),
call("session.visibility.set", { sessionKey, visibility: "draft" }, requestContext),
]);
append.mockRestore();
expect(concurrent.map((result) => result.status)).toEqual(["rejected", "fulfilled"]);
expect(loadSessionEntry({ agentId: "main", sessionKey })?.visibility).toBe("draft");
removeSessionMember({ agentId: "main", sessionKey }, "local-operator");
const memberAppend = vi
.spyOn(SessionManager, "appendMessageToTranscript")
.mockImplementationOnce(() => {
throw new Error("audit unavailable");
});
const concurrentAdds = await Promise.allSettled([
call("session.members.add", { sessionKey, identityId: "local-operator" }, requestContext),
call("session.members.add", { sessionKey, identityId: "local-operator" }, requestContext),
]);
memberAppend.mockRestore();
expect(concurrentAdds.map((result) => result.status)).toEqual(["rejected", "fulfilled"]);
expect(listSessionMembers({ agentId: "main", sessionKey })).toEqual([
expect.objectContaining({ identityId: "local-operator" }),
]);
});
});
});
@@ -30,7 +30,6 @@ import {
resolveSessionVisibility,
} from "../session-sharing.js";
import { gatewayClientSessionCreator } from "./gateway-client-identity.js";
import { appendSessionAudit } from "./session-audit.js";
import { emitSessionsChanged } from "./session-change-event.js";
import type { GatewayClient, GatewayRequestContext, GatewayRequestHandlers } from "./types.js";
import { assertValidParams } from "./validation.js";
@@ -236,28 +235,8 @@ export const sessionSharingHandlers: GatewayRequestHandlers = {
if (sessionChanged) {
throw new Error("session changed before sharing mutation");
}
invalidateSessionSharingSnapshot(current.canonicalKey);
const now = Date.now();
const actor = actorIdentity(client);
try {
await appendSessionAudit({
cfg,
target: { ...current, sessionKey: current.storeKey },
text: `${actor.label ?? actor.id} changed session visibility from ${previous} to ${visibility}.`,
now,
});
} catch (error) {
// Roll back only the exact instance and value we patched; an unexpected
// storage-owner replacement must not inherit the old visibility.
await patchSessionEntryCore(scope, (entry) =>
entry.sessionId === current.entry.sessionId &&
resolveSessionVisibility(entry) === visibility
? { visibility: previous }
: null,
);
invalidateSessionSharingSnapshot(current.canonicalKey);
throw error;
}
publishSharingChange({
context,
agentId: current.agentId,
@@ -370,17 +349,6 @@ export const sessionSharingHandlers: GatewayRequestHandlers = {
if (!added.inserted) {
return;
}
try {
await appendSessionAudit({
cfg,
target: { ...current, sessionKey: current.storeKey },
text: `${actor.label ?? actor.id} added ${params.identityId} as a session member.`,
now,
});
} catch (error) {
removeSessionMember(scope, params.identityId, added.member, current.entry.sessionId);
throw error;
}
publishSharingChange({
context,
agentId: current.agentId,
@@ -441,22 +409,6 @@ export const sessionSharingHandlers: GatewayRequestHandlers = {
}
const now = Date.now();
const actor = actorIdentity(client);
try {
await appendSessionAudit({
cfg,
target: { ...current, sessionKey: current.storeKey },
text: `${actor.label ?? actor.id} removed ${params.identityId} from session members.`,
now,
});
} catch (error) {
addSessionMember(scope, {
identityId: removed.identityId,
addedBy: removed.addedBy,
addedAt: removed.addedAt,
expectedSessionId: current.entry.sessionId,
});
throw error;
}
publishSharingChange({
context,
agentId: current.agentId,
@@ -1,7 +1,6 @@
import { vi } from "vitest";
const mocks = vi.hoisted(() => ({
appendSessionAudit: vi.fn(async () => undefined),
handleChatSend: vi.fn(),
suggestionMutationFailure: undefined as
| "claim"
@@ -13,7 +12,6 @@ const mocks = vi.hoisted(() => ({
}));
vi.mock("./chat-send-handler.js", () => ({ handleChatSend: mocks.handleChatSend }));
vi.mock("./session-audit.js", () => ({ appendSessionAudit: mocks.appendSessionAudit }));
vi.mock("../../infra/system-presence.js", () => ({
listSystemPresence: () => mocks.presence,
}));
@@ -82,13 +82,11 @@ export function responseSuggestionId(result: Awaited<ReturnType<typeof call>>):
}
export function registerSessionSuggestionTestLifecycle(mocks: {
appendSessionAudit: ReturnType<typeof vi.fn>;
handleChatSend: ReturnType<typeof vi.fn>;
suggestionMutationFailure?: string;
presence: unknown[];
}): void {
beforeEach(() => {
mocks.appendSessionAudit.mockClear();
mocks.handleChatSend.mockReset();
mocks.handleChatSend.mockImplementation(({ respond }: { respond: RespondFn }) => {
respond(true, { runId: "suggestion-run", status: "started" });
@@ -1,13 +1,19 @@
import { describe, expect, it, vi } from "vitest";
import { createDeferred } from "../../../test/helpers/promise.js";
import { upsertSessionEntryCore } from "../../config/sessions/session-accessor.js";
import { SessionManager } from "../../agents/sessions/session-manager.js";
import {
readSessionTranscriptMessageEvents,
upsertSessionEntryCore,
} from "../../config/sessions/session-accessor.js";
import { addSessionMember } from "../../config/sessions/session-sharing-store.js";
import {
addSessionSuggestion,
listSessionSuggestions,
SESSION_SUGGESTION_DISPATCH_CLAIM_TTL_MS,
} from "../../config/sessions/session-suggestion-store.js";
import { buildPersistedUserTurnMessage } from "../../sessions/user-turn-transcript.js";
import { withOpenClawTestState } from "../../test-utils/openclaw-test-state.js";
import { resolveSessionSharingTarget } from "../session-sharing.js";
import { getSessionSuggestionTestMocks } from "./sessions-suggestions.test-mocks.js";
import {
call,
@@ -136,7 +142,9 @@ describe("session suggestion handlers", () => {
expect.objectContaining({ action: "added" }),
expect.objectContaining({ sessionKeys: [sessionKey, "main"] }),
);
expect(mocks.appendSessionAudit).not.toHaveBeenCalled();
expect(
readSessionTranscriptMessageEvents({ agentId: "main", sessionId: "session-main" }),
).toEqual([]);
await call(
"session.suggestions.add",
@@ -461,44 +469,102 @@ describe("session suggestion handlers", () => {
);
expect(owner.responses[0]?.[0]).toBe(true);
expect(mocks.handleChatSend).not.toHaveBeenCalled();
expect(mocks.appendSessionAudit).toHaveBeenCalledWith(
expect.objectContaining({ text: "Owner moved a suggestion into the composer." }),
);
expect(mocks.appendSessionAudit).not.toHaveBeenCalledWith(
expect.objectContaining({ text: expect.stringContaining("forged") }),
);
expect(
readSessionTranscriptMessageEvents({ agentId: "main", sessionId: "session-main" }),
).toEqual([]);
});
});
it("publishes a fenced resolution before awaiting the transcript audit", async () => {
await withOpenClawTestState({ scenario: "minimal" }, async () => {
await upsertDefaultSuggestionSession();
const added = await call(
"session.suggestions.add",
{ sessionKey, text: "resolve before audit" },
client("alice", "Alice"),
);
const audit = createDeferred<undefined>();
mocks.appendSessionAudit.mockImplementationOnce(() => audit.promise);
const broadcast = vi.fn();
const pending = call(
"session.suggestions.resolve",
{ sessionKey, id: responseSuggestionId(added), resolution: "edit" },
client("owner", "Owner"),
context(broadcast),
);
it.each([
["send", "accepted", true],
["queue", "accepted", true],
["edit", "accepted", false],
["dismiss", "dismissed", false],
] as const)(
"finalizes and publishes %s without administrative transcript narration",
async (resolution, state, dispatchesConversation) => {
await withOpenClawTestState({ scenario: "minimal" }, async () => {
await upsertDefaultSuggestionSession();
const added = await call(
"session.suggestions.add",
{ sessionKey, text: "Ship the focused change" },
client("alice", "Alice"),
);
const id = responseSuggestionId(added);
const broadcast = vi.fn();
const transcriptScope = { agentId: "main", sessionId: "session-main" };
const target = resolveSessionSharingTarget({ cfg: {}, sessionKey, agentId: "main" });
if (!target) {
throw new Error("Default suggestion session target was not found");
}
expect(readSessionTranscriptMessageEvents(transcriptScope)).toEqual([]);
await vi.waitFor(() => expect(mocks.appendSessionAudit).toHaveBeenCalledOnce());
expect(broadcast).toHaveBeenCalledWith(
"session.suggestion",
expect.objectContaining({ action: "resolved" }),
expect.any(Object),
);
if (dispatchesConversation) {
mocks.handleChatSend.mockImplementationOnce(
({
params,
client: attributedClient,
respond,
}: {
params: { message: string; idempotencyKey: string };
client: { internal?: { senderAttribution?: { id?: string; name?: string } } };
respond: RespondFn;
}) => {
SessionManager.appendMessageToTranscript(
{ ...transcriptScope, sessionKey, storePath: target.storePath },
buildPersistedUserTurnMessage({
text: params.message,
idempotencyKey: params.idempotencyKey,
sender: attributedClient.internal?.senderAttribution,
}),
);
respond(true, { runId: "suggestion-run", status: "started" });
},
);
}
audit.resolve(undefined);
expect((await pending).responses[0]?.[0]).toBe(true);
});
});
const resolved = await call(
"session.suggestions.resolve",
{ sessionKey, id, resolution },
client("owner", "Owner"),
context(broadcast),
);
expect(resolved.responses[0]).toMatchObject([
true,
{ suggestion: { id, state, text: "Ship the focused change" } },
]);
expect(listSessionSuggestions({ agentId: "main", sessionKey })).toMatchObject([
{ id, state, text: "Ship the focused change" },
]);
expect(broadcast).toHaveBeenCalledWith(
"session.suggestion",
expect.objectContaining({
action: "resolved",
suggestion: expect.objectContaining({ id, state }),
}),
expect.objectContaining({ sessionKeys: [sessionKey] }),
);
const events = readSessionTranscriptMessageEvents(transcriptScope);
if (dispatchesConversation) {
expect(events).toHaveLength(1);
expect(events[0]?.event).toMatchObject({
message: {
role: "user",
content: "Ship the focused change",
idempotencyKey: `session-suggestion:${id}`,
__openclaw: { senderId: "alice", senderName: "Suggested by Alice" },
},
});
expect(mocks.handleChatSend).toHaveBeenCalledOnce();
} else {
expect(events).toEqual([]);
expect(mocks.handleChatSend).not.toHaveBeenCalled();
}
});
},
);
it("keeps typing dormant for one identity and broadcasts for two live viewers", async () => {
await withOpenClawTestState({ scenario: "minimal" }, async () => {
@@ -7,7 +7,6 @@ import {
validateSessionTypingParams,
type SessionSuggestion,
type SessionSuggestionResolution,
type SessionSharingIdentity,
type SessionTypingEvent,
} from "../../../packages/gateway-protocol/src/index.js";
import {
@@ -33,7 +32,6 @@ import {
import { resolveSessionSubscriptionKeys as subscriptionKeys } from "../session-subscription-keys.js";
import { handleChatSend } from "./chat-send-handler.js";
import { gatewayClientSessionCreator } from "./gateway-client-identity.js";
import { appendSessionAudit } from "./session-audit.js";
import {
broadcastTypingThrottled,
liveViewerIdentities,
@@ -115,30 +113,6 @@ function runSessionSuggestionMutation<T>(params: {
}
}
function resolutionAuditAction(resolution: SessionSuggestionResolution): string {
switch (resolution) {
case "send":
return "sent a suggestion immediately";
case "queue":
return "queued a suggestion";
case "edit":
return "moved a suggestion into the composer";
case "dismiss":
return "dismissed a suggestion";
}
throw new Error(`unsupported suggestion resolution: ${String(resolution)}`);
}
function actorIdentity(client: GatewayClient | null): SessionSharingIdentity {
return (
gatewayClientSessionCreator(client) ?? {
type: "system",
id: "operator.admin",
label: "Administrator",
}
);
}
function attributedSuggestionClient(
client: GatewayClient,
suggestion: StoredSessionSuggestion,
@@ -518,17 +492,6 @@ export const sessionSuggestionHandlers: GatewayRequestHandlers = {
action: "resolved",
suggestion: projected,
});
const actor = actorIdentity(client);
try {
await appendSessionAudit({
cfg: context.getRuntimeConfig(),
target: { ...target, sessionKey: target.canonicalKey },
text: `${actor.label ?? actor.id} ${resolutionAuditAction(resolution)}.`,
now: Date.now(),
});
} catch (error) {
context.logGateway.warn(`failed to append suggestion resolution audit: ${String(error)}`);
}
respond(true, { suggestion: projected });
},
@@ -30,27 +30,6 @@ import {
import { registerWorkerInferenceSessionDrain } from "./worker-environments/inference-control-internal.js";
import type { WorkerSessionPlacementRecord } from "./worker-environments/placement-record.js";
const sessionAuditGate = vi.hoisted(() => ({
entered: vi.fn(),
wait: undefined as Promise<void> | undefined,
}));
vi.mock("./server-methods/session-audit.js", async () => {
const actual = await vi.importActual<typeof import("./server-methods/session-audit.js")>(
"./server-methods/session-audit.js",
);
return {
...actual,
appendSessionAudit: async (...args: Parameters<typeof actual.appendSessionAudit>) => {
if (sessionAuditGate.wait) {
sessionAuditGate.entered();
await sessionAuditGate.wait;
}
await actual.appendSessionAudit(...args);
},
};
});
const {
createConfiguredGlobalAgentSessionStore,
createSessionStoreDir,
@@ -419,28 +398,39 @@ test("sharing revocation fences archive before cancellation and forces fresh aut
throw new Error("expected resolved sharing target");
}
const releaseAudit = createDeferredCore();
sessionAuditGate.entered.mockClear();
sessionAuditGate.wait = releaseAudit.promise;
const sharingCommitted = createDeferredCore();
const releaseSharingMutation = createDeferredCore();
let sharing: Promise<LifecycleHandlerResponse> | undefined;
let archive: Promise<LifecycleHandlerResponse> | undefined;
try {
let sharingSettled = false;
sharing = invokeVisibilityHandler({
client: owner,
context: requestContext,
sessionKey,
visibility: "draft",
sharing = runExclusiveSessionLifecycleMutation({
scope: sharingTarget.storePath,
identities: [
sharingTarget.canonicalKey,
sharingTarget.storeKey,
...sharingTarget.storeKeys,
sharingTarget.entry.sessionId,
],
run: async () => {
const response = await invokeVisibilityHandler({
client: owner,
context: requestContext,
sessionKey,
visibility: "draft",
});
sharingCommitted.resolve();
await releaseSharingMutation.promise;
return response;
},
}).finally(() => {
sharingSettled = true;
});
await vi.waitFor(() => {
expect(sessionAuditGate.entered).toHaveBeenCalledOnce();
expect(
isSessionLifecycleMutationActive(sharingTarget.storePath, [sessionKey, sessionId]),
).toBe(true);
});
await sharingCommitted.promise;
expect(isSessionLifecycleMutationActive(sharingTarget.storePath, [sessionKey, sessionId])).toBe(
true,
);
expect(sharingSettled).toBe(false);
expect(loadSessionEntry({ storePath, sessionKey })?.visibility).toBe("draft");
@@ -462,7 +452,7 @@ test("sharing revocation fences archive before cancellation and forces fresh aut
expect(reclaim).not.toHaveBeenCalled();
expect(loadSessionEntry({ storePath, sessionKey })?.archivedAt).toBeUndefined();
releaseAudit.resolve();
releaseSharingMutation.resolve();
expect(await sharing).toMatchObject({ ok: true });
expect(loadSessionEntry({ storePath, sessionKey })?.visibility).toBe("draft");
@@ -473,10 +463,10 @@ test("sharing revocation fences archive before cancellation and forces fresh aut
expect(interrupted).toBe(false);
expect(active.controller.signal.aborted).toBe(false);
expectNoSessionQueueCleanup();
expect(reclaim).not.toHaveBeenCalled();
expect(loadSessionEntry({ storePath, sessionKey })?.archivedAt).toBeUndefined();
} finally {
sessionAuditGate.wait = undefined;
releaseAudit.resolve();
releaseSharingMutation.resolve();
admission.release();
await Promise.allSettled([...(sharing ? [sharing] : []), ...(archive ? [archive] : [])]);
active.unsubscribe();