Agents: address inferred commitments review (#74189)

This commit is contained in:
Vignesh Natarajan
2026-04-29 00:42:21 -07:00
committed by Vignesh
parent 8e4035d09a
commit 95bf450dc9
7 changed files with 131 additions and 30 deletions

View File

@@ -0,0 +1,83 @@
import { beforeEach, describe, expect, it, vi } from "vitest";
import type { CommitmentRecord } from "../commitments/types.js";
import type { RuntimeEnv } from "../runtime.js";
import { commitmentsListCommand } from "./commitments.js";
const mocks = vi.hoisted(() => ({
listCommitments: vi.fn(),
markCommitmentsStatus: vi.fn(),
resolveCommitmentStorePath: vi.fn(() => "/tmp/openclaw-commitments.json"),
loadConfig: vi.fn(() => ({
commitments: {
store: "/tmp/openclaw-commitments.json",
},
})),
}));
vi.mock("../commitments/store.js", () => ({
listCommitments: mocks.listCommitments,
markCommitmentsStatus: mocks.markCommitmentsStatus,
resolveCommitmentStorePath: mocks.resolveCommitmentStorePath,
}));
vi.mock("../config/config.js", () => ({
loadConfig: mocks.loadConfig,
}));
function createRuntime(): { runtime: RuntimeEnv; logs: string[] } {
const logs: string[] = [];
return {
logs,
runtime: {
log: (message: unknown) => logs.push(String(message)),
error: vi.fn(),
exit: vi.fn(),
},
};
}
function commitment(overrides?: Partial<CommitmentRecord>): CommitmentRecord {
return {
id: "cm_escape",
agentId: "main\u001b[31m",
sessionKey: "agent:main:session\u001b]8;;https://example.test\u0007",
channel: "telegram",
to: "+15551234567\u001b[0m",
kind: "event_check_in",
sensitivity: "routine",
source: "inferred_user_context",
status: "pending",
reason: "The user mentioned an interview.",
suggestedText: "How did it go?\u001b]52;c;YWJj\u0007\nspoofed",
dedupeKey: "interview:2026-04-30",
confidence: 0.91,
dueWindow: {
earliestMs: Date.parse("2026-04-30T17:00:00.000Z"),
latestMs: Date.parse("2026-04-30T23:00:00.000Z"),
timezone: "America/Los_Angeles",
},
sourceUserText: "I have an interview tomorrow.",
createdAtMs: Date.parse("2026-04-29T16:00:00.000Z"),
updatedAtMs: Date.parse("2026-04-29T16:00:00.000Z"),
attempts: 0,
...overrides,
};
}
describe("commitments command", () => {
beforeEach(() => {
vi.clearAllMocks();
mocks.listCommitments.mockResolvedValue([commitment()]);
});
it("sanitizes untrusted commitment fields in table output", async () => {
const { runtime, logs } = createRuntime();
await commitmentsListCommand({}, runtime);
const output = logs.join("\n");
expect(output).not.toContain("\u001b");
expect(output).not.toContain("\u0007");
expect(output).toContain("\\nspoofed");
});
});

View File

@@ -8,6 +8,7 @@ import { loadConfig } from "../config/config.js";
import { info } from "../globals.js";
import type { RuntimeEnv } from "../runtime.js";
import { normalizeOptionalString } from "../shared/string-coerce.js";
import { sanitizeTerminalText } from "../terminal/safe-text.js";
import { isRich, theme } from "../terminal/theme.js";
const STATUS_VALUES = new Set<CommitmentStatus>([
@@ -22,6 +23,10 @@ function truncate(value: string, maxChars: number): string {
return value.length <= maxChars ? value : `${value.slice(0, maxChars - 1)}...`;
}
function safe(value: string): string {
return sanitizeTerminalText(value);
}
function parseStatus(raw: string | undefined, runtime: RuntimeEnv): CommitmentStatus | undefined {
const status = normalizeOptionalString(raw);
if (!status) {
@@ -55,19 +60,23 @@ function formatRows(commitments: CommitmentRecord[], rich: boolean): string[] {
const lines = [rich ? theme.heading(header) : header];
for (const commitment of commitments) {
const scope = truncate(
[commitment.agentId, commitment.channel, commitment.to ?? commitment.sessionKey]
[
safe(commitment.agentId),
safe(commitment.channel),
safe(commitment.to ?? commitment.sessionKey),
]
.filter(Boolean)
.join("/"),
28,
);
lines.push(
[
truncate(commitment.id, 16).padEnd(16),
commitment.status.padEnd(10),
commitment.kind.padEnd(16),
truncate(safe(commitment.id), 16).padEnd(16),
safe(commitment.status).padEnd(10),
safe(commitment.kind).padEnd(16),
formatDue(commitment.dueWindow.earliestMs).padEnd(24),
scope.padEnd(28),
truncate(commitment.suggestedText, 90),
truncate(safe(commitment.suggestedText), 90),
].join(" "),
);
}

View File

@@ -4,6 +4,7 @@ import path from "node:path";
import { afterEach, describe, expect, it } from "vitest";
import type { OpenClawConfig } from "../config/config.js";
import {
buildCommitmentExtractionPrompt,
parseCommitmentExtractionOutput,
persistCommitmentExtractionResult,
validateCommitmentCandidates,
@@ -77,6 +78,30 @@ describe("commitment extraction", () => {
});
});
it("omits routing scope identifiers from extractor prompts", () => {
const prompt = buildCommitmentExtractionPrompt({
items: [
item({
itemId: "public-item-1",
agentId: "agent-secret",
sessionKey: "session-secret",
channel: "channel-secret",
accountId: "account-secret",
to: "+15551234567",
threadId: "thread-secret",
}),
],
});
expect(prompt).toContain("public-item-1");
expect(prompt).not.toContain("agent-secret");
expect(prompt).not.toContain("session-secret");
expect(prompt).not.toContain("channel-secret");
expect(prompt).not.toContain("account-secret");
expect(prompt).not.toContain("+15551234567");
expect(prompt).not.toContain("thread-secret");
});
it("rejects disabled, low-confidence, and non-future candidates", () => {
const cfg: OpenClawConfig = {
commitments: {

View File

@@ -204,14 +204,6 @@ export function buildCommitmentExtractionPrompt(params: {
itemId: item.itemId,
now: new Date(item.nowMs).toISOString(),
timezone: item.timezone,
scope: {
agentId: item.agentId,
sessionKey: item.sessionKey,
channel: item.channel,
accountId: item.accountId,
to: item.to,
threadId: item.threadId,
},
latestUserMessage: item.userText,
assistantResponse: item.assistantText ?? "",
existingPendingCommitments: formatExistingPending(item),

View File

@@ -109,7 +109,12 @@ describe("commitment extraction runtime", () => {
const store = await loadCommitmentStore(cfg.commitments?.store);
expect(extractBatch).toHaveBeenCalledTimes(1);
expect(extractBatch.mock.calls[0]?.[0].items).toHaveLength(2);
const batchItems = extractBatch.mock.calls[0]?.[0].items;
expect(batchItems).toHaveLength(2);
expect(batchItems?.[0]?.itemId).not.toContain("main");
expect(batchItems?.[0]?.itemId).not.toContain("telegram");
expect(batchItems?.[0]?.itemId).not.toContain("15551234567");
expect(batchItems?.[0]?.itemId).not.toContain("m1");
expect(store.commitments.map((commitment) => commitment.dedupeKey)).toEqual([
"event:1",
"event:2",

View File

@@ -88,17 +88,8 @@ export function resetCommitmentExtractionRuntimeForTests(): void {
}
function buildItemId(params: CommitmentExtractionEnqueueInput, nowMs: number): string {
const source = normalizeOptionalString(params.sourceMessageId) ?? randomUUID();
return [
params.agentId,
params.sessionKey,
params.channel,
params.accountId ?? "",
params.to ?? "",
params.threadId ?? "",
source,
nowMs,
].join(":");
const source = normalizeOptionalString(params.sourceMessageId) ? "message" : "turn";
return `${source}:${nowMs.toString(36)}:${randomUUID()}`;
}
function isUsefulText(value: string | undefined): boolean {

View File

@@ -265,11 +265,7 @@ export async function upsertInferredCommitments(params: {
store.commitments.push(record);
created.push(record);
}
if (created.length > 0) {
await saveCommitmentStore(resolved.store, store);
} else {
await saveCommitmentStore(resolved.store, store);
}
await saveCommitmentStore(resolved.store, store);
return created;
}