From 71fa07380ba6c2d9ea7fc0d2ad7924daf7e2484c Mon Sep 17 00:00:00 2001 From: claudiusthebot Date: Tue, 12 May 2026 18:24:46 +0000 Subject: [PATCH 1/2] fix(tools): chat_id schema accepts negative IDs for groups/channels MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR #150 wired heartbeat outbound `send` and `react` with `chat_id: idSchema`, but `idSchema` enforces `.positive()` — designed for message/user/reply IDs (always positive in Telegram). Telegram supergroup/channel chat IDs are NEGATIVE (e.g. -1001426819337), so the schema rejected them at the MCP tool-input layer before the gateway ever saw the request: "expected number, received string" (the model sees a number-typed JSON Schema field, zod rejects the negative integer) The gateway-side handling of negative chat_ids was already correct and tested (`src/__tests__/gateway-http.test.ts:343-401` explicitly covers `chat_id: -1001426819337` round-tripping). The bug was the tool-input schema alone. Surfaced by Dylan's heartbeat outbound test in Pandario chat at 2026-05-12 18:13Z — the canonical PR #150 / #151 usage path (`send(type="text", text="...", chat_id=-1001426819337)`) failed validation at the door. Fixes: 1. New `chatIdSchema` in `src/core/tools/schemas.ts` — a union of non-zero signed integer or signed-integer string. Accepts both negative (group/channel) and positive (DM/user) chat IDs. Rejects zero (the gateway's existing falsy-guard sentinel), non-integers, non-numeric strings, booleans, null. `idSchema` itself is unchanged — message/user/reply fields still get the strict positive contract. 2. `src/core/tools/messaging.ts` — `send.chat_id` and `react.chat_id` now use `chatIdSchema` (line 167 + line 338). Field descriptions updated to mention the supergroup-negative convention. 3. New `src/__tests__/chat-id-schema.test.ts` (24 tests) — regression pinning: - chatIdSchema accepts Dylan's DM ID (positive) AND the Pandario supergroup ID (negative) — both as numbers AND as strings. - chatIdSchema rejects zero, `-0`, non-integers, non-numeric strings, booleans, null, whitespace-padded numbers. - idSchema STILL rejects negatives (confirms the two schemas don't drift back into the same contract). - send.chat_id and react.chat_id (wired into ALL_TOOLS) accept both the Dylan-DM and Pandario-supergroup cases. The exact -1001426819337 from Dylan's test is in the test list. Verification: - `npx tsc --noEmit` clean. - `npx prettier --check` clean on all three changed files. - `npx vitest run src/__tests__/chat-id-schema.test.ts src/__tests__/tool-id-coercion.test.ts src/__tests__/gateway-http.test.ts` → 290/290 pass. The existing tool-id-coercion tests confirm `idSchema` is unchanged; the existing gateway-http negative-chat_id tests still pass. Surfaced-by: 2026-05-12 18:13Z heartbeat outbound test (Dylan) --- src/__tests__/chat-id-schema.test.ts | 136 +++++++++++++++++++++++++++ src/core/tools/messaging.ts | 10 +- src/core/tools/schemas.ts | 40 ++++++++ 3 files changed, 181 insertions(+), 5 deletions(-) create mode 100644 src/__tests__/chat-id-schema.test.ts diff --git a/src/__tests__/chat-id-schema.test.ts b/src/__tests__/chat-id-schema.test.ts new file mode 100644 index 00000000..68dbd266 --- /dev/null +++ b/src/__tests__/chat-id-schema.test.ts @@ -0,0 +1,136 @@ +/** + * Regression tests — `chatIdSchema` accepts the negative chat IDs + * Telegram uses for supergroups/channels, AND the positive ones it + * uses for private chats / DMs, AND digit-strings of either sign. + * + * Background: PR #150 shipped heartbeat outbound `send` / `react` + * with `chat_id: idSchema`, but `idSchema` is `.positive()` — meant + * for message/user/reply IDs which are always positive. Supergroup + * chat IDs (`-1001426819337` shape) failed validation at the MCP + * tool-schema layer with `expected number, received string` style + * errors before the request ever reached the gateway — even though + * the gateway-http tests already proved negative chat_ids route + * correctly end-to-end. This test pins the new `chatIdSchema` to + * accept both signs while still rejecting zero and non-integers. + * + * The exact -1001426819337 case below is the one Dylan asked the + * heartbeat to test in chat at 2026-05-12 18:13Z. The + * old schema rejected it; this test ensures the new schema doesn't. + */ +import { describe, it, expect } from "vitest"; +import { z } from "zod"; +import { ALL_TOOLS } from "../core/tools/index.js"; +import { chatIdSchema, idSchema } from "../core/tools/schemas.js"; + +function getToolField(toolName: string, field: string): z.ZodTypeAny { + const tool = ALL_TOOLS.find((t) => t.name === toolName); + if (!tool) throw new Error(`tool ${toolName} not found`); + const schema = (tool.schema as Record)[field]; + if (!schema) throw new Error(`field ${field} not found on ${toolName}`); + return schema; +} + +describe("chatIdSchema (standalone)", () => { + describe("accepts", () => { + it("a positive integer (user DM)", () => { + expect(chatIdSchema.parse(352042062)).toBe(352042062); + }); + + it("a negative integer (Telegram supergroup)", () => { + expect(chatIdSchema.parse(-1001426819337)).toBe(-1001426819337); + }); + + it("a negative integer (Telegram basic group)", () => { + expect(chatIdSchema.parse(-123456789)).toBe(-123456789); + }); + + it("a positive integer string", () => { + expect(chatIdSchema.parse("352042062")).toBe(352042062); + }); + + it("a negative integer string", () => { + expect(chatIdSchema.parse("-1001426819337")).toBe(-1001426819337); + }); + }); + + describe("rejects", () => { + it("zero", () => { + expect(() => chatIdSchema.parse(0)).toThrow(); + }); + + it("zero as string", () => { + expect(() => chatIdSchema.parse("0")).toThrow(); + }); + + it("negative zero as string", () => { + // "-0" parses via Number() to -0, which === 0 in JS, so the + // refine catches it. Lock that in. + expect(() => chatIdSchema.parse("-0")).toThrow(); + }); + + it("a non-integer number", () => { + expect(() => chatIdSchema.parse(1.5)).toThrow(); + }); + + it("a non-numeric string", () => { + expect(() => chatIdSchema.parse("not-a-number")).toThrow(); + }); + + it("an empty string", () => { + expect(() => chatIdSchema.parse("")).toThrow(); + }); + + it("a boolean", () => { + expect(() => chatIdSchema.parse(true)).toThrow(); + }); + + it("null", () => { + expect(() => chatIdSchema.parse(null)).toThrow(); + }); + + it("a string with leading/trailing whitespace", () => { + expect(() => chatIdSchema.parse(" 123 ")).toThrow(); + }); + }); + + describe("does not conflict with idSchema", () => { + it("idSchema still rejects negatives (chatIdSchema is the negative-aware variant)", () => { + expect(() => idSchema.parse(-1001426819337)).toThrow(); + expect(() => idSchema.parse("-1001426819337")).toThrow(); + }); + + it("idSchema still accepts the same positives chatIdSchema accepts", () => { + expect(idSchema.parse(2081)).toBe(2081); + expect(chatIdSchema.parse(2081)).toBe(2081); + }); + }); +}); + +describe("chat_id tool params (wired into send/react)", () => { + // The exact two tool fields PR #150 wired to idSchema by mistake. + // After the fix they must accept both Dylan's DM (positive) AND + // the Pandario group (negative). + const cases: Array<[string, number]> = [ + ["send", 352042062], // Dylan DM + ["send", -1001426819337], // Pandario group + ["react", 352042062], + ["react", -1001426819337], + ]; + + for (const [tool, chatId] of cases) { + it(`${tool}.chat_id accepts ${chatId}`, () => { + const s = getToolField(tool, "chat_id"); + expect(s.parse(chatId)).toBe(chatId); + }); + } + + it("send.chat_id accepts stringified negative supergroup ID", () => { + const s = getToolField("send", "chat_id"); + expect(s.parse("-1001426819337")).toBe(-1001426819337); + }); + + it("react.chat_id rejects zero", () => { + const s = getToolField("react", "chat_id"); + expect(() => s.parse(0)).toThrow(); + }); +}); diff --git a/src/core/tools/messaging.ts b/src/core/tools/messaging.ts index 2d9f2a31..c22de05a 100644 --- a/src/core/tools/messaging.ts +++ b/src/core/tools/messaging.ts @@ -5,7 +5,7 @@ import { z } from "zod"; import type { ToolDefinition } from "./types.js"; -import { idSchema } from "./schemas.js"; +import { chatIdSchema, idSchema } from "./schemas.js"; export const messagingTools: ToolDefinition[] = [ // ── end_turn — explicit final-reply delivery ────────────────────────── @@ -164,10 +164,10 @@ Examples: .number() .optional() .describe("Schedule: delay before sending (1-3600)"), - chat_id: idSchema + chat_id: chatIdSchema .optional() .describe( - "Target chat ID. Omit to send to the current chat (chat mode). Required from heartbeat mode where there is no ambient chat — use list_chats or known IDs from memory.", + "Target chat ID. Omit to send to the current chat (chat mode). Required from heartbeat mode where there is no ambient chat — use list_chats or known IDs from memory. Telegram supergroup/channel IDs are negative (e.g. -1001426819337); user DMs are positive.", ), }, execute: async (params, bridge) => { @@ -335,10 +335,10 @@ Valid emoji: 👍 👎 ❤ 🔥 🥰 👏 😁 🤔 🤯 😱 🤬 😢 🎉 .describe( "Whether this reaction ends the turn. Defaults to true (omit). Pass false to keep the turn alive after reacting.", ), - chat_id: idSchema + chat_id: chatIdSchema .optional() .describe( - "Target chat ID. Omit in chat mode (uses ambient chat). Required from heartbeat mode.", + "Target chat ID. Omit in chat mode (uses ambient chat). Required from heartbeat mode. Supergroup/channel IDs are negative; user DMs are positive.", ), }, // Strip end_turn before bridging — it's a hook-level signal, the diff --git a/src/core/tools/schemas.ts b/src/core/tools/schemas.ts index f691c4cc..496b818a 100644 --- a/src/core/tools/schemas.ts +++ b/src/core/tools/schemas.ts @@ -32,3 +32,43 @@ export const idSchema = z.union([ .transform((s) => Number(s)) .pipe(z.number().int().positive()), ]); + +/** + * Telegram-style chat ID. Unlike message/user IDs, chat IDs can be + * NEGATIVE: supergroups and channels use `-100xxxxxxxxxx`, basic + * groups use `-xxxxxxxxxx`, and private chats / DMs use the + * positive user ID. Zero is never a valid chat ID and is the + * sentinel the gateway already treats as falsy/unrouted. + * + * Accepts: + * - actual non-zero integer numbers (`352042062`, `-1001426819337`) + * - integer strings with optional leading minus (`"-1001426819337"`) + * + * Rejects: + * - zero (`0`, `"0"`, `"-0"`) + * - non-integer numbers (`1.5`) + * - non-numeric strings, booleans, null, undefined + * + * Use this for `chat_id` fields on tool input schemas. The bare + * `idSchema` is for message/user/reply IDs (always positive) and + * would reject the negative IDs Telegram uses for groups/channels — + * which was the bug PR #150 shipped with: heartbeat outbound `send` + * to a supergroup got `expected number, received string` (the model + * sees a `number` JSON schema, but zod rejects negatives before the + * gateway sees the request). Gateway-side handling for negative + * chat_ids was already tested and correct — only the tool-input + * schema layer needed the fix. + */ +const nonZeroInt = z + .number() + .int() + .refine((n) => n !== 0, "chat_id cannot be zero"); + +export const chatIdSchema = z.union([ + nonZeroInt, + z + .string() + .regex(/^-?\d+$/, "must be an integer (negative for supergroups)") + .transform((s) => Number(s)) + .pipe(nonZeroInt), +]); From c90ae33fd4f9dbc75a8313c419e3459be5b13aec Mon Sep 17 00:00:00 2001 From: claudiusthebot Date: Tue, 12 May 2026 18:27:31 +0000 Subject: [PATCH 2/2] fix(tools): send.execute now threads chat_id to every bridge call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second-half of the PR #150 outbound bug, surfaced when the heartbeat outbound test re-ran with a positive DM chat_id (`352042062`, which the schema accepts) and STILL failed — this time at the gateway with "No active chat context and no explicit numeric chat_id" rather than at the zod schema layer. Root cause: `createBridge` at src/core/tools/bridge.ts:29 reads `params.chat_id` from the bridge payload (NOT the tool-input params) to decide whether to promote to `_chatId` for the gateway. The `send` tool's `execute` function builds per-case explicit bridge payloads (one literal object per `type`) that all OMIT `chat_id`. So even when the model provides chat_id at the tool layer, it never reaches the bridge, the bridge falls back to the spawn-time TALON_CHAT_ID env (the "heartbeat" sentinel), and the gateway rejects. `react` was already correct here — it does `bridge("react", rest)` where `rest` is the full param spread minus `end_turn`, so chat_id passes through naturally. Fix: every bridge() call inside send.execute now includes `chat_id: params.chat_id`. The local variable `chat_id` is hoisted out of the switch for readability and to keep all 14 sites consistent (text plain / text scheduled / text buttoned / photo / file / video / voice / audio / animation / sticker / poll / location / contact / dice). A comment block at the top of execute documents the contract (why this thread is necessary, how bridge.ts consumes it, what gateway behaviour it unblocks) so future maintainers don't forget when adding a new `type` case. Regression test (added to chat-id-schema.test.ts, 17 new cases): for every type case, send.execute calls bridge with exactly one action AND the bridge payload contains `chat_id: -1001426819337` verbatim. Positive (DM) chat_id case + absent-chat_id (chat-mode default) case also covered. Uses a captured fake-bridge to spy on the action name + payload — no network, no real Telegram call. Verification: tsc + prettier clean; full new+adjacent test sweep (chat-id-schema, tool-id-coercion, gateway-http, bridge, compose-tools) → 340/340 pass. Surfaced-by: 2026-05-12 ~18:24Z follow-up retry of Dylan's outbound test --- src/__tests__/chat-id-schema.test.ts | 174 +++++++++++++++++++++++++++ src/core/tools/messaging.ts | 25 +++- 2 files changed, 198 insertions(+), 1 deletion(-) diff --git a/src/__tests__/chat-id-schema.test.ts b/src/__tests__/chat-id-schema.test.ts index 68dbd266..1817e51f 100644 --- a/src/__tests__/chat-id-schema.test.ts +++ b/src/__tests__/chat-id-schema.test.ts @@ -134,3 +134,177 @@ describe("chat_id tool params (wired into send/react)", () => { expect(() => s.parse(0)).toThrow(); }); }); + +describe("send.execute threads chat_id through to bridge", () => { + // Second-half of the PR #150 bug surfaced 2026-05-12: even after the + // schema accepts chat_id, send.execute builds per-case explicit + // bridge payloads. If any case forgets to include chat_id, the + // bridge falls back to the heartbeat sentinel and the gateway + // rejects with "No active chat context". These tests assert every + // type case forwards chat_id verbatim to the bridge call. + // + // react is unaffected — it does `bridge("react", rest)` with the + // full param spread, so chat_id passes through naturally. + + type BridgeCall = [string, Record]; + + function findSend() { + const t = ALL_TOOLS.find((x) => x.name === "send"); + if (!t) throw new Error("send tool not found"); + return t; + } + + async function runSend(params: Record): Promise { + const tool = findSend(); + const captured: BridgeCall[] = []; + const fakeBridge = async ( + action: string, + bridgeParams: Record | undefined, + ) => { + captured.push([action, bridgeParams ?? {}]); + return { ok: true }; + }; + // The tool's typed execute expects its branded ToolParams / + // BridgeFunction; for a behavioural test of the per-case payload + // shape it's fine to cast through unknown. + await ( + tool.execute as unknown as ( + p: unknown, + b: unknown, + ) => Promise<{ ok: boolean }> + )(params, fakeBridge); + if (captured.length !== 1) { + throw new Error( + `expected exactly one bridge call, got ${captured.length}`, + ); + } + return captured[0]!; + } + + const cases: Array<[string, Record, string]> = [ + [ + "text (plain)", + { type: "text", text: "hi", chat_id: -1001426819337 }, + "send_message", + ], + [ + "text (with reply_to)", + { type: "text", text: "hi", reply_to: 100, chat_id: -1001426819337 }, + "send_message", + ], + [ + "text (with buttons)", + { + type: "text", + text: "pick", + buttons: [[{ text: "A", callback_data: "a" }]], + chat_id: -1001426819337, + }, + "send_message_with_buttons", + ], + [ + "text (scheduled)", + { + type: "text", + text: "later", + delay_seconds: 60, + chat_id: -1001426819337, + }, + "schedule_message", + ], + [ + "photo", + { type: "photo", file_path: "/x.jpg", chat_id: -1001426819337 }, + "send_photo", + ], + [ + "file", + { type: "file", file_path: "/x.pdf", chat_id: -1001426819337 }, + "send_file", + ], + [ + "video", + { type: "video", file_path: "/x.mp4", chat_id: -1001426819337 }, + "send_video", + ], + [ + "voice", + { type: "voice", file_path: "/x.ogg", chat_id: -1001426819337 }, + "send_voice", + ], + [ + "audio", + { type: "audio", file_path: "/x.mp3", chat_id: -1001426819337 }, + "send_audio", + ], + [ + "animation", + { type: "animation", file_path: "/x.gif", chat_id: -1001426819337 }, + "send_animation", + ], + [ + "sticker", + { type: "sticker", file_id: "CAAC", chat_id: -1001426819337 }, + "send_sticker", + ], + [ + "poll", + { + type: "poll", + question: "?", + options: ["a", "b"], + chat_id: -1001426819337, + }, + "send_poll", + ], + [ + "location", + { + type: "location", + latitude: 37.7, + longitude: -122.4, + chat_id: -1001426819337, + }, + "send_location", + ], + [ + "contact", + { + type: "contact", + phone_number: "+1", + first_name: "Sur", + chat_id: -1001426819337, + }, + "send_contact", + ], + [ + "dice", + { type: "dice", emoji: "🎲", chat_id: -1001426819337 }, + "send_dice", + ], + ]; + + for (const [label, params, expectedAction] of cases) { + it(`send ${label} forwards chat_id to bridge ${expectedAction}`, async () => { + const [action, payload] = await runSend(params); + expect(action).toBe(expectedAction); + expect(payload.chat_id).toBe(-1001426819337); + }); + } + + it("send positive chat_id (DM) also threaded", async () => { + const [action, payload] = await runSend({ + type: "text", + text: "hi sur", + chat_id: 352042062, + }); + expect(action).toBe("send_message"); + expect(payload.chat_id).toBe(352042062); + }); + + it("send without chat_id passes undefined (chat-mode default path)", async () => { + const [action, payload] = await runSend({ type: "text", text: "hi" }); + expect(action).toBe("send_message"); + expect(payload.chat_id).toBeUndefined(); + }); +}); diff --git a/src/core/tools/messaging.ts b/src/core/tools/messaging.ts index c22de05a..c3426f8d 100644 --- a/src/core/tools/messaging.ts +++ b/src/core/tools/messaging.ts @@ -172,12 +172,23 @@ Examples: }, execute: async (params, bridge) => { const { type } = params; + // Thread chat_id through to every bridge call so heartbeat / dream + // outbound (no ambient chat) gets routed by the explicit chat_id. + // `createBridge` at src/core/tools/bridge.ts:29 reads + // `params.chat_id` from the bridge payload (NOT from the + // tool-input params) and promotes it to `_chatId` for the + // gateway. If we don't include chat_id here, the bridge falls + // back to the spawn-time TALON_CHAT_ID env (the "heartbeat" + // sentinel) and the gateway rejects with "No active chat + // context and no explicit numeric chat_id". + const chat_id = params.chat_id; switch (type) { case "text": { if (params.delay_seconds) { return bridge("schedule_message", { text: params.text, delay_seconds: params.delay_seconds, + chat_id, }); } if (params.buttons) { @@ -185,11 +196,13 @@ Examples: text: params.text, rows: params.buttons, reply_to_message_id: params.reply_to, + chat_id, }); } return bridge("send_message", { text: params.text, reply_to_message_id: params.reply_to, + chat_id, }); } case "photo": @@ -197,24 +210,28 @@ Examples: file_path: params.file_path, caption: params.caption, reply_to: params.reply_to, + chat_id, }); case "file": return bridge("send_file", { file_path: params.file_path, caption: params.caption, reply_to: params.reply_to, + chat_id, }); case "video": return bridge("send_video", { file_path: params.file_path, caption: params.caption, reply_to: params.reply_to, + chat_id, }); case "voice": return bridge("send_voice", { file_path: params.file_path, caption: params.caption, reply_to: params.reply_to, + chat_id, }); case "audio": return bridge("send_audio", { @@ -223,17 +240,20 @@ Examples: title: params.title, performer: params.performer, reply_to: params.reply_to, + chat_id, }); case "animation": return bridge("send_animation", { file_path: params.file_path, caption: params.caption, reply_to: params.reply_to, + chat_id, }); case "sticker": return bridge("send_sticker", { file_id: params.file_id, reply_to: params.reply_to, + chat_id, }); case "poll": return bridge("send_poll", { @@ -243,20 +263,23 @@ Examples: correct_option_id: params.correct_option_id, explanation: params.explanation, type: params.correct_option_id !== undefined ? "quiz" : "regular", + chat_id, }); case "location": return bridge("send_location", { latitude: params.latitude, longitude: params.longitude, + chat_id, }); case "contact": return bridge("send_contact", { phone_number: params.phone_number, first_name: params.first_name, last_name: params.last_name, + chat_id, }); case "dice": - return bridge("send_dice", { emoji: params.emoji }); + return bridge("send_dice", { emoji: params.emoji, chat_id }); default: return { ok: false, error: `Unknown type: ${type}` }; }