fix: invalid tool call ID recovery and system-reminder tag centralization (#627)
Co-authored-by: Letta <noreply@letta.com>
This commit is contained in:
@@ -3,6 +3,7 @@ import type { Message } from "@letta-ai/letta-client/resources/agents/messages";
|
||||
import {
|
||||
isApprovalPendingError,
|
||||
isApprovalStateDesyncError,
|
||||
isInvalidToolCallIdsError,
|
||||
} from "../agent/approval-recovery";
|
||||
import { extractApprovals } from "../agent/check-approval";
|
||||
|
||||
@@ -31,6 +32,21 @@ describe("isApprovalStateDesyncError", () => {
|
||||
expect(isApprovalStateDesyncError(detail)).toBe(true);
|
||||
});
|
||||
|
||||
test("detects invalid tool call IDs error", () => {
|
||||
const detail =
|
||||
"Invalid tool call IDs: Expected ['tc_abc123'], got ['tc_xyz789']";
|
||||
expect(isApprovalStateDesyncError(detail)).toBe(true);
|
||||
});
|
||||
|
||||
test("detects invalid tool call IDs error case-insensitively", () => {
|
||||
expect(
|
||||
isApprovalStateDesyncError("INVALID TOOL CALL IDS: Expected X, got Y"),
|
||||
).toBe(true);
|
||||
expect(isApprovalStateDesyncError("invalid tool call ids: mismatch")).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
test("returns false for unrelated errors", () => {
|
||||
expect(isApprovalStateDesyncError("Connection timeout")).toBe(false);
|
||||
expect(isApprovalStateDesyncError("Internal server error")).toBe(false);
|
||||
@@ -44,6 +60,43 @@ describe("isApprovalStateDesyncError", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("isInvalidToolCallIdsError", () => {
|
||||
test("detects invalid tool call IDs error", () => {
|
||||
const detail =
|
||||
"Invalid tool call IDs: Expected ['tc_abc123'], got ['tc_xyz789']";
|
||||
expect(isInvalidToolCallIdsError(detail)).toBe(true);
|
||||
});
|
||||
|
||||
test("detects invalid tool call IDs error case-insensitively", () => {
|
||||
expect(
|
||||
isInvalidToolCallIdsError("INVALID TOOL CALL IDS: Expected X, got Y"),
|
||||
).toBe(true);
|
||||
expect(isInvalidToolCallIdsError("invalid tool call ids: mismatch")).toBe(
|
||||
true,
|
||||
);
|
||||
});
|
||||
|
||||
test("returns false for 'no tool call awaiting' error", () => {
|
||||
// This is a different desync type - server has NO pending approvals
|
||||
expect(
|
||||
isInvalidToolCallIdsError("No tool call is currently awaiting approval"),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
test("returns false for unrelated errors", () => {
|
||||
expect(isInvalidToolCallIdsError("Connection timeout")).toBe(false);
|
||||
expect(isInvalidToolCallIdsError("Internal server error")).toBe(false);
|
||||
expect(isInvalidToolCallIdsError("Rate limit exceeded")).toBe(false);
|
||||
});
|
||||
|
||||
test("returns false for non-string input", () => {
|
||||
expect(isInvalidToolCallIdsError(null)).toBe(false);
|
||||
expect(isInvalidToolCallIdsError(undefined)).toBe(false);
|
||||
expect(isInvalidToolCallIdsError(123)).toBe(false);
|
||||
expect(isInvalidToolCallIdsError({ error: "test" })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("isApprovalPendingError", () => {
|
||||
// This is the actual error format from the Letta backend (screenshot from LET-7101)
|
||||
const REAL_ERROR_DETAIL =
|
||||
|
||||
@@ -4,6 +4,7 @@
|
||||
|
||||
import { afterEach, beforeEach, describe, expect, mock, test } from "bun:test";
|
||||
import type Letta from "@letta-ai/letta-client";
|
||||
import { SYSTEM_REMINDER_OPEN } from "../../constants";
|
||||
import { continueConversation } from "../../skills/builtin/messaging-agents/scripts/continue-conversation";
|
||||
import { startConversation } from "../../skills/builtin/messaging-agents/scripts/start-conversation";
|
||||
|
||||
@@ -98,7 +99,7 @@ describe("start-conversation", () => {
|
||||
|
||||
// Check message was sent with system reminder
|
||||
expect(mockMessageCreate).toHaveBeenCalledWith(mockConversation.id, {
|
||||
input: expect.stringContaining("<system-reminder>"),
|
||||
input: expect.stringContaining(SYSTEM_REMINDER_OPEN),
|
||||
});
|
||||
expect(mockMessageCreate).toHaveBeenCalledWith(mockConversation.id, {
|
||||
input: expect.stringContaining("Hello!"),
|
||||
@@ -217,7 +218,7 @@ describe("continue-conversation", () => {
|
||||
|
||||
// Check message was sent with system reminder
|
||||
expect(mockMessageCreate).toHaveBeenCalledWith(mockConversation.id, {
|
||||
input: expect.stringContaining("<system-reminder>"),
|
||||
input: expect.stringContaining(SYSTEM_REMINDER_OPEN),
|
||||
});
|
||||
expect(mockMessageCreate).toHaveBeenCalledWith(mockConversation.id, {
|
||||
input: expect.stringContaining("Follow-up question"),
|
||||
|
||||
@@ -1,4 +1,5 @@
|
||||
import { afterEach, describe, expect, test } from "bun:test";
|
||||
import { SYSTEM_REMINDER_OPEN } from "../../constants";
|
||||
import { read } from "../../tools/impl/Read";
|
||||
import { TestDirectory } from "../helpers/testFs";
|
||||
|
||||
@@ -115,7 +116,7 @@ export default box;
|
||||
|
||||
const result = await read({ file_path: file });
|
||||
|
||||
expect(result.content).toContain("<system-reminder>");
|
||||
expect(result.content).toContain(SYSTEM_REMINDER_OPEN);
|
||||
expect(result.content).toContain("empty contents");
|
||||
});
|
||||
|
||||
@@ -125,7 +126,7 @@ export default box;
|
||||
|
||||
const result = await read({ file_path: file });
|
||||
|
||||
expect(result.content).toContain("<system-reminder>");
|
||||
expect(result.content).toContain(SYSTEM_REMINDER_OPEN);
|
||||
expect(result.content).toContain("empty contents");
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user