fix: treat empty-string optional integer tool params as unset

snotty · snotty · commit 86f91748a868 · 2026-07-05T18:18:18.000+08:00
Optional positive-integer tool params (e.g. Telegram replyTo/threadId)
threw ToolInputError when a tool-calling model populated them with an
empty-string or whitespace-only default. Those defaults carry no value,
so readPositiveIntegerParam/readNonNegativeIntegerParam now treat a
blank string as unset (undefined) instead of throwing, while still
rejecting genuinely invalid present values (0, "42.5", "-3"). This
prevents silent message-delivery failures when models emit empty
routing-param defaults. Adds unit tests covering blank vs invalid.
diff --git a/src/agents/tools/common.params.test.ts b/src/agents/tools/common.params.test.ts
@@ -132,6 +132,37 @@ describe("readNumberParam", () => {
     ).toThrow("deleteDays must be an integer from 0 to 7");
   });
 
+  it("treats empty or whitespace-only strings as unset for optional positive integer params", () => {
+    // Tool-calling models routinely emit empty-string defaults for optional
+    // params (e.g. Telegram replyTo/threadId) they are not actually setting.
+    // An empty/whitespace string carries no value and must not throw.
+    expect(readPositiveIntegerParam({ replyTo: "" }, "replyTo")).toBeUndefined();
+    expect(readPositiveIntegerParam({ threadId: "   " }, "threadId")).toBeUndefined();
+    expect(readPositiveIntegerParam({ replyTo: "\t\n" }, "replyTo")).toBeUndefined();
+    // Genuinely invalid present values must still throw.
+    expect(() => readPositiveIntegerParam({ replyTo: "0" }, "replyTo")).toThrow(
+      "replyTo must be a positive integer",
+    );
+    expect(() => readPositiveIntegerParam({ replyTo: 0 }, "replyTo")).toThrow(
+      "replyTo must be a positive integer",
+    );
+    expect(() => readPositiveIntegerParam({ replyTo: "-3" }, "replyTo")).toThrow(
+      "replyTo must be a positive integer",
+    );
+  });
+
+  it("treats empty or whitespace-only strings as unset for optional non-negative integer params", () => {
+    expect(readNonNegativeIntegerParam({ position: "" }, "position")).toBeUndefined();
+    expect(readNonNegativeIntegerParam({ position: "  " }, "position")).toBeUndefined();
+    // A present, valid zero is still a real value.
+    expect(readNonNegativeIntegerParam({ position: "0" }, "position")).toBe(0);
+    expect(readNonNegativeIntegerParam({ position: 0 }, "position")).toBe(0);
+    // Genuinely invalid present values must still throw.
+    expect(() => readNonNegativeIntegerParam({ position: "4.5" }, "position")).toThrow(
+      "position must be a non-negative integer",
+    );
+  });
+
   it("throws for invalid present bounded finite number params", () => {
     expect(readFiniteNumberParam({ quality: "0.75" }, "quality")).toBe(0.75);
     expect(readFiniteNumberParam({ quality: null }, "quality")).toBeUndefined();
diff --git a/src/agents/tools/common.ts b/src/agents/tools/common.ts
@@ -112,6 +112,19 @@ function readParamRaw(params: Record<string, unknown>, key: string): unknown {
   return readSnakeCaseParamRaw(params, key);
 }
 
+/**
+ * True when a raw tool-param value should be treated as "not provided".
+ *
+ * Tool-calling models frequently populate every optional field with an empty
+ * or whitespace-only string default even when the caller did not set it. Such
+ * a value carries no information and must not be treated as an invalid present
+ * value. This mirrors how {@link readStringParam} / {@link readNumberParam}
+ * already ignore empty/whitespace strings.
+ */
+function isBlankParamValue(raw: unknown): boolean {
+  return typeof raw === "string" && raw.trim() === "";
+}
+
 export function readStringParam(
   params: Record<string, unknown>,
   key: string,
@@ -244,8 +257,11 @@ export function readPositiveIntegerParam(
     positiveInteger: true,
     strict: true,
   });
-  if (value === undefined && readParamRaw(params, key) != null) {
-    throw new ToolInputError(options.message ?? `${key} must be a positive integer`);
+  if (value === undefined) {
+    const raw = readParamRaw(params, key);
+    if (raw != null && !isBlankParamValue(raw)) {
+      throw new ToolInputError(options.message ?? `${key} must be a positive integer`);
+    }
   }
   if (value !== undefined && options.max !== undefined && value > options.max) {
     throw new ToolInputError(options.message ?? `${key} must be a positive integer`);
@@ -265,8 +281,11 @@ export function readNonNegativeIntegerParam(
     nonNegativeInteger: true,
     strict: true,
   });
-  if (value === undefined && readParamRaw(params, key) != null) {
-    throw new ToolInputError(options.message ?? `${key} must be a non-negative integer`);
+  if (value === undefined) {
+    const raw = readParamRaw(params, key);
+    if (raw != null && !isBlankParamValue(raw)) {
+      throw new ToolInputError(options.message ?? `${key} must be a non-negative integer`);
+    }
   }
   if (value !== undefined && options.max !== undefined && value > options.max) {
     throw new ToolInputError(options.message ?? `${key} must be a non-negative integer`);
