Skip to content

fix(tools): chat_id schema accepts negative IDs (groups/channels) - #153

Merged
dylanneve1 merged 2 commits into
mainfrom
fix/chat-id-schema-allow-negative
May 12, 2026
Merged

fix(tools): chat_id schema accepts negative IDs (groups/channels)#153
dylanneve1 merged 2 commits into
mainfrom
fix/chat-id-schema-allow-negative

Conversation

@claudiusthebot

@claudiusthebot claudiusthebot commented May 12, 2026

Copy link
Copy Markdown
Collaborator

Summary

PR #150 shipped heartbeat outbound send / react with two bugs that compound β€” surfaced together by the canonical PR #150 / #151 test path Dylan asked the next heartbeat to exercise (send(chat_id=-1001426819337) to Pandario at 2026-05-12 18:13Z):

  1. Schema rejects negatives. chat_id: idSchema, but idSchema enforces .positive() β€” designed for message/user/reply IDs. Telegram supergroup/channel chat IDs are negative. First call failed at the zod layer with expected number, received string.
  2. send.execute strips chat_id. Even after the schema accepts it (e.g. for a positive DM chat_id like 352042062), send.execute builds per-case explicit bridge payloads that all OMIT chat_id. createBridge reads params.chat_id from the bridge payload, not the tool-input params, to promote to _chatId β€” so chat_id never reaches the gateway and the gateway rejects with "No active chat context and no explicit numeric chat_id".

Both ends of the chain β€” gateway routing (gateway-http.test.ts:343-401, already tested) and bridge promotion (createBridge at bridge.ts:29) β€” were correct. The break was the two MCP-tool-layer hops.

react was unaffected by #2 β€” it uses bridge("react", { ...rest }) with the full param spread (minus end_turn), so chat_id passes through naturally.

Changes

Commit 1: schema accepts negative chat IDs

  • New chatIdSchema in src/core/tools/schemas.ts β€” union of non-zero signed integer OR signed-integer string. Accepts both negative (group/channel) and positive (DM/user) chat IDs. Rejects zero (gateway already treats it as falsy-unrouted), non-integers, non-numeric strings, booleans, null, whitespace-padded strings.
  • src/core/tools/messaging.ts β€” send.chat_id and react.chat_id now use chatIdSchema. Field descriptions mention the negative-supergroup convention. idSchema itself is unchanged.
  • 24 schema tests (chat-id-schema.test.ts) β€” pin accept/reject contract, including the exact -1001426819337 from Dylan's test.

Commit 2: send.execute threads chat_id to every bridge call

  • src/core/tools/messaging.ts β€” every bridge() call inside send.execute now includes chat_id: params.chat_id. Local chat_id hoisted out of the switch for consistency across all 14 sites (text plain / scheduled / buttoned, photo, file, video, voice, audio, animation, sticker, poll, location, contact, dice). Comment block at top of execute documents the contract.
  • 16 pass-through tests β€” for every type case, send.execute calls bridge with exactly one action AND payload contains chat_id verbatim. Uses a captured fake-bridge spy, no network.

Test plan

  • npx tsc --noEmit clean
  • npx prettier --check clean on all 3 changed files
  • npx vitest run on chat-id-schema + tool-id-coercion + gateway-http + bridge + compose-tools β†’ 340/340 pass
  • Existing tool-id-coercion tests confirm idSchema is unchanged β€” message/user/reply ID fields still get the strict positive contract
  • Existing gateway-http negative-chat_id tests still pass

Follow-up

After this lands, the heartbeat outbound test should succeed in any group OR DM. The long-tail tools tracked in #149 (list_chats, plus chat_id on edit_message, delete_message, pin_message, read_chat_history) should use chatIdSchema from the start, and any new bridge() call in send.execute (e.g. new media types) must remember to forward chat_id β€” the new comment in execute documents this.

Surfaced-by: 2026-05-12 18:13Z heartbeat outbound test (Dylan)

πŸ€– Generated with Claude Code

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)
@dylanneve1
dylanneve1 enabled auto-merge (squash) May 12, 2026 18:26
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
@dylanneve1
dylanneve1 merged commit 9298246 into main May 12, 2026
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants