From f55894d33287a3ad07fb7aea745b5e376d405e56 Mon Sep 17 00:00:00 2001 From: Sergey Lavrinenko Date: Thu, 9 Jul 2026 15:25:55 +0300 Subject: [PATCH 1/3] feat: multi-chat fan-out via comma-separated chat_id (unified response) --- CHANGELOG.md | 36 ++ README.md | 7 + charts/express-botx/Chart.yaml | 4 +- docs/async-queues.md | 9 +- docs/commands.md | 35 +- docs/configuration.md | 19 +- docs/integrations.md | 102 ++++- examples/docker-compose-kafka/README.md | 2 +- examples/docker-compose-rabbitmq/README.md | 2 +- examples/gitlab/README.md | 9 +- internal/botapi/client.go | 6 - internal/cmd/enqueue.go | 315 ++++++++++---- internal/cmd/enqueue_test.go | 385 +++++++++++++++- internal/cmd/send.go | 165 +++++-- internal/cmd/send_test.go | 287 ++++++++++++ internal/cmd/serve_integration_test.go | 65 ++- internal/config/config.go | 25 ++ internal/server/api/openapi.yaml | 174 ++++++-- internal/server/handler_alertmanager.go | 64 ++- internal/server/handler_gitlab.go | 138 ++---- internal/server/handler_gitlab_test.go | 101 +++-- internal/server/handler_grafana.go | 64 ++- internal/server/handler_send.go | 210 +++++---- .../server/handler_send_multichat_test.go | 411 ++++++++++++++++++ .../server/handler_webhook_multichat_test.go | 301 +++++++++++++ internal/server/multisend.go | 127 ++++++ internal/server/multisend_test.go | 127 ++++++ internal/server/server_test.go | 147 ++++--- 28 files changed, 2769 insertions(+), 568 deletions(-) create mode 100644 CHANGELOG.md create mode 100644 internal/server/handler_send_multichat_test.go create mode 100644 internal/server/handler_webhook_multichat_test.go create mode 100644 internal/server/multisend.go create mode 100644 internal/server/multisend_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..6fbe0f1 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,36 @@ +# Changelog + +## 0.34.0 + +### ⚠️ Breaking: единый формат ответа (`MultiSendResponse`) на всех точках отправки + +Все эндпоинты отправки (`/send`, `/alertmanager`, `/grafana`, `/gitlab`) и CLI +(`send`, `enqueue`) теперь возвращают **единый конверт** `MultiSendResponse` — +даже для одного чата: + +```json +{"ok": true, + "results": [{"chat": "a", "sync_id": "..."}, + {"chat": "b", "request_id": "...", "queued": true}], + "errors": [{"chat": "c", "error": "resolving chat: ..."}]} +``` + +Раньше `/send`/`/alertmanager`/`/grafana` отвечали `{"ok":true,"sync_id":"..."}`, +а `/gitlab` — то одиночной, то fan-out-формой. Клиентам, которым нужен прежний +контракт, следует остаться на версии `0.33.x`. + +### Added: multi-chat fan-out (`chat_id` через запятую) + +- `chat_id=a,b,c` (в теле `/send` или `?chat_id=a,b,c` у вебхуков и CLI) + рассылает сообщение во **все** перечисленные чаты (fan-out, best-effort). + Дубликаты схлопываются, порядок сохраняется, чат и бот резолвятся для каждого + таргета отдельно; в `/send` inline-mentions парсятся резолвером каждого бота. +- **Коды ответа:** `200` — sync (доставлено ≥1 чата), `202` — async (enqueue), + `502` — во все чаты не удалось. Request-level ошибки (битый JSON, пустой + `chat_id`, невалидный `status`, неподдерживаемый media-type) сохраняют прежнюю + форму `{"ok":false,"error":"..."}` с кодами `400`/`415`. +- **Async expand:** в режиме `serve --enqueue` (и в CLI `enqueue`) многочатовый + `chat_id` раскрывается в N независимых сообщений в очереди (по одному на чат) + для per-chat retry/ack без дублей; worker не изменён. + +См. [docs/integrations.md](docs/integrations.md#мульти-чат-и-единый-ответ-multisendresponse). diff --git a/README.md b/README.md index 1386cc3..e0d54de 100644 --- a/README.md +++ b/README.md @@ -97,6 +97,12 @@ curl -X POST http://localhost:8080/api/v1/send \ Сервер автоматически добавляет заголовок `X-Request-ID` к каждому ответу (если клиент не передал свой, генерируется уникальный). Все HTTP-запросы логируются в stderr (метод, путь, статус, время выполнения). +> **Мульти-чат.** `chat_id` можно указать через запятую (`chat_id=a,b,c` в теле `/send` +> или `?chat_id=a,b,c` в вебхуках) — сообщение рассылается во все чаты (fan-out). +> Ответ всех эндпоинтов — единый `MultiSendResponse` с пер-чатовыми `results`/`errors`. +> **⚠️ Это ломающее изменение формата ответа** (раньше `{"ok":true,"sync_id":"..."}`); +> подробности — [docs/integrations.md](docs/integrations.md#мульти-чат-и-единый-ответ-multisendresponse). + Подробнее: [docs/integrations.md](docs/integrations.md) @@ -232,6 +238,7 @@ helm install express-botx oci://ghcr.io/lavr/charts/express-botx -f values.yaml | [docs/integrations.md](docs/integrations.md) | Alertmanager, Grafana, GitLab, примеры | | [docs/deployment.md](docs/deployment.md) | Docker, Helm, systemd, docker-compose | | [docs/async-queues.md](docs/async-queues.md) | RabbitMQ, Kafka, архитектура очередей | +| [CHANGELOG.md](CHANGELOG.md) | История изменений (в т.ч. ломающие) | | [docs/quickstart.md](docs/quickstart.md) | Базовые сценарии настройки | ## Лицензия diff --git a/charts/express-botx/Chart.yaml b/charts/express-botx/Chart.yaml index 80e2299..eb4e6ae 100644 --- a/charts/express-botx/Chart.yaml +++ b/charts/express-botx/Chart.yaml @@ -2,8 +2,8 @@ apiVersion: v2 name: express-botx description: eXpress BotX API gateway type: application -version: 0.29.1 -appVersion: "0.33.0" +version: 0.30.0 +appVersion: "0.34.0" home: https://github.com/lavr/express-botx sources: - https://github.com/lavr/express-botx diff --git a/docs/async-queues.md b/docs/async-queues.md index b92e87f..dfda9b3 100644 --- a/docs/async-queues.md +++ b/docs/async-queues.md @@ -114,6 +114,9 @@ express-botx enqueue --routing-mode catalog --bot alerts --chat-id deploy "Deplo # Mixed mode (default) express-botx enqueue --chat-id deploy "Hello" + +# Несколько чатов — expand в N сообщений в очереди (по одному на чат) +express-botx enqueue --chat-id deploy,ops-alerts "Hello" ``` ### HTTP-сервер @@ -123,10 +126,12 @@ express-botx enqueue --chat-id deploy "Hello" express-botx serve --enqueue --config config.yaml ``` -Ответ — `202 Accepted`: +Ответ — `202 Accepted`, единый `MultiSendResponse` (по одному `results`-элементу +на чат; `chat_id` через запятую → N сообщений в очереди): ```json -{"ok": true, "queued": true, "request_id": "0d6d7f87-0a2f-4c5b-b0d4-4d0b705a77e2"} +{"ok": true, + "results": [{"chat": "deploy", "request_id": "0d6d7f87-0a2f-4c5b-b0d4-4d0b705a77e2", "queued": true}]} ``` HTTP payload расширяется полями для direct routing: diff --git a/docs/commands.md b/docs/commands.md index 07ec237..45020dd 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -88,12 +88,19 @@ cat image.png | express-botx send --file - --file-name image.png express-botx send --host express.company.ru --bot-id UUID --secret KEY --chat-id UUID "Hello" ``` -При успехе утилита завершается молча (exit 0). Ошибки выводятся в stderr (exit 1). +При успехе (один чат) утилита завершается молча (exit 0). Ошибки выводятся в stderr (exit 1). + +**Несколько чатов.** `--chat-id` принимает список через запятую +(`--chat-id a,b,c`) → сообщение отправляется во все чаты (fan-out, best-effort). +В human-выводе печатается по строке на чат (`chat: sync_id` или `chat: ERROR ...`); +`--format json` отдаёт единый `MultiSendResponse` +(`{"ok":..,"results":[{chat,sync_id}],"errors":[{chat,error}]}`). Exit-код ≠0 +только если упали **все** чаты (частичный отказ — exit 0). ### Флаги ``` ---chat-id UUID или алиас целевого чата (опционально при наличии default) +--chat-id UUID или алиас чата; список через запятую (a,b,c) — fan-out; опционально при наличии default --body-from прочитать сообщение из файла --file путь к файлу-вложению (или - для stdin) --file-name имя файла (обязательно при --file -) @@ -234,7 +241,20 @@ express-botx enqueue --bot-id UUID --chat-id UUID "Привет, @mention[email: express-botx enqueue --no-parse --bot-id UUID --chat-id UUID "Текст с @mention[email:...] как есть" ``` -При успехе выводит `request_id` (text) или `{"ok":true,"queued":true,"request_id":"..."}` (json). +При успехе выводит `request_id` по строке на чат (text) или единый +`{"ok":..,"results":[{chat,request_id,queued:true}],"errors":[{chat,error}]}` +(json). Exit-код ≠0 возвращается только если **ни один** чат не поставлен в +очередь (аналогично 502 all-fail у сервера). + +**Несколько чатов.** `--chat-id` принимает список через запятую → в очередь +кладётся **N отдельных сообщений** (по одному на чат, expand на стороне +producer), выводится N `request_id`. Так каждый чат ретраится/подтверждается +независимо, без дублей; worker при этом не меняется («один чат = одно +сообщение»). Валидация/резолвинг маршрута — «всё или ничего»: если хоть один +чат в списке не проходит (не UUID в direct-режиме, неизвестный alias), команда +завершается ошибкой **до** публикации чего-либо, поэтому повтор не создаёт +дублей на уже успешных чатах. Сама публикация — best-effort: сбой брокера на +одном чате не отменяет остальные, а попадает в `errors[]`. ### Флаги @@ -242,7 +262,7 @@ express-botx enqueue --no-parse --bot-id UUID --chat-id UUID "Текст с @men --routing-mode direct | catalog | mixed (по умолчанию: mixed) --bot-id UUID бота (direct routing) --bot алиас бота из catalog (catalog/mixed) ---chat-id UUID или алиас чата +--chat-id UUID или алиас чата; список через запятую (a,b,c) — N сообщений в очередь --body-from прочитать сообщение из файла --file путь к файлу-вложению (или - для stdin) --file-name имя файла (обязательно при --file -) @@ -306,10 +326,13 @@ express-botx serve --config config.yaml --api-key env:MY_API_KEY express-botx serve --enqueue --config config.yaml ``` -Ответ в async-режиме: +Ответ в async-режиме — единый `MultiSendResponse` c кодом `202` (по одному +`results`-элементу на чат; `chat_id` через запятую раскрывается в N сообщений в +очереди): ```json -{"ok": true, "queued": true, "request_id": "0d6d7f87-0a2f-4c5b-b0d4-4d0b705a77e2"} +{"ok": true, + "results": [{"chat": "deploy", "request_id": "0d6d7f87-0a2f-4c5b-b0d4-4d0b705a77e2", "queued": true}]} ``` HTTP payload расширяется полями `routing_mode` и `bot_id` для direct routing: diff --git a/docs/configuration.md b/docs/configuration.md index b1a3144..15a4615 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -211,6 +211,14 @@ curl /api/v1/send -d '{"bot":"alert-bot","chat_id":"deploy","message":"!"}' curl /api/v1/alertmanager?bot=deploy-bot ``` +## Несколько чатов (`chat_id` через запятую) + +`chat_id` можно задать списком через запятую (`chat_id=a,b,c` в теле `/send` или +`?chat_id=a,b,c` в вебхуках/CLI) — сообщение рассылается во **все** перечисленные +чаты (fan-out, best-effort). Ответ единый для всех эндпоинтов — `MultiSendResponse` +с пер-чатовыми `results`/`errors`; подробности, коды ответа и **ломающее изменение +формата** — в [docs/integrations.md](integrations.md#мульти-чат-и-единый-ответ-multisendresponse). + ## Чат по умолчанию Один чат можно пометить как `default: true`. Он будет использоваться когда `--chat-id` (CLI) или `chat_id` (API) не указан: @@ -223,11 +231,16 @@ express-botx config chat set general UUID --no-default # снять помет express-botx config chat list # покажет (default) ``` -Приоритет выбора чата в HTTP-сервере: -- `/send`: `chat_id` из запроса → чат по умолчанию → ошибка -- `/alertmanager`, `/grafana`: `?chat_id=` → `default_chat_id` из конфига вебхука → чат по умолчанию → единственный чат → ошибка +Приоритет выбора чата в HTTP-сервере (`chat_id` может быть списком через запятую — +тогда фан-аут во все указанные чаты): +- `/send`: `chat_id` из запроса → чат по умолчанию → пустой `chat_id` даёт `400`. + Резолв конкретного чата/бота, если он не удался, — пер-чатовая ошибка в + `errors[]` (а не общий `400`); если упали все чаты — `502`. +- `/alertmanager`, `/grafana`: `?chat_id=` → `default_chat_id` из конфига вебхука → чат по умолчанию → единственный чат → пустой набор даёт `400`; пер-чатовые сбои — в `errors[]`, всё упало — `502` - `/gitlab`: `?chat_id=` → `routes` (все совпавшие правила, объединение+дедуп) → `default_chat_id` → чат по умолчанию → единственный чат → `200 {ignored}`; при совпадении sender-токена (`server.gitlab.senders`) цели — всегда `chats` этого sender'а, остальное игнорируется +Ответ всех эндпоинтов — единый `MultiSendResponse` (см. [Мульти-чат](integrations.md#мульти-чат-и-единый-ответ-multisendresponse)). + ## Формат host ```yaml diff --git a/docs/integrations.md b/docs/integrations.md index 410e625..6ac2a36 100644 --- a/docs/integrations.md +++ b/docs/integrations.md @@ -2,6 +2,72 @@ Подключение express-botx к системам мониторинга. +## Мульти-чат и единый ответ (`MultiSendResponse`) + +Все точки отправки (`/send`, `/alertmanager`, `/grafana`, `/gitlab` и CLI) +поддерживают **несколько чатов через запятую** в `chat_id` и возвращают **единый +формат ответа**. + +### `chat_id` через запятую (фан-аут) + +`chat_id=a,b,c` (в теле `/send` или в query `?chat_id=a,b,c` у вебхуков) +рассылает одно сообщение во **все** перечисленные чаты (fan-out). Дубликаты +схлопываются, порядок сохраняется, чат и бот резолвятся для каждого таргета +отдельно. Доставка **best-effort**: успех в один чат не блокируется падением +другого. + +### Единый ответ `MultiSendResponse` (ломающее изменение) + +> **⚠️ Ломающее изменение формата ответа.** Раньше `/send`/`/alertmanager`/ +> `/grafana` отвечали `{"ok":true,"sync_id":"..."}`, а `/gitlab` — то одиночной, +> то fan-out-формой. Теперь **все** эндпоинты, даже для одного чата, отвечают +> единым конвертом. Клиентам, которым нужен старый контракт, следует остаться на +> предыдущей версии. + +```json +{"ok": true, + "results": [{"chat":"a","sync_id":"..."}, + {"chat":"b","request_id":"...","queued":true}], + "errors": [{"chat":"c","error":"resolving chat: ..."}]} +``` + +- Пер-чатовый успех: `sync_id` (sync-отправка) либо `request_id`+`queued:true` + (async/enqueue). +- Пер-чатовые сбои доставки (резолв чата/бота, ошибка upstream) — в `errors[]`. +- **Коды ответа:** `200` — sync, доставлено ≥1 чата; `202` — async (enqueue); + `502` — во все чаты не удалось. +- **Request-level ошибки** (битый JSON, пустой `chat_id`, невалидный `status`, + неподдерживаемый media-type) сохраняют прежнюю форму `{"ok":false,"error":"..."}` + с кодами `400`/`415` и в `errors[]` **не** попадают. + +### Async: expand в N сообщений + +В async-режиме (`serve --enqueue`) `chat_id=a,b,c` раскрывается в **N отдельных +сообщений в очереди** (по одному на чат) — так каждый чат ретраится/подтверждается +независимо, без дублей; worker не меняется. + +### Переиспользуемый паттерн (для разработчиков) + +Мульти-чат — единый механизм на весь проект, а не копипаста по хендлерам. Любая +новая точка отправки должна переиспользовать примитивы из +[`internal/server/multisend.go`](../internal/server/multisend.go), а не +реализовывать fan-out заново: + +- `parseChatIDs(raw)` — разбор `chat_id` через запятую (trim, dedup, порядок + сохранён, пустые отброшены). +- `fanout(ctx, targets, deliver)` — best-effort обход таргетов: собирает + `[]SendResult` и `[]SendError`, порядок сохранён. +- `(*Server).fanoutSend(...)` — готовый `deliver` с резолвом чата+бота для + простых поверхностей без mentions (`/alertmanager`, `/grafana`). +- `MultiSendResponse` / `writeMultiSend(w, results, errs, successStatus)` — + единый конверт ответа и коды `200`/`202`/`502`. + +Request-level ошибки (битый вход, media-type) остаются вне `errors[]` в форме +`{"ok":false,"error":"..."}`. Кастомный `deliver` нужен только там, где per-target +логика отличается (например, per-bot mentions в `/send` sync). + +--- + ## Alertmanager ### Настройка express-botx @@ -46,7 +112,11 @@ route: ### Несколько чатов -Если нужно отправлять разные алерты в разные чаты, используйте разные receiver'ы с query-параметром `chat_id`: +Один алерт можно разослать сразу в несколько чатов через запятую в `?chat_id=` +(fan-out, единый ответ — см. [Мульти-чат](#мульти-чат-и-единый-ответ-multisendresponse)): +`...?chat_id=infra-alerts,app-alerts`. + +Если же разные алерты должны идти в разные чаты, используйте разные receiver'ы с query-параметром `chat_id`: ```yaml receivers: @@ -133,7 +203,8 @@ server: ### Несколько чатов -Аналогично Alertmanager — создайте несколько contact point'ов с `?chat_id=`: +Аналогично Alertmanager — либо один contact point с фан-аутом через запятую +(`?chat_id=infra-alerts,app-alerts`), либо несколько contact point'ов с `?chat_id=`: - `http://express-botx:8080/api/v1/grafana?chat_id=infra-alerts` - `http://express-botx:8080/api/v1/grafana?chat_id=app-alerts` @@ -355,10 +426,12 @@ server: `default_chat_id` → чат по умолчанию → единственный чат → `200 {ignored}`. **Фан-аут и коды ответа (best-effort):** сообщение отправляется в каждый целевой -чат независимо. Ответ `200` c телом -`{"ok":true,"results":[{"chat","sync_id"}],"errors":[{"chat","error"}]}`, если -доставлено **хотя бы в один** чат (частичные сбои — в `errors`); `502`, если -упали все. Явный `?chat_id=` сохраняет прежний одиночный ответ (`SuccessResponse`). +чат независимо. Ответ — **единый** `MultiSendResponse` +`{"ok":true,"results":[{"chat","sync_id"}],"errors":[{"chat","error"}]}` с кодом +`200`, если доставлено **хотя бы в один** чат (частичные сбои — в `errors`); +`502`, если упали все. Явный `?chat_id=` (в т.ч. с запятой — фан-аут в несколько +чатов) и одиночный `default_chat_id` возвращают ту же форму (`results[0]` для +одного чата) — см. [единый ответ и ломающее изменение](#единый-ответ-multisendresponse-ломающее-изменение). ```yaml server: @@ -547,10 +620,12 @@ def handle_event(): ## Произвольные вебхуки через /send -Для систем без специальных эндпоинтов используйте `/send`: +Для систем без специальных эндпоинтов используйте `/send`. `chat_id` можно +указать через запятую для рассылки в несколько чатов; ответ — единый +`MultiSendResponse` (см. [Мульти-чат](#мульти-чат-и-единый-ответ-multisendresponse)): ```bash -# JSON +# JSON — один чат curl -X POST http://express-botx:8080/api/v1/send \ -H "Authorization: Bearer " \ -H "Content-Type: application/json" \ @@ -560,6 +635,17 @@ curl -X POST http://express-botx:8080/api/v1/send \ "status": "ok" }' +# JSON — несколько чатов (fan-out) +curl -X POST http://express-botx:8080/api/v1/send \ + -H "Authorization: Bearer " \ + -H "Content-Type: application/json" \ + -d '{ + "chat_id": "deploy,ops-alerts", + "message": "Deploy v1.2.3 completed" + }' +# → {"ok":true,"results":[{"chat":"deploy","sync_id":"..."}, +# {"chat":"ops-alerts","sync_id":"..."}]} + # С файлом (multipart) curl -X POST http://express-botx:8080/api/v1/send \ -H "Authorization: Bearer " \ diff --git a/examples/docker-compose-kafka/README.md b/examples/docker-compose-kafka/README.md index 72f0ba8..d8d5468 100644 --- a/examples/docker-compose-kafka/README.md +++ b/examples/docker-compose-kafka/README.md @@ -51,5 +51,5 @@ curl -X POST http://localhost:8080/api/v1/send \ "message": "Hello from async mode!" }' -# Ответ: {"ok":true,"queued":true,"request_id":"..."} +# Ответ (202): {"ok":true,"results":[{"chat":"","request_id":"...","queued":true}]} ``` diff --git a/examples/docker-compose-rabbitmq/README.md b/examples/docker-compose-rabbitmq/README.md index 3769f8b..569f114 100644 --- a/examples/docker-compose-rabbitmq/README.md +++ b/examples/docker-compose-rabbitmq/README.md @@ -43,7 +43,7 @@ curl -X POST http://localhost:8080/api/v1/send \ "message": "Hello from async mode!" }' -# Ответ: {"ok":true,"queued":true,"request_id":"..."} +# Ответ (202): {"ok":true,"results":[{"chat":"","request_id":"...","queued":true}]} ``` ## Мониторинг diff --git a/examples/gitlab/README.md b/examples/gitlab/README.md index b927580..e0d7ff3 100644 --- a/examples/gitlab/README.md +++ b/examples/gitlab/README.md @@ -79,10 +79,13 @@ for the filter rules, template registry, template variables/helpers, and fans a single event out to one or more chats by project, event key, and branch (glob or `/regex/` patterns). All matching rules contribute their chats (unioned and de-duplicated); a rule with `stop: true` ends the scan; unmatched events fall -back to `default_chat_id`. Delivery is best-effort: `200` with `{results,errors}` -once at least one chat is delivered, `502` if they all fail. See the +back to `default_chat_id`. Delivery is best-effort: the response is always a +`MultiSendResponse` — `200` with `{"ok":true,"results":[…],"errors":[…]}` once at +least one chat is delivered, `502` if they all fail. A comma-separated +`?chat_id=a,b` fans out the same way. See the [routing section](../../docs/integrations.md#роутинг-событий-по-чатам-routes) -for the full model and chat-selection priority. +and [Мульти-чат](../../docs/integrations.md#мульти-чат-и-единый-ответ-multisendresponse) +for the full model, response format and chat-selection priority. ## Per-team tokens (senders) diff --git a/internal/botapi/client.go b/internal/botapi/client.go index b5eea76..53c3ab5 100644 --- a/internal/botapi/client.go +++ b/internal/botapi/client.go @@ -290,12 +290,6 @@ func BuildSendRequest(p *SendParams) *SendRequest { // ErrUnauthorized indicates the token is invalid or expired. var ErrUnauthorized = fmt.Errorf("unauthorized (HTTP 401)") -// Send posts a notification (text and/or file) to a chat via BotX API. -func (c *Client) Send(ctx context.Context, sr *SendRequest) error { - _, err := c.SendWithSyncID(ctx, sr) - return err -} - type sendAPIResponse struct { Status string `json:"status"` Result struct { diff --git a/internal/cmd/enqueue.go b/internal/cmd/enqueue.go index c4f20db..9335c2c 100644 --- a/internal/cmd/enqueue.go +++ b/internal/cmd/enqueue.go @@ -12,6 +12,7 @@ import ( "io" "os" "path/filepath" + "strings" "time" "github.com/lavr/express-botx/internal/botapi" @@ -104,59 +105,11 @@ Options: return fmt.Errorf("--status must be ok or error, got %q", status) } - // Determine effective routing mode and validate requirements + // Determine effective routing mode. Bot and chat resolution runs per target + // chat in the enqueue loop below, since a catalog lookup can bind a different + // bot per chat. mode := cfg.Producer.RoutingMode - botID := cfg.BotID botAlias := flags.Bot // --bot flag: alias for catalog resolution (not resolved via config) - chatID := cfg.ChatID - - // Observability fields for work message - var routeHost, routeBotName, routeChatAlias, routeCatalogRevision string - - switch config.RoutingMode(mode) { - case config.RoutingDirect: - if botID == "" { - return fmt.Errorf("--bot-id is required for direct routing mode") - } - if !config.IsUUID(botID) { - return fmt.Errorf("--bot-id must be a valid UUID for direct routing mode, got %q", botID) - } - if chatID == "" { - return fmt.Errorf("--chat-id is required for direct routing mode") - } - if !config.IsUUID(chatID) { - return fmt.Errorf("--chat-id must be a valid UUID for direct routing mode, got %q; use catalog or mixed mode for alias resolution", chatID) - } - case config.RoutingMixed: - // Mixed: use direct if bot_id and chat_id are both provided and both - // look like UUIDs. Otherwise fall back to catalog for alias resolution. - if botID != "" && chatID != "" && config.IsUUID(botID) && config.IsUUID(chatID) { - // Direct path — no catalog needed - } else { - // Need catalog for alias resolution - resolved, err := resolveViaCatalog(cfg, botID, botAlias, chatID) - if err != nil { - return err - } - botID = resolved.BotID - chatID = resolved.ChatID - routeHost = resolved.Host - routeBotName = resolved.BotName - routeChatAlias = resolved.ChatAlias - routeCatalogRevision = resolved.CatalogRevision - } - case config.RoutingCatalog: - resolved, err := resolveViaCatalog(cfg, botID, botAlias, chatID) - if err != nil { - return err - } - botID = resolved.BotID - chatID = resolved.ChatID - routeHost = resolved.Host - routeBotName = resolved.BotName - routeChatAlias = resolved.ChatAlias - routeCatalogRevision = resolved.CatalogRevision - } // Read file attachment if requested var fileAttachment *botapi.SendFile @@ -257,67 +210,239 @@ Options: } defer pub.Close() - // Build work message - requestID := newRequestID() - - msg := &queue.WorkMessage{ - RequestID: requestID, - Routing: queue.Routing{ - Host: routeHost, - BotID: botID, - ChatID: chatID, - BotName: routeBotName, - ChatAlias: routeChatAlias, - CatalogRevision: routeCatalogRevision, - }, - Payload: queue.Payload{ - Message: message, - Status: status, - Metadata: meta, - Mentions: ment, - Opts: queue.DeliveryOpts{ - Silent: silent, - Stealth: stealth, - ForceDND: forceDND, - NoNotify: noNotify, - NoParse: noParse, - }, - }, - ReplyTo: cfg.Queue.ReplyQueue, - EnqueuedAt: time.Now().UTC(), + // Expand a comma-separated --chat-id (--chat-id a,b,c) into N independent + // enqueues — one queued message per chat — rather than one message that fans + // out inside the worker. This mirrors the async /send handler and is a + // deliberate choice: with one message per chat, retry/ack is per-chat and + // independent, so a transient failure re-delivers only the failed chat and + // never produces duplicates on the chats that already succeeded (nor loses a + // failed chat with the ack). The "one message = whole command, fan out in the + // worker" alternative gives per-command retry, which on a partial failure + // re-sends to the already-delivered chats (duplicates) or drops the failed + // chat. The worker stays "one chat = one message" and is not touched. + chats := parseChatIDs(cfg.ChatID) + if len(chats) == 0 { + // Empty/whitespace/only-commas: fall through with the raw value so the + // per-mode "--chat-id is required" checks below produce their usual error. + chats = []string{cfg.ChatID} } - if fileAttachment != nil { - msg.Payload.File = &queue.FileAttachment{ - FileName: fileAttachment.FileName, - Data: fileAttachment.Data, + // Phase 1 — resolve routing and build a work message for every target chat. + // This pass is all-or-nothing: if any chat fails routing validation or catalog + // resolution we return before publishing anything, so nothing lands on the + // queue for a request that a retry would re-run in full. This mirrors the async + // /send handler, which validates every target up front and accepts or rejects + // the request as a whole — and it is what makes the "never produces duplicates + // on the chats that already succeeded" guarantee above hold: a bad chat in the + // list can no longer partially publish the good chats before aborting. + type pendingEnqueue struct { + chat string + msg *queue.WorkMessage + } + pending := make([]pendingEnqueue, 0, len(chats)) + for _, chat := range chats { + // Resolve bot + chat per target; a catalog lookup can bind a different bot + // per chat, so each iteration starts from the original bot_id. + botID := cfg.BotID + chatID := chat + var routeHost, routeBotName, routeChatAlias, routeCatalogRevision string + + switch config.RoutingMode(mode) { + case config.RoutingDirect: + if botID == "" { + return fmt.Errorf("--bot-id is required for direct routing mode") + } + if !config.IsUUID(botID) { + return fmt.Errorf("--bot-id must be a valid UUID for direct routing mode, got %q", botID) + } + if chatID == "" { + return fmt.Errorf("--chat-id is required for direct routing mode") + } + if !config.IsUUID(chatID) { + return fmt.Errorf("--chat-id must be a valid UUID for direct routing mode, got %q; use catalog or mixed mode for alias resolution", chatID) + } + case config.RoutingMixed: + // Mixed: use direct if bot_id and chat_id are both provided and both + // look like UUIDs. Otherwise fall back to catalog for alias resolution. + if botID != "" && chatID != "" && config.IsUUID(botID) && config.IsUUID(chatID) { + // Direct path — no catalog needed + } else { + // Need catalog for alias resolution + resolved, rerr := resolveViaCatalog(cfg, botID, botAlias, chatID) + if rerr != nil { + return rerr + } + botID = resolved.BotID + chatID = resolved.ChatID + routeHost = resolved.Host + routeBotName = resolved.BotName + routeChatAlias = resolved.ChatAlias + routeCatalogRevision = resolved.CatalogRevision + } + case config.RoutingCatalog: + resolved, rerr := resolveViaCatalog(cfg, botID, botAlias, chatID) + if rerr != nil { + return rerr + } + botID = resolved.BotID + chatID = resolved.ChatID + routeHost = resolved.Host + routeBotName = resolved.BotName + routeChatAlias = resolved.ChatAlias + routeCatalogRevision = resolved.CatalogRevision + } + + // Build work message + requestID := newRequestID() + + msg := &queue.WorkMessage{ + RequestID: requestID, + Routing: queue.Routing{ + Host: routeHost, + BotID: botID, + ChatID: chatID, + BotName: routeBotName, + ChatAlias: routeChatAlias, + CatalogRevision: routeCatalogRevision, + }, + Payload: queue.Payload{ + Message: message, + Status: status, + Metadata: meta, + Mentions: ment, + Opts: queue.DeliveryOpts{ + Silent: silent, + Stealth: stealth, + ForceDND: forceDND, + NoNotify: noNotify, + NoParse: noParse, + }, + }, + ReplyTo: cfg.Queue.ReplyQueue, + EnqueuedAt: time.Now().UTC(), + } + + if fileAttachment != nil { + msg.Payload.File = &queue.FileAttachment{ + FileName: fileAttachment.FileName, + Data: fileAttachment.Data, + } } + + pending = append(pending, pendingEnqueue{chat: chat, msg: msg}) } - // Publish - if err := pub.PublishWork(context.Background(), msg); err != nil { - return fmt.Errorf("publishing to queue: %w", err) + // Phase 2 — publish each built message. Publishing is best-effort and per-chat + // independent: a broker failure on one chat is recorded and the remaining chats + // still publish, so a retry re-enqueues only the failed chats rather than + // double-publishing the ones that already landed. The exit code is non-zero + // only when every chat failed (see printEnqueueResults). + results := make([]enqueueResult, 0, len(pending)) + for _, p := range pending { + if err := pub.PublishWork(context.Background(), p.msg); err != nil { + results = append(results, enqueueResult{Chat: p.chat, Error: fmt.Errorf("publishing to queue: %w", err).Error()}) + continue + } + results = append(results, enqueueResult{Chat: p.chat, RequestID: p.msg.RequestID}) } // Output - return printEnqueueResult(deps.Stdout, cfg.Format, requestID) + return printEnqueueResults(deps.Stdout, cfg.Format, results) +} + +// enqueueResult is a single per-chat enqueue outcome: the target chat plus +// either the request_id assigned to its queued message (success) or an error +// string (publish failure). +type enqueueResult struct { + Chat string + RequestID string + Error string +} + +// parseChatIDs splits a raw --chat-id value on commas, trims whitespace, drops +// empties, and deduplicates while preserving first-occurrence order. A blank or +// whitespace-only input yields an empty slice. Mirrors the server-side helper so +// the CLI and HTTP surfaces expand a comma-separated chat_id the same way. +func parseChatIDs(raw string) []string { + parts := strings.Split(raw, ",") + out := make([]string, 0, len(parts)) + seen := make(map[string]struct{}, len(parts)) + for _, p := range parts { + chat := strings.TrimSpace(p) + if chat == "" { + continue + } + if _, dup := seen[chat]; dup { + continue + } + seen[chat] = struct{}{} + out = append(out, chat) + } + return out } -// printEnqueueResult outputs the request_id in the appropriate format. -func printEnqueueResult(w io.Writer, format, requestID string) error { - type enqueueResponse struct { - OK bool `json:"ok"` - Queued bool `json:"queued"` - RequestID string `json:"request_id"` +// printEnqueueResults outputs one request_id per successfully enqueued chat. In +// human format each request_id is printed on its own line (a single chat prints +// one line, as before); when more than one chat is targeted, a per-chat publish +// failure is printed as a "chat: ERROR .." line. In json format it emits the +// uniform multi-chat response body +// {"ok":..,"results":[{chat,request_id,queued:true}],"errors":[{chat,error}]}. +// A non-nil error (non-zero exit) is returned only when every chat failed, +// mirroring the sync send / server 502 all-fail semantics. +func printEnqueueResults(w io.Writer, format string, results []enqueueResult) error { + okCount := 0 + for _, r := range results { + if r.Error == "" { + okCount++ + } } if format == "json" { + type jsonResult struct { + Chat string `json:"chat"` + RequestID string `json:"request_id"` + Queued bool `json:"queued"` + } + type jsonError struct { + Chat string `json:"chat"` + Error string `json:"error"` + } + type jsonResponse struct { + OK bool `json:"ok"` + Results []jsonResult `json:"results"` + Errors []jsonError `json:"errors,omitempty"` + } + resp := jsonResponse{OK: okCount > 0, Results: make([]jsonResult, 0, len(results))} + for _, r := range results { + if r.Error != "" { + resp.Errors = append(resp.Errors, jsonError{Chat: r.Chat, Error: r.Error}) + continue + } + resp.Results = append(resp.Results, jsonResult{Chat: r.Chat, RequestID: r.RequestID, Queued: true}) + } enc := json.NewEncoder(w) enc.SetIndent("", " ") - return enc.Encode(enqueueResponse{OK: true, Queued: true, RequestID: requestID}) + if err := enc.Encode(resp); err != nil { + return err + } + } else { + for _, r := range results { + if r.Error != "" { + if len(results) > 1 { + fmt.Fprintf(w, "%s: ERROR %s\n", r.Chat, r.Error) + } + continue + } + fmt.Fprintln(w, r.RequestID) + } } - fmt.Fprintln(w, requestID) + if okCount == 0 { + if len(results) == 1 { + return fmt.Errorf("%s", results[0].Error) + } + return fmt.Errorf("all %d chats failed", len(results)) + } return nil } diff --git a/internal/cmd/enqueue_test.go b/internal/cmd/enqueue_test.go index 58afdbc..b2f8c1c 100644 --- a/internal/cmd/enqueue_test.go +++ b/internal/cmd/enqueue_test.go @@ -2,12 +2,14 @@ package cmd import ( "bytes" + "context" "encoding/json" "fmt" "net/http" "net/http/httptest" "os" "strings" + "sync" "testing" "time" @@ -19,6 +21,11 @@ import ( // Registered as "testfake" driver so runEnqueue can create publishers via factory. var testFakeQueue = queue.NewFake() +// testFailQueue is a shared publisher that fails PublishWork for chats listed in +// its failChats set. Registered as the "testfailpub" driver so enqueue tests can +// exercise the best-effort phase-2 publish path (partial and all-fail). +var testFailQueue = &failingPublisher{} + func init() { queue.Register("testfake", queue.DriverFactory{ NewPublisher: func(url, name string) (queue.Publisher, error) { @@ -28,8 +35,65 @@ func init() { return testFakeQueue, nil }, }) + queue.Register("testfailpub", queue.DriverFactory{ + NewPublisher: func(url, name string) (queue.Publisher, error) { + return testFailQueue, nil + }, + NewConsumer: func(url, name, group string) (queue.Consumer, error) { + return nil, fmt.Errorf("testfailpub has no consumer") + }, + }) +} + +// failingPublisher records published work but returns an error for any chat in +// failChats, so a comma-separated enqueue can be driven into a partial or total +// publish failure. +type failingPublisher struct { + mu sync.Mutex + published []*queue.WorkMessage + failChats map[string]bool +} + +func (p *failingPublisher) reset(failChats ...string) { + p.mu.Lock() + defer p.mu.Unlock() + p.published = nil + p.failChats = make(map[string]bool, len(failChats)) + for _, c := range failChats { + p.failChats[c] = true + } +} + +func (p *failingPublisher) publishedChats() []string { + p.mu.Lock() + defer p.mu.Unlock() + out := make([]string, 0, len(p.published)) + for _, m := range p.published { + out = append(out, m.Routing.ChatID) + } + return out +} + +func (p *failingPublisher) PublishWork(_ context.Context, msg *queue.WorkMessage) error { + p.mu.Lock() + defer p.mu.Unlock() + if p.failChats[msg.Routing.ChatID] { + return fmt.Errorf("broker down for %s", msg.Routing.ChatID) + } + p.published = append(p.published, msg) + return nil } +func (p *failingPublisher) PublishResult(context.Context, string, *queue.WorkResult) error { + return nil +} + +func (p *failingPublisher) PublishCatalog(context.Context, string, *queue.CatalogSnapshot) error { + return nil +} + +func (p *failingPublisher) Close() error { return nil } + func TestEnqueue_DirectMode_Success(t *testing.T) { testFakeQueue.Reset() @@ -113,9 +177,12 @@ queue: } var resp struct { - OK bool `json:"ok"` - Queued bool `json:"queued"` - RequestID string `json:"request_id"` + OK bool `json:"ok"` + Results []struct { + Chat string `json:"chat"` + RequestID string `json:"request_id"` + Queued bool `json:"queued"` + } `json:"results"` } if err := json.Unmarshal(stdout.Bytes(), &resp); err != nil { t.Fatalf("invalid JSON output: %v\nraw: %s", err, stdout.String()) @@ -123,12 +190,18 @@ queue: if !resp.OK { t.Error("expected ok=true") } - if !resp.Queued { + if len(resp.Results) != 1 { + t.Fatalf("results = %+v, want 1", resp.Results) + } + if !resp.Results[0].Queued { t.Error("expected queued=true") } - if resp.RequestID == "" { + if resp.Results[0].RequestID == "" { t.Error("expected non-empty request_id") } + if resp.Results[0].Chat != "00000000-0000-0000-0000-000000000c02" { + t.Errorf("chat = %q, want target chat", resp.Results[0].Chat) + } } func TestEnqueue_DirectMode_WithOpts(t *testing.T) { @@ -1072,3 +1145,305 @@ queue: t.Errorf("expected 'nothing to send' error, got: %v", err) } } + +// Task 5: --chat-id accepts a comma-separated list and expands into N +// independent enqueues — one queued message per chat — printing one request_id +// per chat. Uses direct mode with UUID chats so no catalog is needed. + +func TestEnqueue_MultiChat_Expands(t *testing.T) { + testFakeQueue.Reset() + cfgPath := writeTestConfig(t, ` +queue: + driver: testfake + url: fake://localhost + name: test-work +`) + deps, stdout, _ := testDeps() + deps.Stdin = strings.NewReader("") + deps.IsTerminal = true + + err := runEnqueue([]string{ + "--config", cfgPath, + "--bot-id", "00000000-0000-0000-0000-000000000b01", + "--chat-id", "00000000-0000-0000-0000-00000000000a,00000000-0000-0000-0000-00000000000b,00000000-0000-0000-0000-00000000000c", + "--routing-mode", "direct", + "fanned out", + }, deps) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // One work message per chat (expand), each carrying only its own chat. + msgs := testFakeQueue.WorkMessages() + if len(msgs) != 3 { + t.Fatalf("expected 3 work messages, got %d", len(msgs)) + } + gotChats := map[string]bool{} + gotReqIDs := map[string]bool{} + for _, m := range msgs { + gotChats[m.Routing.ChatID] = true + gotReqIDs[m.RequestID] = true + if m.Payload.Message != "fanned out" { + t.Errorf("message = %q, want shared payload", m.Payload.Message) + } + } + for _, c := range []string{ + "00000000-0000-0000-0000-00000000000a", + "00000000-0000-0000-0000-00000000000b", + "00000000-0000-0000-0000-00000000000c", + } { + if !gotChats[c] { + t.Errorf("missing enqueue for chat %q", c) + } + } + if len(gotReqIDs) != 3 { + t.Errorf("expected 3 distinct request_ids, got %d", len(gotReqIDs)) + } + + // Human output: one request_id per line. + lines := strings.Fields(strings.TrimSpace(stdout.String())) + if len(lines) != 3 { + t.Fatalf("expected 3 request_id lines, got %d: %q", len(lines), stdout.String()) + } + for _, l := range lines { + if !gotReqIDs[l] { + t.Errorf("printed request_id %q not among enqueued %v", l, gotReqIDs) + } + } +} + +func TestEnqueue_MultiChat_Dedup(t *testing.T) { + testFakeQueue.Reset() + cfgPath := writeTestConfig(t, ` +queue: + driver: testfake + url: fake://localhost + name: test-work +`) + deps, _, _ := testDeps() + deps.Stdin = strings.NewReader("") + deps.IsTerminal = true + + err := runEnqueue([]string{ + "--config", cfgPath, + "--bot-id", "00000000-0000-0000-0000-000000000b01", + "--chat-id", "00000000-0000-0000-0000-00000000000a, 00000000-0000-0000-0000-00000000000a ,00000000-0000-0000-0000-00000000000b", + "--routing-mode", "direct", + "dedup me", + }, deps) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + msgs := testFakeQueue.WorkMessages() + if len(msgs) != 2 { + t.Fatalf("expected 2 deduped work messages, got %d", len(msgs)) + } +} + +func TestEnqueue_MultiChat_JSONOutput(t *testing.T) { + testFakeQueue.Reset() + cfgPath := writeTestConfig(t, ` +queue: + driver: testfake + url: fake://localhost + name: test-work +`) + deps, stdout, _ := testDeps() + deps.Stdin = strings.NewReader("") + deps.IsTerminal = true + + err := runEnqueue([]string{ + "--config", cfgPath, + "--bot-id", "00000000-0000-0000-0000-000000000b01", + "--chat-id", "00000000-0000-0000-0000-00000000000a,00000000-0000-0000-0000-00000000000b", + "--routing-mode", "direct", + "--format", "json", + "json multi", + }, deps) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var resp struct { + OK bool `json:"ok"` + Results []struct { + Chat string `json:"chat"` + RequestID string `json:"request_id"` + Queued bool `json:"queued"` + } `json:"results"` + } + if err := json.Unmarshal(stdout.Bytes(), &resp); err != nil { + t.Fatalf("invalid JSON output: %v\nraw: %s", err, stdout.String()) + } + if !resp.OK || len(resp.Results) != 2 { + t.Fatalf("response = %+v, want ok with 2 results", resp) + } + if resp.Results[0].Chat != "00000000-0000-0000-0000-00000000000a" || + resp.Results[1].Chat != "00000000-0000-0000-0000-00000000000b" { + t.Errorf("chats = %q/%q, want a then b", resp.Results[0].Chat, resp.Results[1].Chat) + } + for _, r := range resp.Results { + if !r.Queued || r.RequestID == "" { + t.Errorf("result = %+v, want queued with request_id", r) + } + } +} + +func TestEnqueue_SingleChat_UnchangedHumanOutput(t *testing.T) { + testFakeQueue.Reset() + cfgPath := writeTestConfig(t, ` +queue: + driver: testfake + url: fake://localhost + name: test-work +`) + deps, stdout, _ := testDeps() + deps.Stdin = strings.NewReader("") + deps.IsTerminal = true + + err := runEnqueue([]string{ + "--config", cfgPath, + "--bot-id", "00000000-0000-0000-0000-000000000b01", + "--chat-id", "00000000-0000-0000-0000-00000000000a", + "--routing-mode", "direct", + "single", + }, deps) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + msgs := testFakeQueue.WorkMessages() + if len(msgs) != 1 { + t.Fatalf("expected 1 work message, got %d", len(msgs)) + } + // Single-chat human output is one line = the request_id (backward compatible). + out := strings.TrimSpace(stdout.String()) + if out != msgs[0].RequestID { + t.Errorf("output = %q, want single request_id %q", out, msgs[0].RequestID) + } +} + +// TestEnqueue_MultiChat_ValidationFailPublishesNothing locks in the all-or-nothing +// phase-1 resolve pass: if any chat fails routing validation the command returns +// before publishing anything, so a retry never double-publishes the good chats. +func TestEnqueue_MultiChat_ValidationFailPublishesNothing(t *testing.T) { + testFakeQueue.Reset() + cfgPath := writeTestConfig(t, ` +queue: + driver: testfake + url: fake://localhost + name: test-work +`) + deps, stdout, _ := testDeps() + deps.Stdin = strings.NewReader("") + deps.IsTerminal = true + + // First chat is a valid UUID, second is not — direct mode rejects the second. + err := runEnqueue([]string{ + "--config", cfgPath, + "--bot-id", "00000000-0000-0000-0000-000000000b01", + "--chat-id", "00000000-0000-0000-0000-00000000000a,not-a-uuid", + "--routing-mode", "direct", + "should not publish", + }, deps) + if err == nil { + t.Fatal("expected error for invalid second chat, got nil") + } + if msgs := testFakeQueue.WorkMessages(); len(msgs) != 0 { + t.Fatalf("expected 0 published messages on validation failure, got %d", len(msgs)) + } + if out := strings.TrimSpace(stdout.String()); out != "" { + t.Errorf("expected no stdout on validation failure, got %q", out) + } +} + +// TestEnqueue_MultiChat_PartialPublishFail verifies best-effort phase-2 publish: +// a broker failure on one chat is recorded while the other chat still publishes, +// and the command exits zero (at least one chat succeeded). +func TestEnqueue_MultiChat_PartialPublishFail(t *testing.T) { + chatA := "00000000-0000-0000-0000-00000000000a" + chatB := "00000000-0000-0000-0000-00000000000b" + testFailQueue.reset(chatB) // chatB publish fails, chatA succeeds + cfgPath := writeTestConfig(t, ` +queue: + driver: testfailpub + url: fake://localhost + name: test-work +`) + deps, stdout, _ := testDeps() + deps.Stdin = strings.NewReader("") + deps.IsTerminal = true + + err := runEnqueue([]string{ + "--config", cfgPath, + "--bot-id", "00000000-0000-0000-0000-000000000b01", + "--chat-id", chatA + "," + chatB, + "--routing-mode", "direct", + "--format", "json", + "partial", + }, deps) + if err != nil { + t.Fatalf("expected nil error when one chat succeeds, got %v", err) + } + + // Only chatA reached the broker; chatB was not re-attempted or dropped silently. + if got := testFailQueue.publishedChats(); len(got) != 1 || got[0] != chatA { + t.Fatalf("published chats = %v, want [%s] only", got, chatA) + } + + var resp struct { + OK bool `json:"ok"` + Results []struct { + Chat string `json:"chat"` + RequestID string `json:"request_id"` + } `json:"results"` + Errors []struct { + Chat string `json:"chat"` + Error string `json:"error"` + } `json:"errors"` + } + if uerr := json.Unmarshal(stdout.Bytes(), &resp); uerr != nil { + t.Fatalf("invalid JSON output: %v\nraw: %s", uerr, stdout.String()) + } + if !resp.OK { + t.Errorf("ok = false, want true (chatA succeeded)") + } + if len(resp.Results) != 1 || resp.Results[0].Chat != chatA || resp.Results[0].RequestID == "" { + t.Errorf("results = %+v, want single request_id for %s", resp.Results, chatA) + } + if len(resp.Errors) != 1 || resp.Errors[0].Chat != chatB { + t.Errorf("errors = %+v, want single error for %s", resp.Errors, chatB) + } +} + +// TestEnqueue_MultiChat_AllPublishFail verifies the all-fail exit: when every +// chat's publish fails the command returns a non-nil error (non-zero exit). +func TestEnqueue_MultiChat_AllPublishFail(t *testing.T) { + chatA := "00000000-0000-0000-0000-00000000000a" + chatB := "00000000-0000-0000-0000-00000000000b" + testFailQueue.reset(chatA, chatB) + cfgPath := writeTestConfig(t, ` +queue: + driver: testfailpub + url: fake://localhost + name: test-work +`) + deps, _, _ := testDeps() + deps.Stdin = strings.NewReader("") + deps.IsTerminal = true + + err := runEnqueue([]string{ + "--config", cfgPath, + "--bot-id", "00000000-0000-0000-0000-000000000b01", + "--chat-id", chatA + "," + chatB, + "--routing-mode", "direct", + "all fail", + }, deps) + if err == nil { + t.Fatal("expected non-nil error when every chat fails, got nil") + } + if got := testFailQueue.publishedChats(); len(got) != 0 { + t.Errorf("published chats = %v, want none", got) + } +} diff --git a/internal/cmd/send.go b/internal/cmd/send.go index cd035b2..c249be7 100644 --- a/internal/cmd/send.go +++ b/internal/cmd/send.go @@ -126,8 +126,18 @@ Options: if err != nil { return err } - if err := cfg.RequireChatID(); err != nil { - return err + + // Determine target chats up front so an empty --chat-id fails fast (before + // authenticating or reading files). A comma-separated --chat-id (--chat-id a,b,c) + // fans the same message out to each chat; an empty value falls back to the + // single/default chat auto-selection. The chats are resolved to UUIDs later, + // per target, so a bad alias fails only that chat rather than the whole command. + chats := parseChatIDs(cfg.ChatID) + if len(chats) == 0 { + if err := cfg.RequireChatID(); err != nil { + return err + } + chats = []string{cfg.ChatID} } // Validate status @@ -240,38 +250,135 @@ Options: fmt.Fprintf(deps.Stderr, "warning: mention %s: %s\n", e.Kind, e.Cause) } - // Build SendRequest - sr := botapi.BuildSendRequest(&botapi.SendParams{ - ChatID: cfg.ChatID, - Message: parseResult.Message, - Status: status, - File: fileAttachment, - Metadata: meta, - Mentions: parseResult.Mentions, - Silent: silent, - Stealth: stealth, - ForceDND: forceDND, - NoNotify: noNotify, - }) - - // Send - err = client.Send(context.Background(), sr) + // Fan out to every target chat via the single configured bot. The CLI uses one + // bot/token per command (unlike the sync /send handler, which resolves a bot + // per chat), so mentions are parsed once above and the resolved message is + // reused across chats with only the target chat swapped. Delivery is + // best-effort and per-chat independent: a failed chat is recorded, the rest + // still send, and the exit code is non-zero only if every chat failed. + results := make([]sendCmdResult, 0, len(chats)) + for _, chat := range chats { + chatID, rerr := cfg.ResolveChatAlias(chat) + if rerr != nil { + results = append(results, sendCmdResult{Chat: chat, Error: rerr.Error()}) + continue + } + + sr := botapi.BuildSendRequest(&botapi.SendParams{ + ChatID: chatID, + Message: parseResult.Message, + Status: status, + File: fileAttachment, + Metadata: meta, + Mentions: parseResult.Mentions, + Silent: silent, + Stealth: stealth, + ForceDND: forceDND, + NoNotify: noNotify, + }) + + syncID, serr := sendWithRefresh(client, cfg, cache, sr) + if serr != nil { + results = append(results, sendCmdResult{Chat: chat, Error: serr.Error()}) + continue + } + results = append(results, sendCmdResult{Chat: chat, SyncID: syncID}) + } + + return printSendResults(deps.Stdout, cfg.Format, results) +} + +// sendWithRefresh posts one SendRequest and returns its sync_id, refreshing the +// token once on a 401 exactly as the original single-chat path did. +func sendWithRefresh(client *botapi.Client, cfg *config.Config, cache token.Cache, sr *botapi.SendRequest) (string, error) { + syncID, err := client.SendWithSyncID(context.Background(), sr) + if err != nil && errors.Is(err, botapi.ErrUnauthorized) { + if cfg.BotToken != "" { + return "", fmt.Errorf("bot token rejected (401), re-configure token") + } + tok, rerr := refreshToken(cfg, cache) + if rerr != nil { + return "", fmt.Errorf("refreshing token: %w", rerr) + } + client.Token = tok + syncID, err = client.SendWithSyncID(context.Background(), sr) + } if err != nil { - if errors.Is(err, botapi.ErrUnauthorized) { - if cfg.BotToken != "" { - return fmt.Errorf("bot token rejected (401), re-configure token") - } - tok, err = refreshToken(cfg, cache) - if err != nil { - return fmt.Errorf("refreshing token: %w", err) + return "", fmt.Errorf("sending: %w", err) + } + return syncID, nil +} + +// sendCmdResult is one per-chat sync send outcome: the target chat plus either +// its sync_id (success) or an error string. +type sendCmdResult struct { + Chat string + SyncID string + Error string +} + +// printSendResults renders per-chat sync send outcomes. In json format it emits +// the uniform multi-chat body +// {"ok":..,"results":[{chat,sync_id}],"errors":[{chat,error}]}. In human format a +// multi-chat send prints one "chat: sync_id" (or "chat: ERROR ..") line per target; +// a single-chat send stays silent on success and surfaces a failure through the +// returned error, preserving the previous CLI behavior. A non-nil error (non-zero +// exit) is returned only when every target failed, mirroring the server's 502 +// all-fail semantics. +func printSendResults(w io.Writer, format string, results []sendCmdResult) error { + okCount := 0 + for _, r := range results { + if r.Error == "" { + okCount++ + } + } + + if format == "json" { + type jsonResult struct { + Chat string `json:"chat"` + SyncID string `json:"sync_id,omitempty"` + } + type jsonError struct { + Chat string `json:"chat"` + Error string `json:"error"` + } + type jsonResponse struct { + OK bool `json:"ok"` + Results []jsonResult `json:"results"` + Errors []jsonError `json:"errors,omitempty"` + } + resp := jsonResponse{OK: okCount > 0, Results: []jsonResult{}} + for _, r := range results { + if r.Error != "" { + resp.Errors = append(resp.Errors, jsonError{Chat: r.Chat, Error: r.Error}) + } else { + resp.Results = append(resp.Results, jsonResult{Chat: r.Chat, SyncID: r.SyncID}) } - client.Token = tok - err = client.Send(context.Background(), sr) } - if err != nil { - return fmt.Errorf("sending: %w", err) + enc := json.NewEncoder(w) + enc.SetIndent("", " ") + if err := enc.Encode(resp); err != nil { + return err + } + } else { + // Multi-chat: one line per chat. Single-chat: silent on success, error + // surfaced via the returned error below (previous behavior). + if len(results) > 1 { + for _, r := range results { + if r.Error != "" { + fmt.Fprintf(w, "%s: ERROR %s\n", r.Chat, r.Error) + } else { + fmt.Fprintf(w, "%s: %s\n", r.Chat, r.SyncID) + } + } } } + if okCount == 0 { + if len(results) == 1 { + return fmt.Errorf("%s", results[0].Error) + } + return fmt.Errorf("all %d chats failed", len(results)) + } return nil } diff --git a/internal/cmd/send_test.go b/internal/cmd/send_test.go index 0293f89..cadd0a8 100644 --- a/internal/cmd/send_test.go +++ b/internal/cmd/send_test.go @@ -426,6 +426,293 @@ bots: } } +// mockBotxSendMulti returns a per-chat sync_id ("sync-") and can be +// told to fail specific group_chat_ids (HTTP 500) to exercise partial/total fan-out. +type mockBotxSendMulti struct { + mu sync.Mutex + chats []string // group_chat_ids received, in call order + failFor map[string]bool + srv *httptest.Server +} + +func newMockBotxSendMulti(failFor map[string]bool) *mockBotxSendMulti { + m := &mockBotxSendMulti{failFor: failFor} + mux := http.NewServeMux() + mux.HandleFunc("POST /api/v4/botx/notifications/direct", func(w http.ResponseWriter, r *http.Request) { + if r.Header.Get("Authorization") != "Bearer test-token" { + w.WriteHeader(http.StatusUnauthorized) + return + } + body, _ := io.ReadAll(r.Body) + var req struct { + GroupChatID string `json:"group_chat_id"` + } + _ = json.Unmarshal(body, &req) + m.mu.Lock() + m.chats = append(m.chats, req.GroupChatID) + m.mu.Unlock() + if m.failFor[req.GroupChatID] { + w.WriteHeader(http.StatusInternalServerError) + fmt.Fprintf(w, `{"status":"error"}`) + return + } + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusAccepted) + fmt.Fprintf(w, `{"status":"ok","result":{"sync_id":"sync-%s"}}`, req.GroupChatID) + }) + m.srv = httptest.NewServer(mux) + return m +} + +func (m *mockBotxSendMulti) close() { m.srv.Close() } + +func (m *mockBotxSendMulti) receivedChats() []string { + m.mu.Lock() + defer m.mu.Unlock() + out := make([]string, len(m.chats)) + copy(out, m.chats) + return out +} + +const ( + chatA = "00000000-0000-0000-0000-00000000000a" + chatB = "00000000-0000-0000-0000-00000000000b" + chatC = "00000000-0000-0000-0000-00000000000c" +) + +func multiSendConfig(t *testing.T, host string) string { + return writeTestConfig(t, fmt.Sprintf(` +bots: + default: + host: %s + id: 00000000-0000-0000-0000-000000000001 + token: test-token +`, host)) +} + +func TestSend_SingleChat_SilentSuccess(t *testing.T) { + mock := newMockBotxSendMulti(nil) + defer mock.close() + + deps, stdout, _ := testDeps() + deps.IsTerminal = true + + err := runSend([]string{ + "--config", multiSendConfig(t, mock.srv.URL), + "--chat-id", chatA, + "hello", + }, deps) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + // Single-chat success stays silent in human output (previous behavior). + if out := stdout.String(); out != "" { + t.Errorf("expected empty stdout for single-chat success, got %q", out) + } + if chats := mock.receivedChats(); len(chats) != 1 || chats[0] != chatA { + t.Errorf("received chats = %v, want [%s]", chats, chatA) + } +} + +func TestSend_SingleChat_Failure(t *testing.T) { + mock := newMockBotxSendMulti(map[string]bool{chatA: true}) // chatA delivery fails + defer mock.close() + + deps, stdout, _ := testDeps() + deps.IsTerminal = true + + err := runSend([]string{ + "--config", multiSendConfig(t, mock.srv.URL), + "--chat-id", chatA, + "hello", + }, deps) + // Single-chat failure surfaces via a non-nil error (non-zero exit), not stdout — + // the legacy CLI contract for one chat. + if err == nil { + t.Fatal("expected error for single-chat delivery failure, got nil") + } + if out := stdout.String(); out != "" { + t.Errorf("expected empty stdout on single-chat failure, got %q", out) + } + if chats := mock.receivedChats(); len(chats) != 1 || chats[0] != chatA { + t.Errorf("received chats = %v, want [%s]", chats, chatA) + } +} + +func TestSend_MultiChat_FanOut(t *testing.T) { + mock := newMockBotxSendMulti(nil) + defer mock.close() + + deps, stdout, _ := testDeps() + deps.IsTerminal = true + + err := runSend([]string{ + "--config", multiSendConfig(t, mock.srv.URL), + "--chat-id", chatA + "," + chatB, + "fanned out", + }, deps) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + chats := mock.receivedChats() + if len(chats) != 2 || chats[0] != chatA || chats[1] != chatB { + t.Fatalf("received chats = %v, want [%s %s]", chats, chatA, chatB) + } + + out := stdout.String() + if !strings.Contains(out, chatA+": sync-"+chatA) { + t.Errorf("missing line for chat A in %q", out) + } + if !strings.Contains(out, chatB+": sync-"+chatB) { + t.Errorf("missing line for chat B in %q", out) + } + if n := strings.Count(strings.TrimSpace(out), "\n"); n != 1 { + t.Errorf("expected 2 output lines, got %d: %q", n+1, out) + } +} + +func TestSend_MultiChat_Dedup(t *testing.T) { + mock := newMockBotxSendMulti(nil) + defer mock.close() + + deps, _, _ := testDeps() + deps.IsTerminal = true + + err := runSend([]string{ + "--config", multiSendConfig(t, mock.srv.URL), + "--chat-id", chatA + " , " + chatA + "," + chatB, + "dedup me", + }, deps) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if chats := mock.receivedChats(); len(chats) != 2 { + t.Errorf("expected 2 deduped sends, got %d: %v", len(chats), chats) + } +} + +func TestSend_MultiChat_PartialFail(t *testing.T) { + mock := newMockBotxSendMulti(map[string]bool{chatB: true}) + defer mock.close() + + deps, stdout, _ := testDeps() + deps.IsTerminal = true + + err := runSend([]string{ + "--config", multiSendConfig(t, mock.srv.URL), + "--chat-id", chatA + "," + chatB, + "partial", + }, deps) + // At least one chat delivered -> exit code zero (best-effort). + if err != nil { + t.Fatalf("partial failure should not fail the command, got: %v", err) + } + + out := stdout.String() + if !strings.Contains(out, chatA+": sync-"+chatA) { + t.Errorf("missing success line for chat A in %q", out) + } + if !strings.Contains(out, chatB+": ERROR") { + t.Errorf("missing error line for chat B in %q", out) + } +} + +func TestSend_MultiChat_AllFail(t *testing.T) { + mock := newMockBotxSendMulti(map[string]bool{chatA: true, chatB: true}) + defer mock.close() + + deps, stdout, _ := testDeps() + deps.IsTerminal = true + + err := runSend([]string{ + "--config", multiSendConfig(t, mock.srv.URL), + "--chat-id", chatA + "," + chatB, + "all fail", + }, deps) + // Every chat failed -> non-zero exit (returned error). + if err == nil { + t.Fatal("expected error when all chats fail") + } + + out := stdout.String() + if !strings.Contains(out, chatA+": ERROR") || !strings.Contains(out, chatB+": ERROR") { + t.Errorf("expected error lines for both chats in %q", out) + } +} + +func TestSend_MultiChat_JSONOutput(t *testing.T) { + mock := newMockBotxSendMulti(map[string]bool{chatB: true}) + defer mock.close() + + deps, stdout, _ := testDeps() + deps.IsTerminal = true + + err := runSend([]string{ + "--config", multiSendConfig(t, mock.srv.URL), + "--chat-id", chatA + "," + chatB, + "--format", "json", + "json multi", + }, deps) + // Partial failure still exits zero, but emits the uniform response body. + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var resp struct { + OK bool `json:"ok"` + Results []struct { + Chat string `json:"chat"` + SyncID string `json:"sync_id"` + } `json:"results"` + Errors []struct { + Chat string `json:"chat"` + Error string `json:"error"` + } `json:"errors"` + } + if err := json.Unmarshal(stdout.Bytes(), &resp); err != nil { + t.Fatalf("invalid JSON output: %v\nraw: %s", err, stdout.String()) + } + if !resp.OK { + t.Errorf("ok = false, want true (chat A delivered)") + } + if len(resp.Results) != 1 || resp.Results[0].Chat != chatA || resp.Results[0].SyncID != "sync-"+chatA { + t.Errorf("results = %+v, want single success for chat A", resp.Results) + } + if len(resp.Errors) != 1 || resp.Errors[0].Chat != chatB { + t.Errorf("errors = %+v, want single error for chat B", resp.Errors) + } +} + +func TestSend_MultiChat_UnknownAliasPerChat(t *testing.T) { + mock := newMockBotxSendMulti(nil) + defer mock.close() + + // chatA is a valid UUID; "nope" is an unknown alias -> per-chat resolve error, + // but chatA still delivers (best-effort), so exit code stays zero. + deps, stdout, _ := testDeps() + deps.IsTerminal = true + + err := runSend([]string{ + "--config", multiSendConfig(t, mock.srv.URL), + "--chat-id", chatA + ",nope", + "mixed", + }, deps) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if chats := mock.receivedChats(); len(chats) != 1 || chats[0] != chatA { + t.Errorf("received chats = %v, want only [%s]", chats, chatA) + } + out := stdout.String() + if !strings.Contains(out, chatA+": sync-"+chatA) { + t.Errorf("missing success line for chat A in %q", out) + } + if !strings.Contains(out, "nope: ERROR") { + t.Errorf("missing resolve-error line for alias nope in %q", out) + } +} + func TestSend_MentionsNotArray(t *testing.T) { deps, _, _ := testDeps() deps.IsTerminal = true diff --git a/internal/cmd/serve_integration_test.go b/internal/cmd/serve_integration_test.go index 61eef11..6cc0e02 100644 --- a/internal/cmd/serve_integration_test.go +++ b/internal/cmd/serve_integration_test.go @@ -212,6 +212,24 @@ func doPost(t *testing.T, url, apiKey, body string) (int, map[string]any) { return resp.StatusCode, result } +// firstChatError returns the error string of the first per-chat error in a +// MultiSendResponse body ({"ok":false,"errors":[{"chat","error"}]}). Chat/bot +// resolution now fails per-chat (HTTP 502) rather than request-level (400), so +// integration tests read the failure from errors[] instead of the top-level +// "error" field. +func firstChatError(resp map[string]any) string { + errs, ok := resp["errors"].([]any) + if !ok || len(errs) == 0 { + return "" + } + e, ok := errs[0].(map[string]any) + if !ok { + return "" + } + msg, _ := e["error"].(string) + return msg +} + // --- integration tests --- func TestServeIntegration_SingleBot_Send(t *testing.T) { @@ -335,13 +353,12 @@ server: baseURL := fmt.Sprintf("http://%s/api/v1", listenAddr) - // Without bot — should fail + // Without bot — per-chat resolution failure (502 with errors[]). code, resp := doPost(t, baseURL+"/send", "test-key", `{"chat_id":"c0000000-0000-0000-0000-000000000003","message":"hi"}`) - if code != 400 { - t.Fatalf("expected 400 without bot, got %d: %v", code, resp) + if code != 502 { + t.Fatalf("expected 502 without bot, got %d: %v", code, resp) } - errMsg, _ := resp["error"].(string) - if !strings.Contains(errMsg, "bot is required") { + if errMsg := firstChatError(resp); !strings.Contains(errMsg, "bot is required") { t.Errorf("expected 'bot is required', got %q", errMsg) } @@ -351,10 +368,10 @@ server: t.Fatalf("expected 200 with bot=prod, got %d: %v", code, resp) } - // With unknown bot + // With unknown bot — per-chat resolution failure (502). code, resp = doPost(t, baseURL+"/send", "test-key", `{"bot":"staging","chat_id":"c0000000-0000-0000-0000-000000000003","message":"hi"}`) - if code != 400 { - t.Fatalf("expected 400 for unknown bot, got %d: %v", code, resp) + if code != 502 { + t.Fatalf("expected 502 for unknown bot, got %d: %v", code, resp) } calls := mock.getCalls() @@ -400,10 +417,11 @@ server: baseURL := fmt.Sprintf("http://%s/api/v1", listenAddr) alertPayload := `{"version":"4","groupKey":"g","status":"firing","receiver":"x","groupLabels":{"alertname":"Test"},"alerts":[{"status":"firing","labels":{"alertname":"HighCPU","severity":"critical","instance":"web-01"},"annotations":{"summary":"CPU high"},"startsAt":"2026-01-01T00:00:00Z"}]}` - // Without ?bot= — should fail + // Without ?bot= — bot resolution is a per-chat delivery outcome now, so an + // ambiguous bot fails the single target and surfaces as 502 (unified contract). code, resp := doPost(t, baseURL+"/alertmanager", "test-key", alertPayload) - if code != 400 { - t.Fatalf("expected 400 without bot, got %d: %v", code, resp) + if code != 502 { + t.Fatalf("expected 502 without bot, got %d: %v", code, resp) } // With ?bot=prod @@ -455,10 +473,11 @@ server: baseURL := fmt.Sprintf("http://%s/api/v1", listenAddr) grafanaPayload := `{"version":"1","groupKey":"g","status":"firing","state":"alerting","title":"[FIRING] Test","receiver":"x","orgId":1,"groupLabels":{"alertname":"Test"},"alerts":[{"status":"firing","labels":{"alertname":"DiskFull","grafana_folder":"Prod"},"annotations":{"summary":"Disk full"},"startsAt":"2026-01-01T00:00:00Z"}]}` - // Without ?bot= — should fail + // Without ?bot= — bot resolution is a per-chat delivery outcome now, so an + // ambiguous bot fails the single target and surfaces as 502 (unified contract). code, resp := doPost(t, baseURL+"/grafana", "test-key", grafanaPayload) - if code != 400 { - t.Fatalf("expected 400 without bot, got %d: %v", code, resp) + if code != 502 { + t.Fatalf("expected 502 without bot, got %d: %v", code, resp) } // With ?bot=test @@ -515,10 +534,13 @@ server: t.Errorf("expected resolved UUID, got %q", calls[0].GroupChatID) } - // Unknown alias — 400 + // Unknown alias — per-chat resolution failure (502 with errors[]). code, resp := doPost(t, baseURL+"/send", "test-key", `{"chat_id":"unknown-alias","message":"hi"}`) - if code != 400 { - t.Fatalf("expected 400 for unknown alias, got %d: %v", code, resp) + if code != 502 { + t.Fatalf("expected 502 for unknown alias, got %d: %v", code, resp) + } + if errMsg := firstChatError(resp); !strings.Contains(errMsg, "unknown chat alias") { + t.Errorf("expected 'unknown chat alias', got %q", errMsg) } // Raw UUID passes through @@ -624,10 +646,13 @@ server: t.Fatalf("expected 200 for chat-bound bot, got %d: %v", code, resp) } - // 2. Chat without bound bot — "bot" is required + // 2. Chat without bound bot — "bot" is required (per-chat failure, 502). code, resp = doPost(t, baseURL+"/send", "test-key", `{"chat_id":"general","message":"hi"}`) - if code != 400 { - t.Fatalf("expected 400 for unbound chat without bot, got %d: %v", code, resp) + if code != 502 { + t.Fatalf("expected 502 for unbound chat without bot, got %d: %v", code, resp) + } + if errMsg := firstChatError(resp); !strings.Contains(errMsg, "bot is required") { + t.Errorf("expected 'bot is required', got %q", errMsg) } // 3. Explicit "bot" overrides chat binding diff --git a/internal/config/config.go b/internal/config/config.go index c5f6a50..b41a01d 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -675,6 +675,31 @@ func IsUUID(s string) bool { return uuidRe.MatchString(s) } +// ResolveChatAlias resolves a single chat value to its UUID without mutating the +// config. A UUID passes through unchanged; an alias is looked up in the Chats map. +// Fan-out send paths use it to resolve several comma-separated chats independently, +// so a bad alias fails per chat rather than aborting the whole command. +func (c *Config) ResolveChatAlias(chat string) (string, error) { + if chat == "" { + return "", fmt.Errorf("chat is required") + } + if IsUUID(chat) { + return chat, nil + } + if cc, ok := c.Chats[chat]; ok { + return cc.ID, nil + } + names := make([]string, 0, len(c.Chats)) + for k := range c.Chats { + names = append(names, k) + } + sort.Strings(names) + if len(names) == 0 { + return "", fmt.Errorf("unknown chat %q (no aliases configured)", chat) + } + return "", fmt.Errorf("unknown chat alias %q, available: %s", chat, strings.Join(names, ", ")) +} + // ResolveChatID resolves ChatID: if it looks like a UUID, use as-is; // otherwise look it up in the Chats alias map. func (c *Config) ResolveChatID() error { diff --git a/internal/server/api/openapi.yaml b/internal/server/api/openapi.yaml index 399c4b5..1dac3e2 100644 --- a/internal/server/api/openapi.yaml +++ b/internal/server/api/openapi.yaml @@ -116,10 +116,30 @@ paths: operationId: send summary: Send a message description: | - Send a text message and/or file to an eXpress chat. + Send a text message and/or file to one or more eXpress chats. Supports JSON and multipart/form-data content types. At least one of `message` or `file` is required. + **Multi-chat fan-out.** `chat_id` may list several chats separated by + commas (`chat_id=a,b,c`). The message is delivered best-effort to every + listed chat; duplicates are collapsed, order is preserved. Chat and bot + are resolved per target, and inline `@mention[...]` tokens are parsed with + each target bot's own resolver. + + **⚠️ Breaking change (response format).** The response is now always a + `MultiSendResponse` — a `results[]`/`errors[]` envelope — even for a single + chat (previously `{"ok":true,"sync_id":"..."}`). Per-chat success carries + `sync_id` (sync) or `request_id`+`queued:true` (async). HTTP status is + `200` when at least one sync delivery succeeded, `202` for async enqueue, + `502` when delivery to every chat failed. Request-level failures (bad JSON, + empty `chat_id`, invalid `status`, unsupported media type) keep the old + `{"ok":false,"error":"..."}` form with `400`/`415` and are NOT reported in + `errors[]`. + + **Async expand.** In async mode (`serve --enqueue`) a multi-chat `chat_id` + is expanded into N independent queue messages (one per chat) so each chat + retries/acks on its own without duplicates; the worker is unchanged. + `chat_id` is optional when a default chat is configured (`default: true` in chats section). In multi-bot mode, `bot` is required unless the chat alias has a default bot binding in config. @@ -214,13 +234,44 @@ paths: description: JSON string containing an array of mentions in BotX API wire format responses: "200": - description: Message sent + description: | + Delivered synchronously to at least one chat. Body is a + `MultiSendResponse`; each `results` entry carries a `sync_id`, and any + per-chat failures are listed in `errors`. + content: + application/json: + schema: + $ref: "#/components/schemas/MultiSendResponse" + example: + ok: true + results: + - chat: deploy + sync_id: "a1b2c3d4-e5f6-7890-abcd-ef1234567890" + errors: + - chat: missing-alias + error: "resolving chat: unknown chat alias" + "202": + description: | + Accepted for asynchronous delivery (`serve --enqueue`). A multi-chat + `chat_id` is expanded into one queued message per chat; each `results` + entry carries `request_id` and `queued:true`. content: application/json: schema: - $ref: "#/components/schemas/SuccessResponse" + $ref: "#/components/schemas/MultiSendResponse" + example: + ok: true + results: + - chat: a + request_id: "0d6d7f87-0a2f-4c5b-b0d4-4d0b705a77e2" + queued: true + - chat: b + request_id: "1e7e8f98-1b3f-5d6c-c1e5-5e1c816b88f3" + queued: true "400": - description: Invalid request + description: | + Request-level error (empty `chat_id`, missing body, invalid `status`, + malformed JSON). Keeps the `{"ok":false,"error":"..."}` form. content: application/json: schema: @@ -253,11 +304,18 @@ paths: schema: $ref: "#/components/schemas/ErrorResponse" "502": - description: Upstream error (eXpress API) + description: | + Delivery to every listed chat failed. Body is a `MultiSendResponse` + with `ok:false` and one `errors` entry per chat. content: application/json: schema: - $ref: "#/components/schemas/ErrorResponse" + $ref: "#/components/schemas/MultiSendResponse" + example: + ok: false + errors: + - chat: a + error: "upstream error: 502 Bad Gateway" /alertmanager: post: @@ -267,15 +325,20 @@ paths: Receives webhook notifications from Prometheus Alertmanager. Renders alerts through a Go template and sends the result to eXpress. + **Multi-chat fan-out.** `?chat_id=` may list several chats separated by + commas (`?chat_id=a,b,c`); the rendered alert is delivered best-effort to + each. The response is always a `MultiSendResponse` (see the breaking-change + note on `/send`), even for a single or default chat. + Chat resolution priority: - 1. `?chat_id=` query parameter + 1. `?chat_id=` query parameter (comma-separated for fan-out) 2. `default_chat_id` from webhook config 3. Global default chat (`default: true` in chats section) 4. Single chat alias from config parameters: - name: chat_id in: query - description: Target chat UUID or alias (overrides config default) + description: Target chat UUID or alias; comma-separated for multi-chat fan-out (overrides config default) schema: type: string - name: bot @@ -315,13 +378,15 @@ paths: generatorURL: http://prometheus:9090/graph responses: "200": - description: Alert processed and sent + description: | + Alert delivered to at least one chat. Body is a `MultiSendResponse` + (`results` with a per-chat `sync_id`; partial failures in `errors`). content: application/json: schema: - $ref: "#/components/schemas/SuccessResponse" + $ref: "#/components/schemas/MultiSendResponse" "400": - description: Invalid request + description: Request-level error (malformed JSON, no alerts, empty chat_id, template error) content: application/json: schema: @@ -339,11 +404,11 @@ paths: schema: $ref: "#/components/schemas/ErrorResponse" "502": - description: Upstream error + description: Delivery to every listed chat failed (`MultiSendResponse` with `ok:false`) content: application/json: schema: - $ref: "#/components/schemas/ErrorResponse" + $ref: "#/components/schemas/MultiSendResponse" /command: post: @@ -456,15 +521,20 @@ paths: Receives webhook notifications from Grafana Alerting. Renders alerts through a Go template and sends the result to eXpress. + **Multi-chat fan-out.** `?chat_id=` may list several chats separated by + commas (`?chat_id=a,b,c`); the rendered alert is delivered best-effort to + each. The response is always a `MultiSendResponse` (see the breaking-change + note on `/send`), even for a single or default chat. + Chat resolution priority: - 1. `?chat_id=` query parameter + 1. `?chat_id=` query parameter (comma-separated for fan-out) 2. `default_chat_id` from webhook config 3. Global default chat (`default: true` in chats section) 4. Single chat alias from config parameters: - name: chat_id in: query - description: Target chat UUID or alias (overrides config default) + description: Target chat UUID or alias; comma-separated for multi-chat fan-out (overrides config default) schema: type: string - name: bot @@ -512,13 +582,15 @@ paths: message: "**Firing**\n\nValue: B=95.2" responses: "200": - description: Alert processed and sent + description: | + Alert delivered to at least one chat. Body is a `MultiSendResponse` + (`results` with a per-chat `sync_id`; partial failures in `errors`). content: application/json: schema: - $ref: "#/components/schemas/SuccessResponse" + $ref: "#/components/schemas/MultiSendResponse" "400": - description: Invalid request + description: Request-level error (malformed JSON, no alerts, empty chat_id, template error) content: application/json: schema: @@ -536,11 +608,11 @@ paths: schema: $ref: "#/components/schemas/ErrorResponse" "502": - description: Upstream error + description: Delivery to every listed chat failed (`MultiSendResponse` with `ok:false`) content: application/json: schema: - $ref: "#/components/schemas/ErrorResponse" + $ref: "#/components/schemas/MultiSendResponse" /gitlab: post: @@ -582,7 +654,7 @@ paths: `/regex/` patterns (the `event` selector uses event-key matching instead). When no rule matches, delivery falls back to the single default chat, and if none is configured the event is ignored (`200`). Fan-out is - best-effort: the response is `200` with a `GitlabFanoutResponse` when at + best-effort: the response is `200` with a `MultiSendResponse` when at least one delivery succeeds (partial failures are listed in `errors`), or `502` when they all fail. @@ -592,14 +664,15 @@ paths: (multi-tenant): besides the default `server.gitlab.secret`, each entry of `server.gitlab.senders` defines its own token bound to an isolated set of chats. A request authenticated with a sender token is delivered - **only** to that sender's chats (fan-out, `GitlabFanoutResponse`); + **only** to that sender's chats (fan-out, `MultiSendResponse`); `?chat_id=`, `?bot=`, `routes` and `default_chat_id` are ignored for it, while the `events` filter, templates and `error_events` apply as usual. The response behaviour is otherwise unchanged; any token that matches neither the default secret nor a sender secret gets `401`. Chat resolution priority (default-token requests): - 1. `?chat_id=` query parameter (overrides routing entirely) + 1. `?chat_id=` query parameter (overrides routing entirely; + comma-separated for multi-chat fan-out) 2. `server.gitlab.routes` (all matching rules, unioned + de-duplicated) 3. `default_chat_id` from webhook config 4. Global default chat (`default: true` in chats section) @@ -607,12 +680,17 @@ paths: Sender-token requests skip this list: the targets are always the sender's configured chats. + + **⚠️ Breaking change (response format).** Every delivered event now returns + a `MultiSendResponse` (`results`/`errors` envelope), including single-chat + `?chat_id=` and single default-chat delivery (previously a bare + `SuccessResponse`). Filtered-out events still return `GitlabIgnoredResponse`. security: - gitlabToken: [] parameters: - name: chat_id in: query - description: Target chat UUID or alias (overrides config default) + description: Target chat UUID or alias; comma-separated for multi-chat fan-out (overrides config default) schema: type: string - name: bot @@ -645,12 +723,10 @@ paths: "200": description: | Event processed. The body depends on how the event was delivered: - - Single-chat delivery (`?chat_id=`, or no `routes` configured): - a `SuccessResponse` (`ok` + `sync_id`). - - Fan-out via `routes` or a sender-token match - (`server.gitlab.senders`): a `GitlabFanoutResponse` with a - `results` entry per successful chat and an `errors` entry per - failed chat (`ok:true` when at least one delivery succeeded). + - Delivered to at least one chat (single `?chat_id=`, `routes` + fan-out, or a sender-token match on `server.gitlab.senders`): a + `MultiSendResponse` with a `results` entry per successful chat and + an `errors` entry per failed chat (`ok:true`). - Filtered out by `only`/`exclude`, or matched no route with no default chat: a `GitlabIgnoredResponse` (`ok:true`, `ignored:true`, `event:`) and no message is sent. @@ -658,8 +734,7 @@ paths: application/json: schema: oneOf: - - $ref: "#/components/schemas/SuccessResponse" - - $ref: "#/components/schemas/GitlabFanoutResponse" + - $ref: "#/components/schemas/MultiSendResponse" - $ref: "#/components/schemas/GitlabIgnoredResponse" "400": description: Invalid request (malformed JSON or template error) @@ -675,16 +750,12 @@ paths: $ref: "#/components/schemas/ErrorResponse" "502": description: | - Upstream error. For single-chat delivery the body is an - `ErrorResponse`. For a `routes` or sender-token fan-out where - every target chat failed, the body is a `GitlabFanoutResponse` + Delivery to every target chat failed. Body is a `MultiSendResponse` (`ok:false`) whose `errors` list the per-chat failures. content: application/json: schema: - oneOf: - - $ref: "#/components/schemas/ErrorResponse" - - $ref: "#/components/schemas/GitlabFanoutResponse" + $ref: "#/components/schemas/MultiSendResponse" components: securitySchemes: @@ -1050,15 +1121,17 @@ components: description: The derived event key that was filtered out. example: "merge_request.update" - GitlabFanoutResponse: + MultiSendResponse: type: object description: | - Returned by /gitlab when `server.gitlab.routes` or a sender-token match - (`server.gitlab.senders`) fans an event out to one - or more chats. `ok` is true when at least one delivery succeeded (HTTP - 200) and false when they all failed (HTTP 502). `results` lists each - successful delivery and `errors` each failed one; on a partial failure - both are present. + The uniform multi-chat fan-out response shared by `/send`, `/alertmanager`, + `/grafana` and `/gitlab`. `ok` is true when at least one chat received the + message (HTTP `200` sync / `202` async) and false when delivery to every + chat failed (HTTP `502`). `results` lists each successful delivery and + `errors` each failed one; on a partial failure both are present. Order + follows the requested chats. Request-level failures (bad JSON, empty + `chat_id`, invalid status, unsupported media) are NOT reported here — they + keep the `{"ok":false,"error":"..."}` form with `400`/`415`. properties: ok: type: boolean @@ -1071,11 +1144,20 @@ components: properties: chat: type: string - description: Target chat alias or UUID as configured in the rule. + description: Target chat alias or UUID as requested. example: backend-mrs sync_id: type: string + description: Synchronous delivery id (present for sync sends). example: "a1b2c3d4-e5f6-7890-abcd-ef1234567890" + request_id: + type: string + description: Enqueue request id (present for async/enqueue sends). + example: "0d6d7f87-0a2f-4c5b-b0d4-4d0b705a77e2" + queued: + type: boolean + description: True when the message was queued (async/enqueue). + example: true errors: type: array description: Failed deliveries, one per chat (chat/bot resolution or upstream send). diff --git a/internal/server/handler_alertmanager.go b/internal/server/handler_alertmanager.go index bf613fc..2d81b13 100644 --- a/internal/server/handler_alertmanager.go +++ b/internal/server/handler_alertmanager.go @@ -77,53 +77,45 @@ func (s *Server) handleAlertmanager(w http.ResponseWriter, r *http.Request) { // Determine status status := s.resolveAlertStatus(webhook) - // Resolve chat: query param > default_chat_id > global default chat > single chat from config - targetChat := s.amCfg.DefaultChatID - if targetChat == "" { - targetChat = s.cfg.DefaultChatAlias - } - if targetChat == "" { - targetChat = s.amCfg.FallbackChatID - } - if q := r.URL.Query().Get("chat_id"); q != "" { - targetChat = q + // Resolve target chats: ?chat_id (now comma-separated, fan-out) > default_chat_id + // > global default chat > single chat from config. With no ?chat_id the endpoint + // keeps its single-default behaviour; the response is the uniform + // MultiSendResponse in every case (results[0] for a single chat). + targets := parseChatIDs(r.URL.Query().Get("chat_id")) + if len(targets) == 0 { + if single := s.amCfg.singleChat(s.cfg.DefaultChatAlias); single != "" { + targets = []string{single} + } } - if targetChat == "" { + if len(targets) == 0 { writeError(w, http.StatusBadRequest, "chat_id is required: set default_chat_id in config, configure a single chat alias, or pass ?chat_id=") return } - chatResult, err := s.chats(targetChat) - if err != nil { - writeError(w, http.StatusBadRequest, "resolving chat: "+err.Error()) - return - } - - // Resolve bot: explicit ?bot= > chat-bound bot > auth bot - botName, errMsg := s.resolveRequestBot(r.Context(), r.URL.Query().Get("bot"), chatResult.Bot) - if errMsg != "" { - writeError(w, http.StatusBadRequest, errMsg) - return - } start := time.Now() - syncID, err := s.send(r.Context(), &SendPayload{ - Bot: botName, - ChatID: chatResult.ChatID, - Message: message, - Status: status, - }) + results, errs := s.fanoutSend(r.Context(), targets, r.URL.Query().Get("bot"), message, status) elapsed := time.Since(start) keyName := KeyName(r.Context()) - if err != nil { - vlog.V1("alertmanager: send failed [key: %s] -> 502 (%dms)", keyName, elapsed.Milliseconds()) - writeError(w, http.StatusBadGateway, "upstream error: "+err.Error()) - return + if len(results) == 0 { + vlog.V1("alertmanager: fan-out to %d chats all failed [key: %s] -> 502 (%dms)", len(targets), keyName, elapsed.Milliseconds()) + } else { + vlog.V1("alertmanager: sent %s to %d/%d chats [key: %s] (%dms)", webhook.Status, len(results), len(targets), keyName, elapsed.Milliseconds()) } + writeMultiSend(w, results, errs, http.StatusOK) +} - vlog.V1("alertmanager: sent %s to %s [key: %s] -> 200 (%dms)", webhook.Status, targetChat, keyName, elapsed.Milliseconds()) - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(sendResponse{OK: true, SyncID: syncID}) +// singleChat returns the fallback delivery chat for alertmanager, following the +// precedence default_chat_id -> global default chat -> the sole configured chat +// alias. It is empty when none is configured. +func (c *AlertmanagerConfig) singleChat(globalDefault string) string { + if c.DefaultChatID != "" { + return c.DefaultChatID + } + if globalDefault != "" { + return globalDefault + } + return c.FallbackChatID } func (s *Server) resolveAlertStatus(webhook AlertmanagerWebhook) string { diff --git a/internal/server/handler_gitlab.go b/internal/server/handler_gitlab.go index 84a921c..f152cbe 100644 --- a/internal/server/handler_gitlab.go +++ b/internal/server/handler_gitlab.go @@ -310,24 +310,27 @@ func (s *Server) handleGitlab(w http.ResponseWriter, r *http.Request) { // (team isolation): ?chat_id, ?bot, Routes and DefaultChatID do not apply. // The filter/template/status logic above is shared with the default path. if isSender { - s.gitlabFanout(w, r, "", senderChats, message, status, view.EventKey) + s.gitlabDeliver(w, r, "", senderChats, message, status, view.EventKey) return } - // An explicit ?chat_id overrides routing entirely; likewise, with no routes - // configured the endpoint keeps its original single-chat behaviour (routes is - // optional, so its absence must not change existing deployments). - queryChat := r.URL.Query().Get("chat_id") - if queryChat != "" || len(s.gitCfg.Routes) == 0 { - targetChat := queryChat - if targetChat == "" { - targetChat = s.singleGitlabChat() + // An explicit ?chat_id overrides routing entirely (and may itself list several + // chats, comma-separated); likewise, with no routes configured the endpoint + // keeps its single-chat default behaviour (routes is optional, so its absence + // must not change existing deployments). + queryChats := parseChatIDs(r.URL.Query().Get("chat_id")) + if len(queryChats) > 0 || len(s.gitCfg.Routes) == 0 { + targets := queryChats + if len(targets) == 0 { + if single := s.singleGitlabChat(); single != "" { + targets = []string{single} + } } - if targetChat == "" { + if len(targets) == 0 { writeError(w, http.StatusBadRequest, "chat_id is required: set default_chat_id in config, configure a single chat alias, or pass ?chat_id=") return } - s.gitlabSendSingle(w, r, targetChat, message, status, view.EventKey) + s.gitlabDeliver(w, r, r.URL.Query().Get("bot"), targets, message, status, view.EventKey) return } @@ -345,7 +348,7 @@ func (s *Server) handleGitlab(w http.ResponseWriter, r *http.Request) { return } } - s.gitlabFanout(w, r, r.URL.Query().Get("bot"), targets, message, status, view.EventKey) + s.gitlabDeliver(w, r, r.URL.Query().Get("bot"), targets, message, status, view.EventKey) } // resolveGitlabAuth authenticates an incoming X-Gitlab-Token value against the @@ -399,84 +402,25 @@ func (s *Server) singleGitlabChat() string { return s.gitCfg.FallbackChatID } -// gitlabSendSingle delivers a rendered event to exactly one chat, preserving the -// endpoint's original response shape (sendResponse) and status codes: 400 on a -// chat/bot resolution error, 502 on an upstream send failure, and 200 with the -// sync_id on success. It backs the ?chat_id override and the no-routes default. -func (s *Server) gitlabSendSingle(w http.ResponseWriter, r *http.Request, targetChat, message, status, eventKey string) { - chatResult, err := s.chats(targetChat) - if err != nil { - writeError(w, http.StatusBadRequest, "resolving chat: "+err.Error()) - return - } - // Resolve bot: explicit ?bot= > chat-bound bot > auth bot. - botName, errMsg := s.resolveRequestBot(r.Context(), r.URL.Query().Get("bot"), chatResult.Bot) - if errMsg != "" { - writeError(w, http.StatusBadRequest, errMsg) - return - } - start := time.Now() - syncID, err := s.send(r.Context(), &SendPayload{ - Bot: botName, - ChatID: chatResult.ChatID, - Message: message, - Status: status, - }) - elapsed := time.Since(start) - if err != nil { - vlog.V1("gitlab: send failed -> 502 (%dms)", elapsed.Milliseconds()) - writeError(w, http.StatusBadGateway, "upstream error: "+err.Error()) - return - } - vlog.V1("gitlab: sent %s to %s -> 200 (%dms)", eventKey, targetChat, elapsed.Milliseconds()) - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(sendResponse{OK: true, SyncID: syncID}) -} - -// gitlabFanout delivers a rendered event to every target chat best-effort, -// resolving chat and bot per target and collecting successes and failures -// independently. It responds 200 with the results (plus any partial errors) when -// at least one delivery succeeds, or 502 with the errors when they all fail. -// requestBot is the ?bot= override; the sender-isolated path passes "" so a -// sender token cannot pick another configured bot's identity. -func (s *Server) gitlabFanout(w http.ResponseWriter, r *http.Request, requestBot string, targets []string, message, status, eventKey string) { - var results []gitlabFanoutResult - var errs []gitlabFanoutError +// gitlabDeliver delivers a rendered event to every target chat best-effort using +// the project-wide fan-out primitives (fanout + writeMultiSend). Chat and bot are +// resolved per target and successes/failures are collected independently; the +// response is always a MultiSendResponse — 200 with the results (plus any partial +// errors) when at least one delivery succeeds, or 502 with the errors when they +// all fail. A single target still returns the uniform shape (results[0]), so +// /gitlab shares the contract of every other send surface. requestBot is the +// ?bot= override; the sender-isolated path passes "" so a sender token cannot +// pick another configured bot's identity. +func (s *Server) gitlabDeliver(w http.ResponseWriter, r *http.Request, requestBot string, targets []string, message, status, eventKey string) { start := time.Now() - for _, target := range targets { - chatResult, err := s.chats(target) - if err != nil { - errs = append(errs, gitlabFanoutError{Chat: target, Error: "resolving chat: " + err.Error()}) - continue - } - botName, errMsg := s.resolveRequestBot(r.Context(), requestBot, chatResult.Bot) - if errMsg != "" { - errs = append(errs, gitlabFanoutError{Chat: target, Error: errMsg}) - continue - } - syncID, err := s.send(r.Context(), &SendPayload{ - Bot: botName, - ChatID: chatResult.ChatID, - Message: message, - Status: status, - }) - if err != nil { - errs = append(errs, gitlabFanoutError{Chat: target, Error: err.Error()}) - continue - } - results = append(results, gitlabFanoutResult{Chat: target, SyncID: syncID}) - } + results, errs := s.fanoutSend(r.Context(), targets, requestBot, message, status) elapsed := time.Since(start) - - w.Header().Set("Content-Type", "application/json") if len(results) == 0 { vlog.V1("gitlab: %s fan-out to %d chats all failed -> 502 (%dms)", eventKey, len(targets), elapsed.Milliseconds()) - w.WriteHeader(http.StatusBadGateway) - json.NewEncoder(w).Encode(gitlabFanoutResponse{OK: false, Errors: errs}) - return + } else { + vlog.V1("gitlab: %s delivered to %d/%d chats (%dms)", eventKey, len(results), len(targets), elapsed.Milliseconds()) } - vlog.V1("gitlab: %s fan-out delivered to %d/%d chats -> 200 (%dms)", eventKey, len(results), len(targets), elapsed.Milliseconds()) - json.NewEncoder(w).Encode(gitlabFanoutResponse{OK: true, Results: results, Errors: errs}) + writeMultiSend(w, results, errs, http.StatusOK) } // gitlabIgnoredResponse is returned with 200 OK when an event is filtered out @@ -487,30 +431,6 @@ type gitlabIgnoredResponse struct { Event string `json:"event"` } -// gitlabFanoutResponse is the routing endpoint's response when routes are -// configured: a best-effort fan-out that reports each successful delivery in -// results and each failed one in errors. OK is true when at least one delivery -// succeeded (HTTP 200); it is false when they all failed (HTTP 502). -type gitlabFanoutResponse struct { - OK bool `json:"ok"` - Results []gitlabFanoutResult `json:"results,omitempty"` - Errors []gitlabFanoutError `json:"errors,omitempty"` -} - -// gitlabFanoutResult is a single successful fan-out delivery: the target chat -// (alias or UUID as configured in the rule) and the BotX sync_id. -type gitlabFanoutResult struct { - Chat string `json:"chat"` - SyncID string `json:"sync_id"` -} - -// gitlabFanoutError is a single failed fan-out delivery: the target chat and the -// error that prevented delivery (chat/bot resolution or the upstream send). -type gitlabFanoutError struct { - Chat string `json:"chat"` - Error string `json:"error"` -} - // DefaultGitlabTemplate is the generic fallback that renders any GitLab event // for which no more specific template exists. It is registered under the // "default" key of DefaultGitlabTemplates. diff --git a/internal/server/handler_gitlab_test.go b/internal/server/handler_gitlab_test.go index 78767ee..37c1009 100644 --- a/internal/server/handler_gitlab_test.go +++ b/internal/server/handler_gitlab_test.go @@ -502,15 +502,48 @@ func TestGitlab_ChatOverride(t *testing.T) { } } +// TestGitlab_MultiChatOverride: ?chat_id may itself list several chats +// (comma-separated) and fans the event out to each, deduplicating repeats. +func TestGitlab_MultiChatOverride(t *testing.T) { + srv, cap := newGitlabTestServer(t, &GitlabConfig{DefaultChatID: "chat1", SecretToken: "secret"}) + w := doRequest(srv, "POST", "/api/v1/gitlab?chat_id=chatA,+chatB+,chatA", strings.NewReader(mrOpenPayload), gitlabHeaders("secret")) + if w.Code != 200 { + t.Fatalf("status = %d, body: %s", w.Code, w.Body.String()) + } + if cap.count() != 2 { + t.Fatalf("send count = %d, want 2 (deduped fan-out)", cap.count()) + } + var resp MultiSendResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) + } + if !resp.OK || len(resp.Results) != 2 || len(resp.Errors) != 0 { + t.Fatalf("response = %+v, want ok with 2 results, 0 errors", resp) + } + if resp.Results[0].Chat != "chatA" || resp.Results[1].Chat != "chatB" { + t.Errorf("results = %+v, want chatA then chatB in order", resp.Results) + } +} + func TestGitlab_ChatResolveError(t *testing.T) { + // A chat that fails to resolve is a per-chat delivery outcome, not a + // request-level error: with the single default chat unresolvable, every + // target fails -> 502 with the error in errors[] (unified contract). srv, cap := newGitlabTestServer(t, &GitlabConfig{DefaultChatID: "unknown-alias", SecretToken: "secret"}) w := doRequest(srv, "POST", "/api/v1/gitlab", strings.NewReader(mrOpenPayload), gitlabHeaders("secret")) - if w.Code != 400 { - t.Fatalf("status = %d, want 400 (body: %s)", w.Code, w.Body.String()) + if w.Code != 502 { + t.Fatalf("status = %d, want 502 (body: %s)", w.Code, w.Body.String()) } if cap.count() != 0 { t.Errorf("send count = %d, want 0", cap.count()) } + var resp MultiSendResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) + } + if resp.OK || len(resp.Results) != 0 || len(resp.Errors) != 1 || resp.Errors[0].Chat != "unknown-alias" { + t.Errorf("response = %+v, want not-ok with single unknown-alias error", resp) + } } func TestGitlab_MissingChatID(t *testing.T) { @@ -1003,13 +1036,13 @@ func TestGitlab_QueryChatBypassesRoutes(t *testing.T) { if cap.last().ChatID != "override" { t.Errorf("chat = %q, want override", cap.last().ChatID) } - // Single-chat path keeps the plain sendResponse shape (sync_id, no results). - var resp sendResponse + // Single-chat path returns the uniform MultiSendResponse (results[0].sync_id). + var resp MultiSendResponse if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) } - if !resp.OK || resp.SyncID != "sync-1" { - t.Errorf("response = %+v, want ok/sync-1", resp) + if !resp.OK || len(resp.Results) != 1 || resp.Results[0].Chat != "override" || resp.Results[0].SyncID != "sync-1" { + t.Errorf("response = %+v, want ok with single override->sync-1 result", resp) } } @@ -1026,7 +1059,7 @@ func TestGitlab_FanoutTwoChats(t *testing.T) { if cap.count() != 2 { t.Fatalf("send count = %d, want 2", cap.count()) } - var resp gitlabFanoutResponse + var resp MultiSendResponse if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) } @@ -1126,7 +1159,7 @@ func TestGitlab_FanoutPartialFailure(t *testing.T) { if cap.count() != 2 { t.Fatalf("send count = %d, want 2 (both attempted)", cap.count()) } - var resp gitlabFanoutResponse + var resp MultiSendResponse if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) } @@ -1154,7 +1187,7 @@ func TestGitlab_FanoutAllFail(t *testing.T) { if cap.count() != 2 { t.Fatalf("send count = %d, want 2 (both attempted)", cap.count()) } - var resp gitlabFanoutResponse + var resp MultiSendResponse if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) } @@ -1182,7 +1215,7 @@ func TestGitlab_NoRouteMatchFallsBackToDefault(t *testing.T) { return cap.last().ChatID }()) } - var resp gitlabFanoutResponse + var resp MultiSendResponse if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) } @@ -1234,7 +1267,7 @@ func TestGitlab_FanoutChatResolveError(t *testing.T) { if cap.count() != 1 { t.Fatalf("send count = %d, want 1 (only the resolvable chat)", cap.count()) } - var resp gitlabFanoutResponse + var resp MultiSendResponse if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) } @@ -1268,7 +1301,7 @@ func TestGitlab_SenderFanout(t *testing.T) { if cap.count() != 2 { t.Fatalf("send count = %d, want 2 (both sender chats)", cap.count()) } - var resp gitlabFanoutResponse + var resp MultiSendResponse if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) } @@ -1385,7 +1418,7 @@ func TestGitlab_SenderSingleChat(t *testing.T) { if got := cap.last().ChatID; got != "team-b-chat" { t.Errorf("chat = %q, want team-b-chat", got) } - var resp gitlabFanoutResponse + var resp MultiSendResponse if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) } @@ -1406,7 +1439,7 @@ func TestGitlab_SenderIgnoresQueryBot(t *testing.T) { if cap.count() != 2 { t.Fatalf("send count = %d, want 2 (both sender chats)", cap.count()) } - var resp gitlabFanoutResponse + var resp MultiSendResponse if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) } @@ -1416,7 +1449,7 @@ func TestGitlab_SenderIgnoresQueryBot(t *testing.T) { } // TestGitlab_QueryBotHonoredOnRoutesFanout: mirror of SenderIgnoresQueryBot — -// on the default-token path ?bot= must reach gitlabFanout. In this single-bot +// on the default-token path ?bot= must reach gitlabDeliver. In this single-bot // harness (no SingleBotName) an honoured ?bot=other fails every delivery with // "bot ... is not available", so 502 here proves the query bot is threaded // through; if a refactor passed "" at the default call site, the request would @@ -1434,7 +1467,7 @@ func TestGitlab_QueryBotHonoredOnRoutesFanout(t *testing.T) { if cap.count() != 0 { t.Fatalf("send count = %d, want 0 (bot resolution fails before send)", cap.count()) } - var resp gitlabFanoutResponse + var resp MultiSendResponse if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) } @@ -1443,20 +1476,26 @@ func TestGitlab_QueryBotHonoredOnRoutesFanout(t *testing.T) { } } -// TestGitlab_QueryBotHonoredOnSinglePath: same guarantee for gitlabSendSingle — -// ?bot= on the default-token single-chat path must be validated, yielding 400 -// in this harness rather than being silently dropped. +// TestGitlab_QueryBotHonoredOnSinglePath: same guarantee for the single-chat +// path — ?bot= on the default-token single-chat delivery must be validated. Bot +// resolution failure is now a per-chat delivery outcome, so it surfaces as a 502 +// with the error in errors[] (the whole single-target fan-out failed) rather than +// being silently dropped. func TestGitlab_QueryBotHonoredOnSinglePath(t *testing.T) { srv, cap := newGitlabFanoutServer(t, &GitlabConfig{SecretToken: "secret", DefaultChatID: "chat1"}, okSend, nil) w := doRequest(srv, "POST", "/api/v1/gitlab?bot=other", strings.NewReader(mrOpenPayload), gitlabHeaders("secret")) - if w.Code != 400 { - t.Fatalf("status = %d, want 400 (?bot honoured must fail bot resolution); body: %s", w.Code, w.Body.String()) + if w.Code != 502 { + t.Fatalf("status = %d, want 502 (?bot honoured must fail bot resolution); body: %s", w.Code, w.Body.String()) } if cap.count() != 0 { t.Fatalf("send count = %d, want 0", cap.count()) } - if !strings.Contains(w.Body.String(), `bot \"other\" is not available`) { - t.Errorf("body = %s, want bot-not-available error", w.Body.String()) + var resp MultiSendResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) + } + if len(resp.Errors) != 1 || resp.Errors[0].Chat != "chat1" || !strings.Contains(resp.Errors[0].Error, `bot "other" is not available`) { + t.Errorf("errors = %+v, want single chat1 bot-not-available error", resp.Errors) } } @@ -1474,7 +1513,7 @@ func TestGitlab_SenderFanoutAllFail(t *testing.T) { if cap.count() != 2 { t.Fatalf("send count = %d, want 2 (both attempted)", cap.count()) } - var resp gitlabFanoutResponse + var resp MultiSendResponse if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) } @@ -1500,7 +1539,7 @@ func TestGitlab_SenderFanoutPartialFailure(t *testing.T) { if cap.count() != 2 { t.Fatalf("send count = %d, want 2 (both attempted)", cap.count()) } - var resp gitlabFanoutResponse + var resp MultiSendResponse if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) } @@ -1550,7 +1589,7 @@ func TestGitlab_SenderMultiBot(t *testing.T) { if w.Code != 200 { t.Fatalf("status = %d, want 200 (partial success — bound chat delivers); body: %s", w.Code, w.Body.String()) } - var resp gitlabFanoutResponse + var resp MultiSendResponse if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) } @@ -1566,8 +1605,8 @@ func TestGitlab_SenderMultiBot(t *testing.T) { } // TestGitlab_DefaultTokenUnchangedWithSenders: with senders configured, the -// default token keeps its original behaviour — single delivery to -// default_chat_id, plain sendResponse shape, ?chat_id override still works. +// default token keeps its original chat-selection behaviour — single delivery to +// default_chat_id, uniform MultiSendResponse shape, ?chat_id override still works. func TestGitlab_DefaultTokenUnchangedWithSenders(t *testing.T) { t.Run("default_chat", func(t *testing.T) { srv, cap := newGitlabFanoutServer(t, senderTestConfig(), okSend, nil) @@ -1581,12 +1620,12 @@ func TestGitlab_DefaultTokenUnchangedWithSenders(t *testing.T) { if got := cap.last().ChatID; got != "chat1" { t.Errorf("chat = %q, want chat1 (default)", got) } - var resp sendResponse + var resp MultiSendResponse if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) } - if !resp.OK || resp.SyncID != "sync-1" { - t.Errorf("response = %+v, want ok/sync-1", resp) + if !resp.OK || len(resp.Results) != 1 || resp.Results[0].Chat != "chat1" || resp.Results[0].SyncID != "sync-1" { + t.Errorf("response = %+v, want ok with single chat1->sync-1 result", resp) } }) t.Run("query_chat_override", func(t *testing.T) { diff --git a/internal/server/handler_grafana.go b/internal/server/handler_grafana.go index a07dcf8..1c6dff6 100644 --- a/internal/server/handler_grafana.go +++ b/internal/server/handler_grafana.go @@ -88,53 +88,45 @@ func (s *Server) handleGrafana(w http.ResponseWriter, r *http.Request) { // Determine status status := s.resolveGrafanaStatus(webhook) - // Resolve chat: query param > default_chat_id > global default chat > single chat from config - targetChat := s.grCfg.DefaultChatID - if targetChat == "" { - targetChat = s.cfg.DefaultChatAlias + // Resolve target chats: ?chat_id (now comma-separated, fan-out) > default_chat_id + // > global default chat > single chat from config. With no ?chat_id the endpoint + // keeps its single-default behaviour; the response is the uniform + // MultiSendResponse in every case (results[0] for a single chat). + targets := parseChatIDs(r.URL.Query().Get("chat_id")) + if len(targets) == 0 { + if single := s.grCfg.singleChat(s.cfg.DefaultChatAlias); single != "" { + targets = []string{single} + } } - if targetChat == "" { - targetChat = s.grCfg.FallbackChatID - } - if q := r.URL.Query().Get("chat_id"); q != "" { - targetChat = q - } - if targetChat == "" { + if len(targets) == 0 { writeError(w, http.StatusBadRequest, "chat_id is required: set default_chat_id in config, configure a single chat alias, or pass ?chat_id=") return } - chatResult, err := s.chats(targetChat) - if err != nil { - writeError(w, http.StatusBadRequest, "resolving chat: "+err.Error()) - return - } - - // Resolve bot: explicit ?bot= > chat-bound bot > auth bot - botName, errMsg := s.resolveRequestBot(r.Context(), r.URL.Query().Get("bot"), chatResult.Bot) - if errMsg != "" { - writeError(w, http.StatusBadRequest, errMsg) - return - } start := time.Now() - syncID, err := s.send(r.Context(), &SendPayload{ - Bot: botName, - ChatID: chatResult.ChatID, - Message: message, - Status: status, - }) + results, errs := s.fanoutSend(r.Context(), targets, r.URL.Query().Get("bot"), message, status) elapsed := time.Since(start) keyName := KeyName(r.Context()) - if err != nil { - vlog.V1("grafana: send failed [key: %s] -> 502 (%dms)", keyName, elapsed.Milliseconds()) - writeError(w, http.StatusBadGateway, "upstream error: "+err.Error()) - return + if len(results) == 0 { + vlog.V1("grafana: fan-out to %d chats all failed [key: %s] -> 502 (%dms)", len(targets), keyName, elapsed.Milliseconds()) + } else { + vlog.V1("grafana: sent %s to %d/%d chats [key: %s] (%dms)", webhook.Status, len(results), len(targets), keyName, elapsed.Milliseconds()) } + writeMultiSend(w, results, errs, http.StatusOK) +} - vlog.V1("grafana: sent %s to %s [key: %s] -> 200 (%dms)", webhook.Status, targetChat, keyName, elapsed.Milliseconds()) - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(sendResponse{OK: true, SyncID: syncID}) +// singleChat returns the fallback delivery chat for grafana, following the +// precedence default_chat_id -> global default chat -> the sole configured chat +// alias. It is empty when none is configured. +func (c *GrafanaConfig) singleChat(globalDefault string) string { + if c.DefaultChatID != "" { + return c.DefaultChatID + } + if globalDefault != "" { + return globalDefault + } + return c.FallbackChatID } func (s *Server) resolveGrafanaStatus(webhook GrafanaWebhook) string { diff --git a/internal/server/handler_send.go b/internal/server/handler_send.go index 6995f27..cab2c1a 100644 --- a/internal/server/handler_send.go +++ b/internal/server/handler_send.go @@ -2,8 +2,10 @@ package server import ( "bytes" + "context" "encoding/base64" "encoding/json" + "errors" "fmt" "io" "mime" @@ -49,12 +51,13 @@ type OptsPayload struct { NoNotify bool `json:"no_notify"` } +// sendResponse is the request-level error envelope for /send (400/415/500). The +// success/partial-success body is MultiSendResponse; a per-chat outcome is a +// SendResult. sendResponse now carries only ok/error — the earlier single-chat +// sync_id/queued/request_id fields moved into MultiSendResponse.results[]. type sendResponse struct { - OK bool `json:"ok"` - SyncID string `json:"sync_id,omitempty"` - Queued bool `json:"queued,omitempty"` - RequestID string `json:"request_id,omitempty"` - Error string `json:"error,omitempty"` + OK bool `json:"ok"` + Error string `json:"error,omitempty"` } func (s *Server) handleSend(w http.ResponseWriter, r *http.Request) { @@ -114,6 +117,23 @@ func (s *Server) handleSend(w http.ResponseWriter, r *http.Request) { // after catalog routing resolves the actual target bot. This ensures // email lookups hit the correct eXpress host in multi-bot setups. payload.NoParse = noParse + + // Expand a comma-separated chat_id (chat_id=a,b,c) into N independent + // enqueues — one queued message per chat — rather than one queued message + // that fans out inside the worker. This is a deliberate choice: with one + // message per chat, retry/ack is per-chat and independent, so a transient + // failure on chat b never re-delivers to chat a (no duplicates) and never + // drops b. The "one message = whole command, fan out in worker" + // alternative would give per-command retry: on a partial failure the retry + // re-sends to the chats that already succeeded (duplicates) or the failed + // chat is lost with the ack. The worker stays "one chat = one message" and + // is not touched. See docs/plans/20260708-multi-chat-fanout.md. + targets := parseChatIDs(payload.ChatID) + if len(targets) == 0 { + writeError(w, http.StatusBadRequest, "chat_id is required") + return + } + // Async mode: for direct routing, bot_id is required. // For catalog/mixed modes, bot_id or bot alias can be used. rm := payload.RoutingMode @@ -123,38 +143,14 @@ func (s *Server) handleSend(w http.ResponseWriter, r *http.Request) { if rm == "" { rm = "mixed" } - switch rm { - case "catalog": - // Catalog mode: bot can come from bot_id, bot alias, or chat-bound bot. - // If no bot info is provided, chat_id must be a non-UUID alias - // so the bot can be derived from the chat binding. - if payload.BotID == "" && payload.Bot == "" && isUUID(payload.ChatID) { - writeError(w, http.StatusBadRequest, "bot_id or bot alias is required when chat_id is a UUID in catalog mode; use a chat alias with a catalog-bound bot, or provide bot_id/bot") - return - } - case "mixed": - // Mixed mode: bot can come from bot_id (direct), bot alias, or chat-bound bot. - if payload.BotID == "" && payload.Bot == "" && isUUID(payload.ChatID) { - writeError(w, http.StatusBadRequest, "bot_id or bot alias is required when chat_id is a UUID in mixed mode; provide bot_id for direct routing or bot alias for catalog resolution") - return - } - case "direct": - // Direct mode: bot_id is required and must be a UUID - if payload.BotID == "" { - writeError(w, http.StatusBadRequest, "bot_id is required for async direct mode") + // Routing validation is request-level (400): apply the routing-mode + // requirements to every target chat before enqueuing any of them, so a + // multi-chat request is accepted or rejected as a whole. + for _, chat := range targets { + if msg := validateAsyncRouting(rm, payload.BotID, payload.Bot, chat); msg != "" { + writeError(w, http.StatusBadRequest, msg) return } - if !isUUID(payload.BotID) { - writeError(w, http.StatusBadRequest, "bot_id must be a valid UUID for direct routing mode") - return - } - if !isUUID(payload.ChatID) { - writeError(w, http.StatusBadRequest, "chat_id must be a valid UUID for direct routing mode; use catalog or mixed mode for alias resolution") - return - } - default: - writeError(w, http.StatusBadRequest, fmt.Sprintf("invalid routing_mode %q: must be direct, catalog, or mixed", rm)) - return } // Enforce max_file_size for async mode @@ -172,69 +168,123 @@ func (s *Server) handleSend(w http.ResponseWriter, r *http.Request) { } start := time.Now() - requestID, err := s.send(r.Context(), &payload) + results, errs := fanout(r.Context(), targets, func(ctx context.Context, chat string) (SendResult, error) { + // Copy the shared payload and enqueue one message for this chat only. + p := payload + p.ChatID = chat + requestID, err := s.send(ctx, &p) + if err != nil { + return SendResult{}, err + } + return SendResult{Chat: chat, RequestID: requestID, Queued: true}, nil + }) elapsed := time.Since(start) keyName := KeyName(r.Context()) - if err != nil { + if len(results) == 0 { vlog.V1("server: %s %s [key: %s] -> 502 (%dms)", r.Method, r.URL.Path, keyName, elapsed.Milliseconds()) - writeError(w, http.StatusBadGateway, "enqueue error: "+err.Error()) - return + } else { + vlog.V1("server: %s %s [key: %s] -> 202 (%dms)", r.Method, r.URL.Path, keyName, elapsed.Milliseconds()) } - - vlog.V1("server: %s %s [key: %s] -> 202 (%dms)", r.Method, r.URL.Path, keyName, elapsed.Milliseconds()) - w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusAccepted) - json.NewEncoder(w).Encode(sendResponse{OK: true, Queued: true, RequestID: requestID}) - return - } - - // Sync mode: resolve chat alias and bot, send directly - // Resolve chat alias (before bot — chat may have a bound bot) - chatResult, err := s.chats(payload.ChatID) - if err != nil { - writeError(w, http.StatusBadRequest, err.Error()) + writeMultiSend(w, results, errs, http.StatusAccepted) return } - payload.ChatID = chatResult.ChatID - // Resolve bot: explicit request bot > chat-bound bot > auth bot - resolvedBot, errMsg := s.resolveRequestBot(r.Context(), payload.Bot, chatResult.Bot) - if errMsg != "" { - writeError(w, http.StatusBadRequest, errMsg) + // Sync mode: fan out to every target chat best-effort. chat_id may list + // several chats (chat_id=a,b,c); each target independently resolves its chat + // alias, then its bot, then parses mentions under that bot's own resolver — + // a chat-bound bot can differ per chat in multi-bot setups, so mentions must + // be resolved after the per-target bot is known. The shared payload (file, + // metadata, opts, status) is reused for every target; only chat/bot/message/ + // mentions differ. The response is the uniform MultiSendResponse even for a + // single chat (results[0]). + targets := parseChatIDs(payload.ChatID) + if len(targets) == 0 { + writeError(w, http.StatusBadRequest, "chat_id is required") return } - payload.Bot = resolvedBot - - // Parse mentions after bot resolution so the correct per-bot resolver is used - // when the bot was derived from chat binding rather than the request payload. - resolver := s.mentionsResolver - if payload.Bot != "" && s.botMentionsResolvers != nil { - if br, ok := s.botMentionsResolvers[payload.Bot]; ok { - resolver = br - } - } - parseResult := mentions.Parse(r.Context(), payload.Message, payload.Mentions, !noParse, resolver) - payload.Message = parseResult.Message - payload.Mentions = parseResult.Mentions - if len(parseResult.Errors) > 0 { - vlog.V2("server: mentions parse: %d error(s)", len(parseResult.Errors)) - } start := time.Now() - syncID, err := s.send(r.Context(), &payload) + results, errs := fanout(r.Context(), targets, func(ctx context.Context, chat string) (SendResult, error) { + // Resolve chat alias (before bot — chat may have a bound bot). + chatResult, err := s.chats(chat) + if err != nil { + return SendResult{}, err + } + // Resolve bot: explicit request bot > chat-bound bot > auth bot. + resolvedBot, errMsg := s.resolveRequestBot(ctx, payload.Bot, chatResult.Bot) + if errMsg != "" { + return SendResult{}, errors.New(errMsg) + } + // Parse mentions after bot resolution so the correct per-bot resolver is + // used when the bot was derived from chat binding rather than the payload. + resolver := s.mentionsResolver + if resolvedBot != "" && s.botMentionsResolvers != nil { + if br, ok := s.botMentionsResolvers[resolvedBot]; ok { + resolver = br + } + } + parseResult := mentions.Parse(ctx, payload.Message, payload.Mentions, !noParse, resolver) + if len(parseResult.Errors) > 0 { + vlog.V2("server: mentions parse: %d error(s)", len(parseResult.Errors)) + } + // Copy the shared payload and override the per-target fields. + p := payload + p.ChatID = chatResult.ChatID + p.Bot = resolvedBot + p.Message = parseResult.Message + p.Mentions = parseResult.Mentions + syncID, err := s.send(ctx, &p) + if err != nil { + return SendResult{}, err + } + return SendResult{Chat: chat, SyncID: syncID}, nil + }) elapsed := time.Since(start) keyName := KeyName(r.Context()) - if err != nil { + if len(results) == 0 { vlog.V1("server: %s %s [key: %s] -> 502 (%dms)", r.Method, r.URL.Path, keyName, elapsed.Milliseconds()) - writeError(w, http.StatusBadGateway, "upstream error: "+err.Error()) - return + } else { + vlog.V1("server: %s %s [key: %s] -> 200 (%dms)", r.Method, r.URL.Path, keyName, elapsed.Milliseconds()) } + writeMultiSend(w, results, errs, http.StatusOK) +} - vlog.V1("server: %s %s [key: %s] -> 200 (%dms)", r.Method, r.URL.Path, keyName, elapsed.Milliseconds()) - w.Header().Set("Content-Type", "application/json") - json.NewEncoder(w).Encode(sendResponse{OK: true, SyncID: syncID}) +// validateAsyncRouting checks a single target chat against the async routing-mode +// requirements. It returns an empty string when the chat is acceptable, or a +// request-level error message (HTTP 400) describing what is missing. When +// chat_id lists several chats it is called once per chat so the whole request is +// accepted or rejected together. +func validateAsyncRouting(rm, botID, botAlias, chat string) string { + switch rm { + case "catalog": + // Catalog mode: bot can come from bot_id, bot alias, or chat-bound bot. + // If no bot info is provided, chat_id must be a non-UUID alias + // so the bot can be derived from the chat binding. + if botID == "" && botAlias == "" && isUUID(chat) { + return "bot_id or bot alias is required when chat_id is a UUID in catalog mode; use a chat alias with a catalog-bound bot, or provide bot_id/bot" + } + case "mixed": + // Mixed mode: bot can come from bot_id (direct), bot alias, or chat-bound bot. + if botID == "" && botAlias == "" && isUUID(chat) { + return "bot_id or bot alias is required when chat_id is a UUID in mixed mode; provide bot_id for direct routing or bot alias for catalog resolution" + } + case "direct": + // Direct mode: bot_id is required and must be a UUID. + if botID == "" { + return "bot_id is required for async direct mode" + } + if !isUUID(botID) { + return "bot_id must be a valid UUID for direct routing mode" + } + if !isUUID(chat) { + return "chat_id must be a valid UUID for direct routing mode; use catalog or mixed mode for alias resolution" + } + default: + return fmt.Sprintf("invalid routing_mode %q: must be direct, catalog, or mixed", rm) + } + return "" } func parseJSON(body io.ReadCloser, p *SendPayload) error { diff --git a/internal/server/handler_send_multichat_test.go b/internal/server/handler_send_multichat_test.go new file mode 100644 index 0000000..8803706 --- /dev/null +++ b/internal/server/handler_send_multichat_test.go @@ -0,0 +1,411 @@ +package server + +import ( + "context" + "fmt" + "strings" + "testing" + + "github.com/lavr/express-botx/internal/mentions" +) + +// Task 4: /send sync fans out over a comma-separated chat_id. Each target +// resolves its own chat and bot, and mentions are parsed under that bot's own +// resolver — a chat-bound bot can differ per chat in multi-bot setups. The +// response is always the uniform MultiSendResponse, even for a single chat. + +// fakeUserResolver implements mentions.UserResolver, returning a fixed huid/name +// for every email so a test can tell which per-bot resolver ran by the huid that +// ends up in the resulting mention. +type fakeUserResolver struct { + huid string + name string +} + +func (f fakeUserResolver) GetUserByEmail(_ context.Context, _ string) (string, string, error) { + return f.huid, f.name, nil +} + +// newMultiBotSendServer builds a two-bot server whose chat resolver binds chatA +// -> botA and chatB -> botB, with a distinct per-bot mentions resolver each, so +// per-target bot + mention resolution can be observed. failChats resolve to an +// error (for partial/all-fail cases). +func newMultiBotSendServer(sendFn SendFunc, failChats ...string) *Server { + fail := make(map[string]bool, len(failChats)) + for _, c := range failChats { + fail[c] = true + } + chatResolver := func(chatID string) (ChatResolveResult, error) { + if fail[chatID] { + return ChatResolveResult{}, fmt.Errorf("unknown chat alias %q", chatID) + } + bound := "" + switch chatID { + case "chatA": + bound = "botA" + case "chatB": + bound = "botB" + } + return ChatResolveResult{ChatID: chatID, Bot: bound}, nil + } + cfg := Config{ + Listen: ":0", + BasePath: "/api/v1", + Keys: []ResolvedKey{{Name: "t", Key: "k"}}, + BotNames: []string{"botA", "botB"}, + } + return New(cfg, sendFn, chatResolver, + WithBotMentionsResolvers(map[string]mentions.UserResolver{ + "botA": fakeUserResolver{huid: "huid-A", name: "Alice A"}, + "botB": fakeUserResolver{huid: "huid-B", name: "Bob B"}, + }), + ) +} + +func TestSendSync_SingleChat_UnifiedResponse(t *testing.T) { + cap := &captureSend{} + send := func(_ context.Context, p *SendPayload) (string, error) { + cap.record(p) + return "sync-1", nil + } + srv := newMultiBotSendServer(send) + body := `{"chat_id":"chatA","message":"hello"}` + w := doRequest(srv, "POST", "/api/v1/send", strings.NewReader(body), map[string]string{ + "X-API-Key": "k", + "Content-Type": "application/json", + }) + if w.Code != 200 { + t.Fatalf("status = %d, body: %s", w.Code, w.Body.String()) + } + resp := parseMultiResponse(t, w) + if !resp.OK || len(resp.Results) != 1 || len(resp.Errors) != 0 { + t.Fatalf("response = %+v, want ok with 1 result, 0 errors", resp) + } + if resp.Results[0].Chat != "chatA" || resp.Results[0].SyncID != "sync-1" { + t.Errorf("results[0] = %+v, want chatA -> sync-1", resp.Results[0]) + } + if cap.count() != 1 || cap.last().Bot != "botA" { + t.Errorf("send: count=%d bot=%q, want 1 send to botA", cap.count(), cap.last().Bot) + } +} + +func TestSendSync_MultiChat_PerBotMentions(t *testing.T) { + cap := &captureSend{} + send := func(_ context.Context, p *SendPayload) (string, error) { + cap.record(p) + return "sync-" + p.ChatID, nil + } + srv := newMultiBotSendServer(send) + // Same message with an inline email mention: chatA -> botA's resolver + // (huid-A), chatB -> botB's resolver (huid-B). If mentions were parsed once + // up front with a single resolver, both chats would carry the same huid. + body := `{"chat_id":"chatA,chatB","message":"hi @mention[email:x@example.com]"}` + w := doRequest(srv, "POST", "/api/v1/send", strings.NewReader(body), map[string]string{ + "X-API-Key": "k", + "Content-Type": "application/json", + }) + if w.Code != 200 { + t.Fatalf("status = %d, body: %s", w.Code, w.Body.String()) + } + resp := parseMultiResponse(t, w) + if !resp.OK || len(resp.Results) != 2 || len(resp.Errors) != 0 { + t.Fatalf("response = %+v, want ok with 2 results", resp) + } + if resp.Results[0].Chat != "chatA" || resp.Results[1].Chat != "chatB" { + t.Fatalf("results = %+v, want chatA then chatB", resp.Results) + } + if cap.count() != 2 { + t.Fatalf("send count = %d, want 2", cap.count()) + } + // Verify each chat was sent under its own bot with its own resolver's huid. + byChat := map[string]*SendPayload{} + for _, c := range cap.calls { + byChat[c.ChatID] = c + } + a, b := byChat["chatA"], byChat["chatB"] + if a == nil || b == nil { + t.Fatalf("missing captured payloads: %+v", byChat) + } + if a.Bot != "botA" || b.Bot != "botB" { + t.Errorf("bots = %q/%q, want botA/botB", a.Bot, b.Bot) + } + if !strings.Contains(string(a.Mentions), "huid-A") { + t.Errorf("chatA mentions = %s, want huid-A (botA resolver)", a.Mentions) + } + if !strings.Contains(string(b.Mentions), "huid-B") { + t.Errorf("chatB mentions = %s, want huid-B (botB resolver)", b.Mentions) + } +} + +func TestSendSync_MultiChat_PartialFailure(t *testing.T) { + cap := &captureSend{} + send := func(_ context.Context, p *SendPayload) (string, error) { + cap.record(p) + return "sync-1", nil + } + srv := newMultiBotSendServer(send, "chatB") + body := `{"chat_id":"chatA,chatB","message":"hello"}` + w := doRequest(srv, "POST", "/api/v1/send", strings.NewReader(body), map[string]string{ + "X-API-Key": "k", + "Content-Type": "application/json", + }) + if w.Code != 200 { + t.Fatalf("status = %d, want 200 (partial), body: %s", w.Code, w.Body.String()) + } + resp := parseMultiResponse(t, w) + if !resp.OK || len(resp.Results) != 1 || len(resp.Errors) != 1 { + t.Fatalf("response = %+v, want ok with 1 result, 1 error", resp) + } + if resp.Results[0].Chat != "chatA" { + t.Errorf("results[0].Chat = %q, want chatA", resp.Results[0].Chat) + } + if resp.Errors[0].Chat != "chatB" { + t.Errorf("errors[0].Chat = %q, want chatB", resp.Errors[0].Chat) + } + if cap.count() != 1 { + t.Errorf("send count = %d, want 1 (only chatA)", cap.count()) + } +} + +func TestSendSync_MultiChat_AllFail(t *testing.T) { + cap := &captureSend{} + send := func(_ context.Context, p *SendPayload) (string, error) { + cap.record(p) + return "sync-1", nil + } + srv := newMultiBotSendServer(send, "chatA", "chatB") + body := `{"chat_id":"chatA,chatB","message":"hello"}` + w := doRequest(srv, "POST", "/api/v1/send", strings.NewReader(body), map[string]string{ + "X-API-Key": "k", + "Content-Type": "application/json", + }) + if w.Code != 502 { + t.Fatalf("status = %d, want 502 (all failed), body: %s", w.Code, w.Body.String()) + } + resp := parseMultiResponse(t, w) + if resp.OK || len(resp.Results) != 0 || len(resp.Errors) != 2 { + t.Fatalf("response = %+v, want not-ok with 0 results, 2 errors", resp) + } + if cap.count() != 0 { + t.Errorf("send count = %d, want 0 (nothing resolved)", cap.count()) + } +} + +func TestSendSync_MultiChat_Dedup(t *testing.T) { + cap := &captureSend{} + send := func(_ context.Context, p *SendPayload) (string, error) { + cap.record(p) + return "sync-1", nil + } + srv := newMultiBotSendServer(send) + body := `{"chat_id":"chatA, chatA ,chatB,chatA","message":"hello"}` + w := doRequest(srv, "POST", "/api/v1/send", strings.NewReader(body), map[string]string{ + "X-API-Key": "k", + "Content-Type": "application/json", + }) + if w.Code != 200 { + t.Fatalf("status = %d, body: %s", w.Code, w.Body.String()) + } + resp := parseMultiResponse(t, w) + if len(resp.Results) != 2 || resp.Results[0].Chat != "chatA" || resp.Results[1].Chat != "chatB" { + t.Fatalf("results = %+v, want deduped chatA then chatB", resp.Results) + } + if cap.count() != 2 { + t.Errorf("send count = %d, want 2 (deduped)", cap.count()) + } +} + +// Task 5: async /send expands a comma-separated chat_id into N independent +// enqueues — one queued message per chat — and returns the uniform +// MultiSendResponse with 202, one SendResult{Chat, RequestID, Queued} per chat. + +// newAsyncSendServer builds an async-mode server whose enqueue stub records every +// payload and returns a per-chat request id. Uses mixed routing so chat aliases +// with a bot alias pass request-level validation. +func newAsyncSendServer(sendFn SendFunc) *Server { + cfg := Config{ + Listen: ":0", + BasePath: "/api/v1", + Keys: []ResolvedKey{{Name: "t", Key: "k"}}, + AsyncMode: true, + DefaultRoutingMode: "mixed", + } + chatResolver := func(chatID string) (ChatResolveResult, error) { + return ChatResolveResult{ChatID: chatID}, nil + } + return New(cfg, sendFn, chatResolver) +} + +func TestSendAsync_SingleChat_UnifiedResponse(t *testing.T) { + cap := &captureSend{} + send := func(_ context.Context, p *SendPayload) (string, error) { + cap.record(p) + return "req-" + p.ChatID, nil + } + srv := newAsyncSendServer(send) + body := `{"bot":"alerts","chat_id":"deploy","message":"hi"}` + w := doRequest(srv, "POST", "/api/v1/send", strings.NewReader(body), map[string]string{ + "X-API-Key": "k", + "Content-Type": "application/json", + }) + if w.Code != 202 { + t.Fatalf("status = %d, body: %s", w.Code, w.Body.String()) + } + resp := parseMultiResponse(t, w) + if !resp.OK || len(resp.Results) != 1 || len(resp.Errors) != 0 { + t.Fatalf("response = %+v, want ok with 1 result", resp) + } + r0 := resp.Results[0] + if r0.Chat != "deploy" || r0.RequestID != "req-deploy" || !r0.Queued { + t.Errorf("results[0] = %+v, want deploy -> req-deploy queued", r0) + } + if cap.count() != 1 { + t.Errorf("enqueue count = %d, want 1", cap.count()) + } +} + +func TestSendAsync_MultiChat_Expands(t *testing.T) { + cap := &captureSend{} + send := func(_ context.Context, p *SendPayload) (string, error) { + cap.record(p) + return "req-" + p.ChatID, nil + } + srv := newAsyncSendServer(send) + body := `{"bot":"alerts","chat_id":"a,b","message":"hi"}` + w := doRequest(srv, "POST", "/api/v1/send", strings.NewReader(body), map[string]string{ + "X-API-Key": "k", + "Content-Type": "application/json", + }) + if w.Code != 202 { + t.Fatalf("status = %d, body: %s", w.Code, w.Body.String()) + } + resp := parseMultiResponse(t, w) + if !resp.OK || len(resp.Results) != 2 || len(resp.Errors) != 0 { + t.Fatalf("response = %+v, want ok with 2 results", resp) + } + if resp.Results[0].Chat != "a" || resp.Results[0].RequestID != "req-a" || !resp.Results[0].Queued { + t.Errorf("results[0] = %+v, want a -> req-a queued", resp.Results[0]) + } + if resp.Results[1].Chat != "b" || resp.Results[1].RequestID != "req-b" || !resp.Results[1].Queued { + t.Errorf("results[1] = %+v, want b -> req-b queued", resp.Results[1]) + } + // Expand: one independent enqueue per chat, each carrying only its own chat. + if cap.count() != 2 { + t.Fatalf("enqueue count = %d, want 2 (one per chat)", cap.count()) + } + seen := map[string]bool{} + for _, c := range cap.calls { + seen[c.ChatID] = true + } + if !seen["a"] || !seen["b"] { + t.Errorf("enqueued chats = %v, want a and b", seen) + } +} + +func TestSendAsync_MultiChat_Dedup(t *testing.T) { + cap := &captureSend{} + send := func(_ context.Context, p *SendPayload) (string, error) { + cap.record(p) + return "req-" + p.ChatID, nil + } + srv := newAsyncSendServer(send) + body := `{"bot":"alerts","chat_id":"a, a ,b,a","message":"hi"}` + w := doRequest(srv, "POST", "/api/v1/send", strings.NewReader(body), map[string]string{ + "X-API-Key": "k", + "Content-Type": "application/json", + }) + if w.Code != 202 { + t.Fatalf("status = %d, body: %s", w.Code, w.Body.String()) + } + resp := parseMultiResponse(t, w) + if len(resp.Results) != 2 || resp.Results[0].Chat != "a" || resp.Results[1].Chat != "b" { + t.Fatalf("results = %+v, want deduped a then b", resp.Results) + } + if cap.count() != 2 { + t.Errorf("enqueue count = %d, want 2 (deduped)", cap.count()) + } +} + +func TestSendAsync_MultiChat_PartialFailure(t *testing.T) { + cap := &captureSend{} + send := func(_ context.Context, p *SendPayload) (string, error) { + cap.record(p) + if p.ChatID == "b" { + return "", fmt.Errorf("broker refused chat b") + } + return "req-" + p.ChatID, nil + } + srv := newAsyncSendServer(send) + body := `{"bot":"alerts","chat_id":"a,b","message":"hi"}` + w := doRequest(srv, "POST", "/api/v1/send", strings.NewReader(body), map[string]string{ + "X-API-Key": "k", + "Content-Type": "application/json", + }) + if w.Code != 202 { + t.Fatalf("status = %d, want 202 (partial), body: %s", w.Code, w.Body.String()) + } + resp := parseMultiResponse(t, w) + if !resp.OK || len(resp.Results) != 1 || len(resp.Errors) != 1 { + t.Fatalf("response = %+v, want ok with 1 result, 1 error", resp) + } + if resp.Results[0].Chat != "a" { + t.Errorf("results[0].Chat = %q, want a", resp.Results[0].Chat) + } + if resp.Errors[0].Chat != "b" || !strings.Contains(resp.Errors[0].Error, "broker refused chat b") { + t.Errorf("errors[0] = %+v, want b -> broker refused", resp.Errors[0]) + } +} + +func TestSendAsync_MultiChat_AllFail(t *testing.T) { + send := func(_ context.Context, p *SendPayload) (string, error) { + return "", fmt.Errorf("broker down") + } + srv := newAsyncSendServer(send) + body := `{"bot":"alerts","chat_id":"a,b","message":"hi"}` + w := doRequest(srv, "POST", "/api/v1/send", strings.NewReader(body), map[string]string{ + "X-API-Key": "k", + "Content-Type": "application/json", + }) + if w.Code != 502 { + t.Fatalf("status = %d, want 502 (all failed), body: %s", w.Code, w.Body.String()) + } + resp := parseMultiResponse(t, w) + if resp.OK || len(resp.Results) != 0 || len(resp.Errors) != 2 { + t.Fatalf("response = %+v, want not-ok with 0 results, 2 errors", resp) + } +} + +func TestSendAsync_EmptyAfterParse(t *testing.T) { + // chat_id present but only commas -> request-level 400. + send := func(_ context.Context, p *SendPayload) (string, error) { return "x", nil } + srv := newAsyncSendServer(send) + body := `{"bot":"alerts","chat_id":",,","message":"hi"}` + w := doRequest(srv, "POST", "/api/v1/send", strings.NewReader(body), map[string]string{ + "X-API-Key": "k", + "Content-Type": "application/json", + }) + if w.Code != 400 { + t.Fatalf("status = %d, want 400, body: %s", w.Code, w.Body.String()) + } + resp := parseResponse(t, w) + if resp.Error != "chat_id is required" { + t.Errorf("error = %q, want 'chat_id is required'", resp.Error) + } +} + +func TestSendSync_EmptyAfterParse(t *testing.T) { + // chat_id present but only commas -> request-level 400 (not a per-chat error). + srv := newTestServer([]ResolvedKey{{Name: "t", Key: "k"}}) + body := `{"chat_id":",,","message":"hello"}` + w := doRequest(srv, "POST", "/api/v1/send", strings.NewReader(body), map[string]string{ + "X-API-Key": "k", + "Content-Type": "application/json", + }) + if w.Code != 400 { + t.Fatalf("status = %d, want 400, body: %s", w.Code, w.Body.String()) + } + resp := parseResponse(t, w) + if resp.Error != "chat_id is required" { + t.Errorf("error = %q, want 'chat_id is required'", resp.Error) + } +} diff --git a/internal/server/handler_webhook_multichat_test.go b/internal/server/handler_webhook_multichat_test.go new file mode 100644 index 0000000..236642f --- /dev/null +++ b/internal/server/handler_webhook_multichat_test.go @@ -0,0 +1,301 @@ +package server + +import ( + "context" + "encoding/json" + "fmt" + "strings" + "testing" +) + +// Task 3: /alertmanager and /grafana share the project-wide multi-chat contract. +// ?chat_id may list several chats (comma-separated) and the event fans out to +// each best-effort; the response is always a MultiSendResponse, even for a single +// or default chat (results[0]). These tests exercise both handlers through the +// same table so the two surfaces stay in lockstep. + +// webhookSurface abstracts the two notification endpoints so one table drives +// both: a POST path, an already-decodable firing payload, and a config builder +// that injects a custom send + chat resolver. +type webhookSurface struct { + name string + path string + payload string + // newServer builds a server for this surface with the given send/chat + // resolvers and default chat wiring (webhookDefault -> DefaultChatID, + // globalDefault -> cfg.DefaultChatAlias). + newServer func(t *testing.T, webhookDefault, globalDefault string, sendFn SendFunc, chatFn ChatResolver) *Server +} + +func webhookSurfaces(t *testing.T) []webhookSurface { + t.Helper() + amTmpl, err := ParseAlertmanagerTemplate(`alert {{ .Status }}`) + if err != nil { + t.Fatalf("parse alertmanager template: %v", err) + } + grTmpl, err := ParseGrafanaTemplate(`grafana {{ .Status }}`) + if err != nil { + t.Fatalf("parse grafana template: %v", err) + } + amPayload := alertmanagerPayload("firing", AlertItem{ + Status: "firing", + Labels: map[string]string{"alertname": "Test", "severity": "critical"}, + }) + grPayload := grafanaPayload("firing", "alerting", "test", GrafanaAlertItem{ + Status: "firing", + Labels: map[string]string{"alertname": "Test"}, + }) + return []webhookSurface{ + { + name: "alertmanager", + path: "/api/v1/alertmanager", + payload: amPayload, + newServer: func(t *testing.T, webhookDefault, globalDefault string, sendFn SendFunc, chatFn ChatResolver) *Server { + return New( + Config{Listen: ":0", BasePath: "/api/v1", DefaultChatAlias: globalDefault, Keys: []ResolvedKey{{Name: "t", Key: "k"}}}, + sendFn, chatFn, + WithAlertmanager(&AlertmanagerConfig{ + DefaultChatID: webhookDefault, + ErrorSeverities: []string{"critical"}, + Template: amTmpl, + }), + ) + }, + }, + { + name: "grafana", + path: "/api/v1/grafana", + payload: grPayload, + newServer: func(t *testing.T, webhookDefault, globalDefault string, sendFn SendFunc, chatFn ChatResolver) *Server { + return New( + Config{Listen: ":0", BasePath: "/api/v1", DefaultChatAlias: globalDefault, Keys: []ResolvedKey{{Name: "t", Key: "k"}}}, + sendFn, chatFn, + WithGrafana(&GrafanaConfig{ + DefaultChatID: webhookDefault, + ErrorStates: []string{"alerting"}, + Template: grTmpl, + }), + ) + }, + }, + } +} + +func webhookHeaders() map[string]string { + return map[string]string{"X-API-Key": "k", "Content-Type": "application/json"} +} + +// selfChatResolver resolves every alias to itself, failing only for aliases in +// the fail set. +func selfChatResolver(fail ...string) ChatResolver { + failSet := make(map[string]bool, len(fail)) + for _, f := range fail { + failSet[f] = true + } + return func(chatID string) (ChatResolveResult, error) { + if failSet[chatID] { + return ChatResolveResult{}, fmt.Errorf("unknown chat alias %q", chatID) + } + return ChatResolveResult{ChatID: chatID}, nil + } +} + +// TestWebhook_SingleChatUnifiedResponse: a single/default chat still returns the +// uniform MultiSendResponse (results[0].sync_id), not the old {ok,sync_id} form. +func TestWebhook_SingleChatUnifiedResponse(t *testing.T) { + for _, s := range webhookSurfaces(t) { + t.Run(s.name, func(t *testing.T) { + cap := &captureSend{} + send := func(ctx context.Context, p *SendPayload) (string, error) { + cap.record(p) + return "sync-1", nil + } + srv := s.newServer(t, "default-chat", "", send, selfChatResolver()) + w := doRequest(srv, "POST", s.path, strings.NewReader(s.payload), webhookHeaders()) + if w.Code != 200 { + t.Fatalf("status = %d, body: %s", w.Code, w.Body.String()) + } + if cap.count() != 1 { + t.Fatalf("send count = %d, want 1", cap.count()) + } + if cap.last().ChatID != "default-chat" { + t.Errorf("chat = %q, want default-chat", cap.last().ChatID) + } + var resp MultiSendResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) + } + if !resp.OK || len(resp.Results) != 1 || len(resp.Errors) != 0 { + t.Fatalf("response = %+v, want ok with 1 result, 0 errors", resp) + } + if resp.Results[0].Chat != "default-chat" || resp.Results[0].SyncID != "sync-1" { + t.Errorf("results[0] = %+v, want default-chat -> sync-1", resp.Results[0]) + } + }) + } +} + +// TestWebhook_MultiChatFanout: ?chat_id=a,b fans out to both chats (deduping +// repeats), preserving target order in the results. +func TestWebhook_MultiChatFanout(t *testing.T) { + for _, s := range webhookSurfaces(t) { + t.Run(s.name, func(t *testing.T) { + cap := &captureSend{} + send := func(ctx context.Context, p *SendPayload) (string, error) { + cap.record(p) + return "sync-1", nil + } + srv := s.newServer(t, "default-chat", "", send, selfChatResolver()) + w := doRequest(srv, "POST", s.path+"?chat_id=chatA,+chatB+,chatA", strings.NewReader(s.payload), webhookHeaders()) + if w.Code != 200 { + t.Fatalf("status = %d, body: %s", w.Code, w.Body.String()) + } + if cap.count() != 2 { + t.Fatalf("send count = %d, want 2 (deduped fan-out)", cap.count()) + } + var resp MultiSendResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) + } + if !resp.OK || len(resp.Results) != 2 || len(resp.Errors) != 0 { + t.Fatalf("response = %+v, want ok with 2 results, 0 errors", resp) + } + if resp.Results[0].Chat != "chatA" || resp.Results[1].Chat != "chatB" { + t.Errorf("results = %+v, want chatA then chatB in order", resp.Results) + } + }) + } +} + +// TestWebhook_PartialFailure: one chat resolves and one does not — 200 with the +// successful result and the failed chat in errors[] (best-effort). +func TestWebhook_PartialFailure(t *testing.T) { + for _, s := range webhookSurfaces(t) { + t.Run(s.name, func(t *testing.T) { + cap := &captureSend{} + send := func(ctx context.Context, p *SendPayload) (string, error) { + cap.record(p) + return "sync-1", nil + } + srv := s.newServer(t, "", "", send, selfChatResolver("bad")) + w := doRequest(srv, "POST", s.path+"?chat_id=chatA,bad", strings.NewReader(s.payload), webhookHeaders()) + if w.Code != 200 { + t.Fatalf("status = %d, want 200 (partial success), body: %s", w.Code, w.Body.String()) + } + if cap.count() != 1 { + t.Fatalf("send count = %d, want 1 (only chatA sent)", cap.count()) + } + var resp MultiSendResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) + } + if !resp.OK || len(resp.Results) != 1 || len(resp.Errors) != 1 { + t.Fatalf("response = %+v, want ok with 1 result, 1 error", resp) + } + if resp.Results[0].Chat != "chatA" { + t.Errorf("results[0].Chat = %q, want chatA", resp.Results[0].Chat) + } + if resp.Errors[0].Chat != "bad" || !strings.Contains(resp.Errors[0].Error, "resolving chat") { + t.Errorf("errors[0] = %+v, want bad -> resolving chat error", resp.Errors[0]) + } + }) + } +} + +// TestWebhook_AllFail: every target fails to resolve -> 502 with all errors and +// no results (unified contract, no send attempted). +func TestWebhook_AllFail(t *testing.T) { + for _, s := range webhookSurfaces(t) { + t.Run(s.name, func(t *testing.T) { + cap := &captureSend{} + send := func(ctx context.Context, p *SendPayload) (string, error) { + cap.record(p) + return "sync-1", nil + } + srv := s.newServer(t, "", "", send, selfChatResolver("badA", "badB")) + w := doRequest(srv, "POST", s.path+"?chat_id=badA,badB", strings.NewReader(s.payload), webhookHeaders()) + if w.Code != 502 { + t.Fatalf("status = %d, want 502 (all failed), body: %s", w.Code, w.Body.String()) + } + if cap.count() != 0 { + t.Errorf("send count = %d, want 0", cap.count()) + } + var resp MultiSendResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) + } + if resp.OK || len(resp.Results) != 0 || len(resp.Errors) != 2 { + t.Errorf("response = %+v, want not-ok with 0 results, 2 errors", resp) + } + }) + } +} + +// TestWebhook_SendFailureAllFail: chats resolve but the upstream send fails for +// every target -> 502 with per-chat errors (send was attempted). +func TestWebhook_SendFailureAllFail(t *testing.T) { + for _, s := range webhookSurfaces(t) { + t.Run(s.name, func(t *testing.T) { + cap := &captureSend{} + send := func(ctx context.Context, p *SendPayload) (string, error) { + cap.record(p) + return "", fmt.Errorf("botx unavailable") + } + srv := s.newServer(t, "", "", send, selfChatResolver()) + w := doRequest(srv, "POST", s.path+"?chat_id=chatA,chatB", strings.NewReader(s.payload), webhookHeaders()) + if w.Code != 502 { + t.Fatalf("status = %d, want 502, body: %s", w.Code, w.Body.String()) + } + if cap.count() != 2 { + t.Errorf("send count = %d, want 2 (both attempted then failed)", cap.count()) + } + var resp MultiSendResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) + } + if resp.OK || len(resp.Errors) != 2 { + t.Errorf("response = %+v, want not-ok with 2 errors", resp) + } + }) + } +} + +// TestWebhook_DefaultChatNoQuery: with no ?chat_id the endpoint delivers to the +// configured default chain (webhook default, then global default), unified shape. +func TestWebhook_DefaultChatNoQuery(t *testing.T) { + cases := []struct { + name string + webhookDefault string + globalDefault string + wantChat string + }{ + {"webhook_default", "webhook-chat", "global-chat", "webhook-chat"}, + {"global_default", "", "global-chat", "global-chat"}, + } + for _, s := range webhookSurfaces(t) { + for _, tc := range cases { + t.Run(s.name+"/"+tc.name, func(t *testing.T) { + cap := &captureSend{} + send := func(ctx context.Context, p *SendPayload) (string, error) { + cap.record(p) + return "sync-1", nil + } + srv := s.newServer(t, tc.webhookDefault, tc.globalDefault, send, selfChatResolver()) + w := doRequest(srv, "POST", s.path, strings.NewReader(s.payload), webhookHeaders()) + if w.Code != 200 { + t.Fatalf("status = %d, body: %s", w.Code, w.Body.String()) + } + if cap.count() != 1 || cap.last().ChatID != tc.wantChat { + t.Fatalf("chat = %q (count %d), want %q", cap.last().ChatID, cap.count(), tc.wantChat) + } + var resp MultiSendResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) + } + if !resp.OK || len(resp.Results) != 1 || resp.Results[0].Chat != tc.wantChat { + t.Errorf("response = %+v, want single %q result", resp, tc.wantChat) + } + }) + } + } +} diff --git a/internal/server/multisend.go b/internal/server/multisend.go new file mode 100644 index 0000000..74cecc6 --- /dev/null +++ b/internal/server/multisend.go @@ -0,0 +1,127 @@ +package server + +// multisend holds the project-wide multi-chat fan-out primitives shared by every +// send surface (/send, /alertmanager, /grafana, /gitlab and the CLI). The +// contract is deliberately uniform: a chat_id may list several chats separated by +// commas (chat_id=a,b,c), the message is delivered best-effort to each, and the +// response is always a MultiSendResponse — even for a single chat. See +// docs/plans/20260708-multi-chat-fanout.md for the rationale. + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "net/http" + "strings" +) + +// SendResult is a single successful per-chat delivery. Exactly one of SyncID +// (synchronous send) or RequestID+Queued (async/enqueue) is populated. +type SendResult struct { + Chat string `json:"chat"` + SyncID string `json:"sync_id,omitempty"` + RequestID string `json:"request_id,omitempty"` + Queued bool `json:"queued,omitempty"` +} + +// SendError is a single failed per-chat delivery: the target chat and the error +// that prevented delivery (chat/bot resolution or the upstream send/enqueue). +type SendError struct { + Chat string `json:"chat"` + Error string `json:"error"` +} + +// MultiSendResponse is the uniform response body for every send surface. OK is +// true when at least one chat received the message (HTTP 200/202); it is false +// when delivery to every chat failed (HTTP 502). Request-level failures (bad +// JSON, empty chat_id, invalid status, unsupported media) are NOT reported here — +// they keep the {"ok":false,"error":"..."} form with 400/415. +type MultiSendResponse struct { + OK bool `json:"ok"` + Results []SendResult `json:"results,omitempty"` + Errors []SendError `json:"errors,omitempty"` +} + +// parseChatIDs splits a raw chat_id value on commas, trims whitespace around each +// entry, drops empties, and deduplicates while preserving first-occurrence order. +// A blank or whitespace-only input yields an empty slice. +func parseChatIDs(raw string) []string { + parts := strings.Split(raw, ",") + out := make([]string, 0, len(parts)) + seen := make(map[string]struct{}, len(parts)) + for _, p := range parts { + chat := strings.TrimSpace(p) + if chat == "" { + continue + } + if _, dup := seen[chat]; dup { + continue + } + seen[chat] = struct{}{} + out = append(out, chat) + } + return out +} + +// fanout delivers to each target best-effort, calling deliver per chat and +// collecting the successful results and per-chat errors independently. Order is +// preserved: results and errors appear in the target order they were produced. +func fanout(ctx context.Context, targets []string, deliver func(ctx context.Context, chat string) (SendResult, error)) (results []SendResult, errs []SendError) { + for _, target := range targets { + res, err := deliver(ctx, target) + if err != nil { + errs = append(errs, SendError{Chat: target, Error: err.Error()}) + continue + } + results = append(results, res) + } + return results, errs +} + +// fanoutSend delivers message+status to every target chat best-effort using the +// shared fan-out primitive. Chat and bot are resolved per target (explicit +// requestBot ?bot= override > chat-bound bot > auth bot); mentions are NOT parsed +// here — per-bot mention parsing is a /send-only concern. Successes and per-chat +// failures (chat/bot resolution or the upstream send) are collected independently +// and returned in target order; callers write the response with writeMultiSend. +// This is the plain notification path shared by /alertmanager, /grafana and +// /gitlab, all of which deliver an already-rendered message with no mentions. +func (s *Server) fanoutSend(ctx context.Context, targets []string, requestBot, message, status string) ([]SendResult, []SendError) { + return fanout(ctx, targets, func(ctx context.Context, chat string) (SendResult, error) { + chatResult, err := s.chats(chat) + if err != nil { + return SendResult{}, fmt.Errorf("resolving chat: %w", err) + } + botName, errMsg := s.resolveRequestBot(ctx, requestBot, chatResult.Bot) + if errMsg != "" { + return SendResult{}, errors.New(errMsg) + } + syncID, err := s.send(ctx, &SendPayload{ + Bot: botName, + ChatID: chatResult.ChatID, + Message: message, + Status: status, + }) + if err != nil { + return SendResult{}, err + } + return SendResult{Chat: chat, SyncID: syncID}, nil + }) +} + +// writeMultiSend writes a MultiSendResponse. When at least one delivery +// succeeded it uses successStatus (200 for sync, 202 for async); when every +// delivery failed it uses 502 and OK is false. +func writeMultiSend(w http.ResponseWriter, results []SendResult, errs []SendError, successStatus int) { + w.Header().Set("Content-Type", "application/json") + if len(results) == 0 { + w.WriteHeader(http.StatusBadGateway) + json.NewEncoder(w).Encode(MultiSendResponse{OK: false, Errors: errs}) + return + } + if successStatus != http.StatusOK { + w.WriteHeader(successStatus) + } + json.NewEncoder(w).Encode(MultiSendResponse{OK: true, Results: results, Errors: errs}) +} diff --git a/internal/server/multisend_test.go b/internal/server/multisend_test.go new file mode 100644 index 0000000..f8dd0c3 --- /dev/null +++ b/internal/server/multisend_test.go @@ -0,0 +1,127 @@ +package server + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "reflect" + "testing" +) + +func TestParseChatIDs(t *testing.T) { + tests := []struct { + name string + raw string + want []string + }{ + {"single", "a", []string{"a"}}, + {"multi", "a,b,c", []string{"a", "b", "c"}}, + {"spaces", "a , b", []string{"a", "b"}}, + {"dups", "a,a,b", []string{"a", "b"}}, + {"trailing comma", "a,", []string{"a"}}, + {"empty", "", []string{}}, + {"only commas", ",,", []string{}}, + {"whitespace only", " ", []string{}}, + {"mixed spaces and dups", " a , b ,a, c ,b ", []string{"a", "b", "c"}}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := parseChatIDs(tt.raw) + if !reflect.DeepEqual(got, tt.want) { + t.Fatalf("parseChatIDs(%q) = %#v, want %#v", tt.raw, got, tt.want) + } + }) + } +} + +func TestFanout(t *testing.T) { + targets := []string{"a", "b", "c"} + deliver := func(_ context.Context, chat string) (SendResult, error) { + if chat == "b" { + return SendResult{}, errors.New("resolving chat: boom") + } + return SendResult{Chat: chat, SyncID: "sync-" + chat}, nil + } + results, errs := fanout(context.Background(), targets, deliver) + + wantResults := []SendResult{ + {Chat: "a", SyncID: "sync-a"}, + {Chat: "c", SyncID: "sync-c"}, + } + if !reflect.DeepEqual(results, wantResults) { + t.Fatalf("results = %#v, want %#v", results, wantResults) + } + wantErrs := []SendError{{Chat: "b", Error: "resolving chat: boom"}} + if !reflect.DeepEqual(errs, wantErrs) { + t.Fatalf("errs = %#v, want %#v", errs, wantErrs) + } +} + +func TestWriteMultiSend(t *testing.T) { + tests := []struct { + name string + results []SendResult + errs []SendError + successStatus int + wantStatus int + wantOK bool + }{ + { + name: "all success sync", + results: []SendResult{{Chat: "a", SyncID: "s1"}, {Chat: "b", SyncID: "s2"}}, + successStatus: http.StatusOK, + wantStatus: http.StatusOK, + wantOK: true, + }, + { + name: "partial failure", + results: []SendResult{{Chat: "a", SyncID: "s1"}}, + errs: []SendError{{Chat: "b", Error: "boom"}}, + successStatus: http.StatusOK, + wantStatus: http.StatusOK, + wantOK: true, + }, + { + name: "all failed", + errs: []SendError{{Chat: "a", Error: "boom"}, {Chat: "b", Error: "boom2"}}, + successStatus: http.StatusOK, + wantStatus: http.StatusBadGateway, + wantOK: false, + }, + { + name: "async success 202", + results: []SendResult{{Chat: "a", RequestID: "r1", Queued: true}}, + successStatus: http.StatusAccepted, + wantStatus: http.StatusAccepted, + wantOK: true, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + rec := httptest.NewRecorder() + writeMultiSend(rec, tt.results, tt.errs, tt.successStatus) + + if rec.Code != tt.wantStatus { + t.Fatalf("status = %d, want %d", rec.Code, tt.wantStatus) + } + if ct := rec.Header().Get("Content-Type"); ct != "application/json" { + t.Fatalf("Content-Type = %q, want application/json", ct) + } + var got MultiSendResponse + if err := json.Unmarshal(rec.Body.Bytes(), &got); err != nil { + t.Fatalf("decode body: %v", err) + } + if got.OK != tt.wantOK { + t.Fatalf("ok = %v, want %v", got.OK, tt.wantOK) + } + if !reflect.DeepEqual(got.Results, tt.results) && !(len(got.Results) == 0 && len(tt.results) == 0) { + t.Fatalf("results = %#v, want %#v", got.Results, tt.results) + } + if !reflect.DeepEqual(got.Errors, tt.errs) && !(len(got.Errors) == 0 && len(tt.errs) == 0) { + t.Fatalf("errors = %#v, want %#v", got.Errors, tt.errs) + } + }) + } +} diff --git a/internal/server/server_test.go b/internal/server/server_test.go index c872b32..a80f5c5 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -72,6 +72,15 @@ func parseResponse(t *testing.T, w *httptest.ResponseRecorder) sendResponse { return resp } +func parseMultiResponse(t *testing.T, w *httptest.ResponseRecorder) MultiSendResponse { + t.Helper() + var resp MultiSendResponse + if err := json.NewDecoder(w.Body).Decode(&resp); err != nil { + t.Fatalf("decode response: %v (body: %s)", err, w.Body.String()) + } + return resp +} + // --- middleware --- func TestRequestID_Generated(t *testing.T) { @@ -265,12 +274,12 @@ func TestSend_JSON_TextOnly(t *testing.T) { if w.Code != 200 { t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) } - resp := parseResponse(t, w) + resp := parseMultiResponse(t, w) if !resp.OK { t.Fatalf("expected ok=true") } - if resp.SyncID != "test-sync-id" { - t.Fatalf("expected sync_id=test-sync-id, got %q", resp.SyncID) + if len(resp.Results) != 1 || resp.Results[0].Chat != "chat-1" || resp.Results[0].SyncID != "test-sync-id" { + t.Fatalf("expected results[0]=chat-1->test-sync-id, got %+v", resp.Results) } } @@ -402,14 +411,21 @@ func TestSend_JSON_InvalidJSON(t *testing.T) { } func TestSend_JSON_ChatAlias_NotFound(t *testing.T) { + // Chat resolution is a per-chat delivery concern under the unified contract: + // a single unresolved chat means the whole fan-out failed -> 502 with the + // failure in errors[0], not a request-level 400. srv := newTestServer([]ResolvedKey{{Name: "t", Key: "k"}}) body := `{"chat_id":"unknown-alias","message":"hi"}` w := doRequest(srv, "POST", "/api/v1/send", strings.NewReader(body), map[string]string{ "X-API-Key": "k", "Content-Type": "application/json", }) - if w.Code != 400 { - t.Fatalf("expected 400, got %d", w.Code) + if w.Code != 502 { + t.Fatalf("expected 502, got %d", w.Code) + } + resp := parseMultiResponse(t, w) + if resp.OK || len(resp.Errors) != 1 || resp.Errors[0].Chat != "unknown-alias" { + t.Fatalf("expected not-ok with unknown-alias error, got %+v", resp) } } @@ -648,9 +664,12 @@ func TestSend_UpstreamError(t *testing.T) { if w.Code != 502 { t.Fatalf("expected 502, got %d", w.Code) } - resp := parseResponse(t, w) - if !strings.Contains(resp.Error, "upstream error") { - t.Fatalf("expected upstream error, got: %s", resp.Error) + resp := parseMultiResponse(t, w) + if resp.OK || len(resp.Results) != 0 || len(resp.Errors) != 1 { + t.Fatalf("expected not-ok with 0 results, 1 error, got %+v", resp) + } + if resp.Errors[0].Chat != "chat-1" || !strings.Contains(resp.Errors[0].Error, "connection refused") { + t.Fatalf("expected chat-1 -> connection refused, got %+v", resp.Errors[0]) } } @@ -1447,12 +1466,12 @@ func TestSend_MultiBot_RequiresBot(t *testing.T) { "X-API-Key": "k", "Content-Type": "application/json", }) - if w.Code != 400 { - t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + if w.Code != 502 { + t.Fatalf("expected 502, got %d: %s", w.Code, w.Body.String()) } - resp := parseResponse(t, w) - if !strings.Contains(resp.Error, "bot is required") { - t.Errorf("expected 'bot is required', got: %s", resp.Error) + resp := parseMultiResponse(t, w) + if len(resp.Errors) != 1 || !strings.Contains(resp.Errors[0].Error, "bot is required") { + t.Errorf("expected 'bot is required' per-chat error, got: %+v", resp.Errors) } } @@ -1463,12 +1482,12 @@ func TestSend_MultiBot_UnknownBot(t *testing.T) { "X-API-Key": "k", "Content-Type": "application/json", }) - if w.Code != 400 { - t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + if w.Code != 502 { + t.Fatalf("expected 502, got %d: %s", w.Code, w.Body.String()) } - resp := parseResponse(t, w) - if !strings.Contains(resp.Error, "unknown bot") { - t.Errorf("expected 'unknown bot', got: %s", resp.Error) + resp := parseMultiResponse(t, w) + if len(resp.Errors) != 1 || !strings.Contains(resp.Errors[0].Error, "unknown bot") { + t.Errorf("expected 'unknown bot' per-chat error, got: %+v", resp.Errors) } } @@ -1482,9 +1501,9 @@ func TestSend_MultiBot_ValidBot(t *testing.T) { if w.Code != 200 { t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) } - resp := parseResponse(t, w) - if resp.SyncID != "sync-prod" { - t.Errorf("expected sync_id=sync-prod, got %q", resp.SyncID) + resp := parseMultiResponse(t, w) + if len(resp.Results) != 1 || resp.Results[0].SyncID != "sync-prod" { + t.Errorf("expected results[0].sync_id=sync-prod, got %+v", resp.Results) } } @@ -1574,12 +1593,12 @@ func TestSend_SingleBot_RejectsMismatchedChatBot(t *testing.T) { "X-API-Key": "k", "Content-Type": "application/json", }) - if w.Code != 400 { - t.Fatalf("expected 400 for mismatched chat-bound bot, got %d: %s", w.Code, w.Body.String()) + if w.Code != 502 { + t.Fatalf("expected 502 for mismatched chat-bound bot, got %d: %s", w.Code, w.Body.String()) } - resp := parseResponse(t, w) - if !strings.Contains(resp.Error, "not available") { - t.Errorf("expected 'not available' error, got: %s", resp.Error) + resp := parseMultiResponse(t, w) + if len(resp.Errors) != 1 || !strings.Contains(resp.Errors[0].Error, "not available") { + t.Errorf("expected 'not available' per-chat error, got: %+v", resp.Errors) } // Chat without binding → should pass @@ -1620,12 +1639,12 @@ func TestSend_SingleBot_EnvFlags_RejectsChatBoundBot(t *testing.T) { "X-API-Key": "k", "Content-Type": "application/json", }) - if w.Code != 400 { - t.Fatalf("expected 400 for chat-bound bot with unnamed sender, got %d: %s", w.Code, w.Body.String()) + if w.Code != 502 { + t.Fatalf("expected 502 for chat-bound bot with unnamed sender, got %d: %s", w.Code, w.Body.String()) } - resp := parseResponse(t, w) - if !strings.Contains(resp.Error, "not available") { - t.Errorf("expected 'not available' error, got: %s", resp.Error) + resp := parseMultiResponse(t, w) + if len(resp.Errors) != 1 || !strings.Contains(resp.Errors[0].Error, "not available") { + t.Errorf("expected 'not available' per-chat error, got: %+v", resp.Errors) } // Chat without binding → should pass @@ -1659,13 +1678,22 @@ func TestAlertmanager_MultiBot_RequiresBot(t *testing.T) { Annotations: map[string]string{"summary": "test"}, }) - // Without ?bot= — should fail + // Without ?bot= — bot resolution is now a per-chat delivery outcome, so an + // ambiguous bot fails the (single) target and surfaces as 502 with the error + // in errors[] (unified contract), not a request-level 400. w := doRequest(srv, "POST", "/api/v1/alertmanager", strings.NewReader(body), map[string]string{ "X-API-Key": "k", "Content-Type": "application/json", }) - if w.Code != 400 { - t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + if w.Code != 502 { + t.Fatalf("expected 502, got %d: %s", w.Code, w.Body.String()) + } + var resp MultiSendResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) + } + if len(resp.Errors) != 1 || !strings.Contains(resp.Errors[0].Error, "bot is required") { + t.Errorf("errors = %+v, want single bot-required error", resp.Errors) } // With ?bot=prod — should succeed @@ -1698,13 +1726,22 @@ func TestGrafana_MultiBot_RequiresBot(t *testing.T) { Annotations: map[string]string{"summary": "test"}, }) - // Without ?bot= — should fail + // Without ?bot= — bot resolution is now a per-chat delivery outcome, so an + // ambiguous bot fails the (single) target and surfaces as 502 with the error + // in errors[] (unified contract), not a request-level 400. w := doRequest(srv, "POST", "/api/v1/grafana", strings.NewReader(body), map[string]string{ "X-API-Key": "k", "Content-Type": "application/json", }) - if w.Code != 400 { - t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + if w.Code != 502 { + t.Fatalf("expected 502, got %d: %s", w.Code, w.Body.String()) + } + var resp MultiSendResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) + } + if len(resp.Errors) != 1 || !strings.Contains(resp.Errors[0].Error, "bot is required") { + t.Errorf("errors = %+v, want single bot-required error", resp.Errors) } // With ?bot=test — should succeed @@ -1788,12 +1825,15 @@ func TestAuth_BotSignature_PrivilegeEscalation(t *testing.T) { "X-Bot-Signature": "SIG_TEST", "Content-Type": "application/json", }) - if w.Code != 400 { - t.Fatalf("expected 400 for privilege escalation, got %d: %s", w.Code, w.Body.String()) + // The escalation is still refused (send never attempted); under the unified + // contract a rejected per-chat bot resolution surfaces as a 502 with the + // reason in errors[], not a request-level 400. + if w.Code != 502 { + t.Fatalf("expected 502 for privilege escalation, got %d: %s", w.Code, w.Body.String()) } - resp := parseResponse(t, w) - if !strings.Contains(resp.Error, "does not match authenticated bot") { - t.Errorf("expected mismatch error, got: %s", resp.Error) + resp := parseMultiResponse(t, w) + if len(resp.Errors) != 1 || !strings.Contains(resp.Errors[0].Error, "does not match authenticated bot") { + t.Errorf("expected mismatch per-chat error, got: %+v", resp.Errors) } } @@ -2118,15 +2158,21 @@ func TestSend_AsyncMode_DirectPublish(t *testing.T) { t.Fatalf("expected 202, got %d: %s", w.Code, w.Body.String()) } - resp := parseResponse(t, w) + resp := parseMultiResponse(t, w) if !resp.OK { t.Error("expected ok=true") } - if !resp.Queued { + if len(resp.Results) != 1 || len(resp.Errors) != 0 { + t.Fatalf("response = %+v, want 1 result, 0 errors", resp) + } + if !resp.Results[0].Queued { t.Error("expected queued=true") } - if resp.RequestID != "test-request-id" { - t.Errorf("request_id = %q, want %q", resp.RequestID, "test-request-id") + if resp.Results[0].RequestID != "test-request-id" { + t.Errorf("request_id = %q, want %q", resp.Results[0].RequestID, "test-request-id") + } + if resp.Results[0].Chat != "00000000-0000-0000-0000-000000000002" { + t.Errorf("chat = %q, want target chat", resp.Results[0].Chat) } if capturedPayload == nil { @@ -2245,9 +2291,12 @@ func TestSend_AsyncMode_EnqueueError(t *testing.T) { if w.Code != 502 { t.Fatalf("expected 502, got %d: %s", w.Code, w.Body.String()) } - resp := parseResponse(t, w) - if !strings.Contains(resp.Error, "enqueue error") { - t.Errorf("expected 'enqueue error', got: %s", resp.Error) + resp := parseMultiResponse(t, w) + if resp.OK || len(resp.Results) != 0 || len(resp.Errors) != 1 { + t.Fatalf("response = %+v, want not-ok with 0 results, 1 error", resp) + } + if !strings.Contains(resp.Errors[0].Error, "broker connection refused") { + t.Errorf("expected 'broker connection refused', got: %s", resp.Errors[0].Error) } } From 2dae88959adce0f5143605508f02c1870fc23ad4 Mon Sep 17 00:00:00 2001 From: Sergey Lavrinenko Date: Thu, 9 Jul 2026 17:06:23 +0300 Subject: [PATCH 2/3] fix: per-chat bot resolution in CLI multi-bot send; reject empty ?chat_id on webhooks --- internal/cmd/send.go | 126 ++++++++++++------ internal/cmd/send_test.go | 122 +++++++++++++++++ internal/config/config.go | 24 ++++ internal/server/handler_alertmanager.go | 6 + internal/server/handler_gitlab.go | 6 + internal/server/handler_gitlab_test.go | 21 +++ internal/server/handler_grafana.go | 6 + .../server/handler_webhook_multichat_test.go | 27 ++++ 8 files changed, 299 insertions(+), 39 deletions(-) diff --git a/internal/cmd/send.go b/internal/cmd/send.go index c249be7..b104ad7 100644 --- a/internal/cmd/send.go +++ b/internal/cmd/send.go @@ -122,22 +122,24 @@ Options: return fmt.Errorf("--secret and --token are mutually exclusive") } - cfg, err := config.Load(flags) + cfg, err := config.LoadForSend(flags) if err != nil { return err } // Determine target chats up front so an empty --chat-id fails fast (before // authenticating or reading files). A comma-separated --chat-id (--chat-id a,b,c) - // fans the same message out to each chat; an empty value falls back to the - // single/default chat auto-selection. The chats are resolved to UUIDs later, - // per target, so a bad alias fails only that chat rather than the whole command. + // fans the same message out to each chat. Tokens are kept as given (aliases, not + // yet resolved to UUIDs) so each chat's bot binding is honoured per target in + // multi-bot configs. An empty value falls back to the single/default configured + // chat by alias, preserving that chat's bot binding. chats := parseChatIDs(cfg.ChatID) if len(chats) == 0 { - if err := cfg.RequireChatID(); err != nil { - return err + alias, ok := cfg.SingleOrDefaultChatAlias() + if !ok { + return cfg.RequireChatID() } - chats = []string{cfg.ChatID} + chats = []string{alias} } // Validate status @@ -228,56 +230,73 @@ Options: ment = raw } - // Authenticate - tok, cache, err := authenticate(cfg) - if err != nil { - return err - } - - client := botapi.NewClient(cfg.Host, tok, cfg.HTTPTimeout()) - - // Run inline mentions parser. - // Use refreshableClientResolver so the token is refreshed on 401 — the - // cached token may have expired since it was stored. - parseResult := mentions.Parse( - context.Background(), - message, - ment, - !noParse, - &refreshableClientResolver{client: client, cfg: cfg, cache: cache}, - ) - for _, e := range parseResult.Errors { - fmt.Fprintf(deps.Stderr, "warning: mention %s: %s\n", e.Kind, e.Cause) + // Fan out to every target chat, resolving the bot bound to each chat so a + // multi-bot config delivers each chat via its own bot/token (matching the + // sync /send handler). Per-bot state — token, HTTP client, and the inline + // mentions parse (email→huid lookups are per-host) — is built once per bot and + // reused across that bot's chats. Delivery is best-effort and per-chat + // independent: a failed chat is recorded, the rest still send, and a non-zero + // exit is returned only if every chat failed. + execs := make(map[string]*botExec) + getExec := func(botName string) (*botExec, error) { + if be, ok := execs[botName]; ok { + return be, nil + } + botCfg := *cfg + if cfg.IsMultiBot() { + if botName == "" { + return nil, fmt.Errorf("no bot is bound to this chat; add a bot binding in the chats section or pass --bot") + } + if aerr := (&botCfg).ApplyChatBot(botName); aerr != nil { + return nil, aerr + } + } + tok, cache, aerr := authenticate(&botCfg) + if aerr != nil { + return nil, aerr + } + client := botapi.NewClient(botCfg.Host, tok, botCfg.HTTPTimeout()) + // Inline mentions parser (per-bot resolver; token refreshed on 401). + pr := mentions.Parse( + context.Background(), + message, + ment, + !noParse, + &refreshableClientResolver{client: client, cfg: &botCfg, cache: cache}, + ) + for _, e := range pr.Errors { + fmt.Fprintf(deps.Stderr, "warning: mention %s: %s\n", e.Kind, e.Cause) + } + be := &botExec{client: client, cfg: &botCfg, cache: cache, message: pr.Message, mentions: pr.Mentions} + execs[botName] = be + return be, nil } - // Fan out to every target chat via the single configured bot. The CLI uses one - // bot/token per command (unlike the sync /send handler, which resolves a bot - // per chat), so mentions are parsed once above and the resolved message is - // reused across chats with only the target chat swapped. Delivery is - // best-effort and per-chat independent: a failed chat is recorded, the rest - // still send, and the exit code is non-zero only if every chat failed. results := make([]sendCmdResult, 0, len(chats)) for _, chat := range chats { - chatID, rerr := cfg.ResolveChatAlias(chat) + chatID, botName, rerr := resolveSendTarget(cfg, chat) if rerr != nil { results = append(results, sendCmdResult{Chat: chat, Error: rerr.Error()}) continue } - + be, eerr := getExec(botName) + if eerr != nil { + results = append(results, sendCmdResult{Chat: chat, Error: eerr.Error()}) + continue + } sr := botapi.BuildSendRequest(&botapi.SendParams{ ChatID: chatID, - Message: parseResult.Message, + Message: be.message, Status: status, File: fileAttachment, Metadata: meta, - Mentions: parseResult.Mentions, + Mentions: be.mentions, Silent: silent, Stealth: stealth, ForceDND: forceDND, NoNotify: noNotify, }) - - syncID, serr := sendWithRefresh(client, cfg, cache, sr) + syncID, serr := sendWithRefresh(be.client, be.cfg, be.cache, sr) if serr != nil { results = append(results, sendCmdResult{Chat: chat, Error: serr.Error()}) continue @@ -288,6 +307,35 @@ Options: return printSendResults(deps.Stdout, cfg.Format, results) } +// botExec is a per-bot send context: an authenticated client plus the message and +// mentions resolved with that bot's (per-host) mentions resolver. +type botExec struct { + client *botapi.Client + cfg *config.Config + cache token.Cache + message string + mentions json.RawMessage +} + +// resolveSendTarget maps a --chat-id token to its chat UUID and the bot bound to +// it. A raw UUID, or any single-bot config, has no per-chat bot and returns +// botName "" (the caller then uses the command's single bot). In multi-bot mode +// an alias's bound bot is returned so each chat sends via its own bot; an alias +// with no binding returns "" and getExec reports it as an error for that chat. +func resolveSendTarget(cfg *config.Config, chat string) (chatID, botName string, err error) { + if config.IsUUID(chat) { + return chat, "", nil + } + chatID, err = cfg.ResolveChatAlias(chat) + if err != nil { + return "", "", err + } + if !cfg.IsMultiBot() { + return chatID, "", nil + } + return chatID, cfg.Chats[chat].Bot, nil +} + // sendWithRefresh posts one SendRequest and returns its sync_id, refreshing the // token once on a 401 exactly as the original single-chat path did. func sendWithRefresh(client *botapi.Client, cfg *config.Config, cache token.Cache, sr *botapi.SendRequest) (string, error) { diff --git a/internal/cmd/send_test.go b/internal/cmd/send_test.go index cadd0a8..508bcc4 100644 --- a/internal/cmd/send_test.go +++ b/internal/cmd/send_test.go @@ -738,3 +738,125 @@ bots: t.Errorf("expected '--mentions must be a JSON array' error, got: %v", err) } } + +// --- Bug fix: CLI multi-chat honours per-chat bot binding in multi-bot configs --- + +// mockBotxMultiBot accepts several bearer tokens and records, per group_chat_id, +// which token was used — so a test can assert each chat was delivered via its +// bound bot's credentials. +type mockBotxMultiBot struct { + mu sync.Mutex + byChat map[string]string // group_chat_id -> bearer token seen + tokens map[string]bool // accepted bearer tokens + srv *httptest.Server +} + +func newMockBotxMultiBot(tokens map[string]bool) *mockBotxMultiBot { + m := &mockBotxMultiBot{byChat: map[string]string{}, tokens: tokens} + mux := http.NewServeMux() + mux.HandleFunc("POST /api/v4/botx/notifications/direct", func(w http.ResponseWriter, r *http.Request) { + tok := strings.TrimPrefix(r.Header.Get("Authorization"), "Bearer ") + if !m.tokens[tok] { + w.WriteHeader(http.StatusUnauthorized) + return + } + body, _ := io.ReadAll(r.Body) + var req struct { + GroupChatID string `json:"group_chat_id"` + } + _ = json.Unmarshal(body, &req) + m.mu.Lock() + m.byChat[req.GroupChatID] = tok + m.mu.Unlock() + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusAccepted) + fmt.Fprintf(w, `{"status":"ok","result":{"sync_id":"sync-%s"}}`, req.GroupChatID) + }) + m.srv = httptest.NewServer(mux) + return m +} + +func (m *mockBotxMultiBot) close() { m.srv.Close() } + +func (m *mockBotxMultiBot) tokenFor(chat string) string { + m.mu.Lock() + defer m.mu.Unlock() + return m.byChat[chat] +} + +// multiBotConfig has two bots and two chat aliases, each bound to a different bot. +func multiBotConfig(t *testing.T, host string) string { + return writeTestConfig(t, fmt.Sprintf(` +bots: + bota: + host: %s + id: 00000000-0000-0000-0000-0000000000a1 + token: token-a + botb: + host: %s + id: 00000000-0000-0000-0000-0000000000b1 + token: token-b +chats: + deploy: + id: %s + bot: bota + alerts: + id: %s + bot: botb +`, host, host, chatA, chatB)) +} + +// A multi-bot config with --chat-id deploy,alerts must (1) not fail at config +// load (the previous bug rejected the comma value as "multiple bots configured") +// and (2) deliver each chat via its own bound bot's token. +func TestSend_MultiBot_PerChatBot(t *testing.T) { + mock := newMockBotxMultiBot(map[string]bool{"token-a": true, "token-b": true}) + defer mock.close() + + deps, _, _ := testDeps() + deps.IsTerminal = true + + err := runSend([]string{ + "--config", multiBotConfig(t, mock.srv.URL), + "--chat-id", "deploy,alerts", + "hi", + }, deps) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got := mock.tokenFor(chatA); got != "token-a" { + t.Errorf("chat deploy (%s) delivered with token %q, want token-a", chatA, got) + } + if got := mock.tokenFor(chatB); got != "token-b" { + t.Errorf("chat alerts (%s) delivered with token %q, want token-b", chatB, got) + } +} + +// In multi-bot mode a target with no bound bot (here a raw UUID) fails only that +// chat; bound chats still deliver and the command does not error overall. +func TestSend_MultiBot_UnboundChatFailsOnlyThatChat(t *testing.T) { + mock := newMockBotxMultiBot(map[string]bool{"token-a": true, "token-b": true}) + defer mock.close() + + deps, stdout, _ := testDeps() + deps.IsTerminal = true + + err := runSend([]string{ + "--config", multiBotConfig(t, mock.srv.URL), + "--chat-id", "deploy," + chatC, // chatC: raw UUID, no bot binding + "hi", + }, deps) + if err != nil { + t.Fatalf("partial success must not return an error, got: %v", err) + } + if got := mock.tokenFor(chatA); got != "token-a" { + t.Errorf("deploy should deliver via token-a, got %q", got) + } + if got := mock.tokenFor(chatC); got != "" { + t.Errorf("unbound chatC must not be delivered, but saw token %q", got) + } + out := stdout.String() + if !strings.Contains(out, chatC+": ERROR") { + t.Errorf("expected an ERROR line for unbound chatC, got %q", out) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index b41a01d..5c9894f 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -447,6 +447,30 @@ func LoadForServe(flags Flags) (*Config, error) { return cfg, nil } +// LoadForSend reads configuration for the send command. Like LoadForServe it +// tolerates multi-bot configs by deferring bot resolution, so a multi-chat send +// (--chat-id a,b,c) can resolve the bot bound to each target chat independently +// instead of forcing a single bot for the whole command. In single-bot configs +// (or with an explicit --bot / credentials) the bot is resolved as before. +func LoadForSend(flags Flags) (*Config, error) { + return LoadForServe(flags) +} + +// SingleOrDefaultChatAlias returns the alias of the sole configured chat, or the +// default chat, so a bare send (no --chat-id) can resolve a target by alias and +// still honour that chat's bot binding. ok is false when neither applies. +func (c *Config) SingleOrDefaultChatAlias() (alias string, ok bool) { + if len(c.Chats) == 1 { + for a := range c.Chats { + return a, true + } + } + if a, chat := c.DefaultChat(); a != "" && chat.ID != "" { + return a, true + } + return "", false +} + // IsMultiBot returns true if the config was loaded in multi-bot serve mode. func (c *Config) IsMultiBot() bool { return c.multiBot diff --git a/internal/server/handler_alertmanager.go b/internal/server/handler_alertmanager.go index 2d81b13..043c3e6 100644 --- a/internal/server/handler_alertmanager.go +++ b/internal/server/handler_alertmanager.go @@ -82,6 +82,12 @@ func (s *Server) handleAlertmanager(w http.ResponseWriter, r *http.Request) { // keeps its single-default behaviour; the response is the uniform // MultiSendResponse in every case (results[0] for a single chat). targets := parseChatIDs(r.URL.Query().Get("chat_id")) + // An explicitly-present but empty chat_id (?chat_id= / ?chat_id=,,) is a + // request error, not a silent fall-back to the default chat. + if len(targets) == 0 && r.URL.Query().Has("chat_id") { + writeError(w, http.StatusBadRequest, "chat_id is empty: provide at least one chat, or omit chat_id to use the default") + return + } if len(targets) == 0 { if single := s.amCfg.singleChat(s.cfg.DefaultChatAlias); single != "" { targets = []string{single} diff --git a/internal/server/handler_gitlab.go b/internal/server/handler_gitlab.go index f152cbe..a5a829b 100644 --- a/internal/server/handler_gitlab.go +++ b/internal/server/handler_gitlab.go @@ -319,6 +319,12 @@ func (s *Server) handleGitlab(w http.ResponseWriter, r *http.Request) { // keeps its single-chat default behaviour (routes is optional, so its absence // must not change existing deployments). queryChats := parseChatIDs(r.URL.Query().Get("chat_id")) + // An explicitly-present but empty chat_id (?chat_id= / ?chat_id=,,) is a + // request error, not a silent fall-back to routes or the default chat. + if len(queryChats) == 0 && r.URL.Query().Has("chat_id") { + writeError(w, http.StatusBadRequest, "chat_id is empty: provide at least one chat, or omit chat_id to use routing/default") + return + } if len(queryChats) > 0 || len(s.gitCfg.Routes) == 0 { targets := queryChats if len(targets) == 0 { diff --git a/internal/server/handler_gitlab_test.go b/internal/server/handler_gitlab_test.go index 37c1009..7eeec51 100644 --- a/internal/server/handler_gitlab_test.go +++ b/internal/server/handler_gitlab_test.go @@ -1653,3 +1653,24 @@ func TestGitlab_DefaultTokenUnchangedWithSenders(t *testing.T) { } }) } + +// TestGitlab_EmptyChatIDIsRequestError: an explicit-but-empty ?chat_id (?chat_id= +// or ?chat_id=,,) is a 400 request error, not a silent fall-through to the routing +// engine or default chat. +func TestGitlab_EmptyChatIDIsRequestError(t *testing.T) { + routes := mustCompileRoutes(t, []config.GitlabRouteYAMLConfig{ + {Match: map[string][]string{"project": {"myproj"}}, Chats: []string{"chatA"}}, + }) + for _, q := range []string{"?chat_id=", "?chat_id=,,"} { + t.Run(q, func(t *testing.T) { + srv, cap := newGitlabFanoutServer(t, &GitlabConfig{SecretToken: "secret", Routes: routes}, okSend, nil) + w := doRequest(srv, "POST", "/api/v1/gitlab"+q, strings.NewReader(mrOpenPayload), gitlabHeaders("secret")) + if w.Code != 400 { + t.Fatalf("status = %d, want 400 (body: %s)", w.Code, w.Body.String()) + } + if cap.count() != 0 { + t.Errorf("send count = %d, want 0 (must not fall through to routes)", cap.count()) + } + }) + } +} diff --git a/internal/server/handler_grafana.go b/internal/server/handler_grafana.go index 1c6dff6..fcbd3d0 100644 --- a/internal/server/handler_grafana.go +++ b/internal/server/handler_grafana.go @@ -93,6 +93,12 @@ func (s *Server) handleGrafana(w http.ResponseWriter, r *http.Request) { // keeps its single-default behaviour; the response is the uniform // MultiSendResponse in every case (results[0] for a single chat). targets := parseChatIDs(r.URL.Query().Get("chat_id")) + // An explicitly-present but empty chat_id (?chat_id= / ?chat_id=,,) is a + // request error, not a silent fall-back to the default chat. + if len(targets) == 0 && r.URL.Query().Has("chat_id") { + writeError(w, http.StatusBadRequest, "chat_id is empty: provide at least one chat, or omit chat_id to use the default") + return + } if len(targets) == 0 { if single := s.grCfg.singleChat(s.cfg.DefaultChatAlias); single != "" { targets = []string{single} diff --git a/internal/server/handler_webhook_multichat_test.go b/internal/server/handler_webhook_multichat_test.go index 236642f..b205ed3 100644 --- a/internal/server/handler_webhook_multichat_test.go +++ b/internal/server/handler_webhook_multichat_test.go @@ -299,3 +299,30 @@ func TestWebhook_DefaultChatNoQuery(t *testing.T) { } } } + +// TestWebhook_EmptyChatIDIsRequestError: an explicitly-present but empty chat_id +// (?chat_id= or ?chat_id=,,) is a 400 request error, not a silent fall-back to the +// configured default chat. +func TestWebhook_EmptyChatIDIsRequestError(t *testing.T) { + for _, s := range webhookSurfaces(t) { + for _, q := range []string{"?chat_id=", "?chat_id=,,", "?chat_id=+,+"} { + t.Run(s.name+" "+q, func(t *testing.T) { + cap := &captureSend{} + send := func(ctx context.Context, p *SendPayload) (string, error) { + cap.record(p) + return "sync-1", nil + } + // default-chat is configured, so a *missing* chat_id would deliver + // there; an explicit-but-empty chat_id must instead 400. + srv := s.newServer(t, "default-chat", "", send, selfChatResolver()) + w := doRequest(srv, "POST", s.path+q, strings.NewReader(s.payload), webhookHeaders()) + if w.Code != 400 { + t.Fatalf("status = %d, want 400 (body: %s)", w.Code, w.Body.String()) + } + if cap.count() != 0 { + t.Errorf("send count = %d, want 0 (must not fall back to default)", cap.count()) + } + }) + } + } +} From 958e903d002abdbc69e3df29a0f6ab43b44abaf3 Mon Sep 17 00:00:00 2001 From: Sergey Lavrinenko Date: Thu, 9 Jul 2026 18:11:03 +0300 Subject: [PATCH 3/3] feat: sender-scoped ?chat_id filter for GitLab webhook --- README.md | 2 +- docs/configuration.md | 4 +- docs/integrations.md | 39 +++- examples/gitlab/README.md | 39 +++- examples/gitlab/config-senders.yaml | 14 +- internal/server/api/openapi.yaml | 38 +++- internal/server/handler_gitlab.go | 77 ++++++- internal/server/handler_gitlab_test.go | 285 ++++++++++++++++++++++++- internal/server/handler_send.go | 2 +- internal/server/multisend.go | 3 +- internal/server/server_test.go | 40 ++++ 11 files changed, 503 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index e0d54de..3a393d9 100644 --- a/README.md +++ b/README.md @@ -173,7 +173,7 @@ chats: ## Интеграции -В режиме веб-сервера есть методы для интеграции с alertmanager, grafana и gitlab (универсальный приёмник любых событий GitLab с фильтрами `only`/`exclude`, шаблонами по типам и `error_events`). Несколько команд могут делить один GitLab-эндпоинт с изоляцией по своим `X-Gitlab-Token` — см. [senders](docs/integrations.md#изоляция-команд-senders-несколько-токенов) и пример [examples/gitlab/config-senders.yaml](examples/gitlab/config-senders.yaml). +В режиме веб-сервера есть методы для интеграции с alertmanager, grafana и gitlab (универсальный приёмник любых событий GitLab с фильтрами `only`/`exclude`, шаблонами по типам и `error_events`). Несколько команд могут делить один GitLab-эндпоинт с изоляцией по своим `X-Gitlab-Token`; при этом `?chat_id=` работает как фильтр внутри разрешённого набора чатов sender'а (400 на пустой, 403 на выход за scope, эквивалентность alias/UUID) — см. [senders](docs/integrations.md#изоляция-команд-senders-несколько-токенов) и пример [examples/gitlab/config-senders.yaml](examples/gitlab/config-senders.yaml) ([Per-team tokens](examples/gitlab/README.md#per-team-tokens-senders)). Пример конфига alertmanager: diff --git a/docs/configuration.md b/docs/configuration.md index 15a4615..967c191 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -105,7 +105,7 @@ server: | `template_files` | Мапа `event-ключ → путь к файлу шаблона`. Один ключ нельзя задать и в `templates`, и в `template_files`. | | `error_events` | Список event-ключей, доставляемых с `notification.status=error` (та же грануляция матчинга). | | `routes` | Опциональный упорядоченный список правил роутинга. Событие уходит в чаты **всех** совпавших правил (объединение+дедуп), `stop:true` обрывает перебор. Каждое правило: `match` (селектор → паттерны glob/`/regex/`; `event` — по event-ключу), `chats` (непустой список алиасов/UUID), `stop`. Без секции — прежнее поведение (один чат). Подробнее и приоритет чатов — в [docs/integrations.md](integrations.md#роутинг-событий-по-чатам-routes). | -| `senders` | Опциональный список дополнительных входящих токенов с жёсткой привязкой к чатам (изоляция команд). Каждый элемент: `secret`/`secret_token` (ссылка `literal`/`env:`/`vault:`, обязателен) и непустой `chats` (алиасы/UUID существующих чатов). Совпал sender-токен → событие уходит **только** в его `chats`; `?chat_id`, `?bot`, `routes` и `default_chat_id` игнорируются. Глобальные `events.only/exclude`, `templates` и `error_events` применяются как обычно. Дубликаты разрезолвленных токенов (sender↔sender, sender↔`secret`) — ошибка на старте. Подробнее — в [docs/integrations.md](integrations.md#изоляция-команд-senders-несколько-токенов). | +| `senders` | Опциональный список дополнительных входящих токенов с жёсткой привязкой к чатам (изоляция команд). Каждый элемент: `secret`/`secret_token` (ссылка `literal`/`env:`/`vault:`, обязателен) и непустой `chats` (алиасы/UUID существующих чатов). Совпал sender-токен → событие уходит в его `chats`; `?bot`, `routes` и `default_chat_id` игнорируются. `?chat_id` работает как **фильтр внутри scope**: отсутствует → все `chats`; непустой subset → только эти чаты (эквивалентность alias↔UUID); чат вне `chats` → `403`; явно пустой (`?chat_id=`) → `400`. Глобальные `events.only/exclude`, `templates` и `error_events` применяются как обычно. Дубликаты разрезолвленных токенов (sender↔sender, sender↔`secret`) — ошибка на старте. Подробнее — в [docs/integrations.md](integrations.md#изоляция-команд-senders-несколько-токенов). | ## Переменные окружения @@ -237,7 +237,7 @@ express-botx config chat list # покажет (defa Резолв конкретного чата/бота, если он не удался, — пер-чатовая ошибка в `errors[]` (а не общий `400`); если упали все чаты — `502`. - `/alertmanager`, `/grafana`: `?chat_id=` → `default_chat_id` из конфига вебхука → чат по умолчанию → единственный чат → пустой набор даёт `400`; пер-чатовые сбои — в `errors[]`, всё упало — `502` -- `/gitlab`: `?chat_id=` → `routes` (все совпавшие правила, объединение+дедуп) → `default_chat_id` → чат по умолчанию → единственный чат → `200 {ignored}`; при совпадении sender-токена (`server.gitlab.senders`) цели — всегда `chats` этого sender'а, остальное игнорируется +- `/gitlab`: `?chat_id=` → `routes` (все совпавшие правила, объединение+дедуп) → `default_chat_id` → чат по умолчанию → единственный чат → `200 {ignored}`; при совпадении sender-токена (`server.gitlab.senders`) цели — `chats` этого sender'а, а `?chat_id=` фильтрует внутри них (subset → только они; вне scope → `403`; явно пустой → `400`), `?bot`/`routes`/`default_chat_id` игнорируются Ответ всех эндпоинтов — единый `MultiSendResponse` (см. [Мульти-чат](integrations.md#мульти-чат-и-единый-ответ-multisendresponse)). diff --git a/docs/integrations.md b/docs/integrations.md index 6ac2a36..33ce4b9 100644 --- a/docs/integrations.md +++ b/docs/integrations.md @@ -480,10 +480,11 @@ server: **Резолв аутентификации.** Входящий `X-Gitlab-Token` сверяется со всеми sender-токенами и с дефолтным `secret` (constant-time, без early-exit): -- совпал **sender-токен** → событие уходит **только** в `chats` этого sender'а - (fan-out, best-effort — как у `routes`); `?chat_id=`, `?bot=`, `routes` и - `default_chat_id` **игнорируются** — команда A не может отправить в чаты - команды B или от имени чужого бота, даже подставив `?chat_id`/`?bot`; +- совпал **sender-токен** → событие уходит в `chats` этого sender'а (fan-out, + best-effort — как у `routes`). `?chat_id=` при этом работает как **фильтр + внутри разрешённого набора** (см. ниже); `?bot=`, `routes` и `default_chat_id` + **игнорируются** — команда A не может отправить от имени чужого бота или в чат + вне своего scope, даже подставив `?bot`/`?chat_id`; - совпал **дефолтный `secret`** → прежнее поведение без изменений (`?chat_id` → `routes` → `default_chat_id` → …); - не совпал ни один → `401`. @@ -493,6 +494,36 @@ sender-токенами и с дефолтным `secret` (constant-time, без {ignored}` и для sender'а). Своих `routes`/фильтров/шаблонов у sender'а нет — его скоуп только чаты. +**`?chat_id=` как фильтр внутри scope.** Один sender-токен остаётся изолированным +в рамках команды, но команда может направлять конкретные вебхуки в нужный чат +внутри своего разрешённого набора: + +- `?chat_id` **отсутствует** → событие уходит во **все** `chats` sender'а + (прежнее поведение); +- `?chat_id=a,b` (непустой) → доставка **только** в перечисленные чаты, но + **только если каждый** входит в `chats` sender'а (синтаксис общий: список через + запятую, trim, dedup, сохранение порядка); +- хотя бы один чат из `?chat_id` **вне** `chats` sender'а → `403 Forbidden` + (`{"ok":false,"error":"chat \"…\" is outside this token's allowed chats"}`), + ничего не отправляется — попытка выйти за scope, а не тихое игнорирование; +- `?chat_id` **явно пустой** (`?chat_id=`, `?chat_id=,,`) → `400` request-level, + ничего не отправляется; +- сравнение эквивалентно по **алиасу и UUID**: `chats: [team-a-alerts]` разрешает + и `?chat_id=team-a-alerts`, и UUID, в который этот алиас резолвится (но не чужой + алиас/UUID). Направление не важно — если в `chats` указан UUID, `?chat_id=` + можно передать алиасом, и наоборот. + +``` +# всё разрешённое (оба чата sender'а): +POST /api/v1/gitlab X-Gitlab-Token: +# только один чат из scope: +POST /api/v1/gitlab?chat_id=team-a-alerts X-Gitlab-Token: → 200 +# чужой чат → отказ: +POST /api/v1/gitlab?chat_id=team-b X-Gitlab-Token: → 403 +# пустой chat_id → ошибка запроса: +POST /api/v1/gitlab?chat_id= X-Gitlab-Token: → 400 +``` + **Правила конфигурации:** - Каждый sender: непустой `secret`/`secret_token` + непустой `chats` diff --git a/examples/gitlab/README.md b/examples/gitlab/README.md index e0d7ff3..62c5770 100644 --- a/examples/gitlab/README.md +++ b/examples/gitlab/README.md @@ -91,12 +91,37 @@ for the full model, response format and chat-selection priority. `config-senders.yaml` adds `server.gitlab.senders` — extra incoming `X-Gitlab-Token` values, each hard-bound to its own chats. A request -authenticated with a sender token is delivered **only** to that sender's chats -(`?chat_id=`, `routes` and `default_chat_id` are ignored), so teams sharing one -endpoint cannot post into each other's chats. The global `events` filter, -templates and `error_events` apply as usual; the default `secret` keeps its -ordinary behaviour and may be omitted when only senders are used. Token values -are `env:`/`vault:` references — the shared YAML never contains plaintext -secrets, and duplicate resolved tokens fail at startup. See the +authenticated with a sender token is delivered to that sender's chats (`?bot=`, +`routes` and `default_chat_id` are ignored), so teams sharing one endpoint +cannot post into each other's chats or as another bot. The global `events` +filter, templates and `error_events` apply as usual; the default `secret` keeps +its ordinary behaviour and may be omitted when only senders are used. Token +values are `env:`/`vault:` references — the shared YAML never contains plaintext +secrets, and duplicate resolved tokens fail at startup. + +`?chat_id=` is honoured for sender tokens as a **filter within** the allowed +chats: omit it to reach all of the sender's chats, or pass a subset to target +only those (aliases and the UUIDs they resolve to are equivalent). A chat +outside the sender's scope is refused with `403`, and an explicitly-empty +`?chat_id=` is a `400` — in both cases nothing is sent: + +```bash +# team-b token -> both team-b chats +curl -X POST "http://localhost:8080/api/v1/gitlab" \ + -H "X-Gitlab-Token: " -H "Content-Type: application/json" \ + -d @webhook-merge-request-open.json + +# team-b token, only the alerts chat (subset of scope) +curl -X POST "http://localhost:8080/api/v1/gitlab?chat_id=team-b-alerts" \ + -H "X-Gitlab-Token: " -H "Content-Type: application/json" \ + -d @webhook-pipeline-failed.json + +# team-b token targeting team-a -> 403 Forbidden, nothing sent +curl -X POST "http://localhost:8080/api/v1/gitlab?chat_id=team-a" \ + -H "X-Gitlab-Token: " -H "Content-Type: application/json" \ + -d @webhook-push.json +``` + +See the [senders section](../../docs/integrations.md#изоляция-команд-senders-несколько-токенов) for details. diff --git a/examples/gitlab/config-senders.yaml b/examples/gitlab/config-senders.yaml index 2d75aec..8473fb5 100644 --- a/examples/gitlab/config-senders.yaml +++ b/examples/gitlab/config-senders.yaml @@ -2,11 +2,19 @@ # with per-sender tokens (`server.gitlab.senders`) — team isolation. # # Several teams share one endpoint, each with its OWN X-Gitlab-Token bound to -# its own chats. An event authenticated with a sender token is delivered ONLY -# to that sender's chats: ?chat_id=, routes and default_chat_id are ignored -# for it, so team A cannot post into team B's chats even on purpose. The +# its own chats. An event authenticated with a sender token is delivered to +# that sender's chats: ?bot=, routes and default_chat_id are ignored for it, so +# team A cannot post into team B's chats or as another bot even on purpose. The # global events filter, templates and error_events still apply. # +# ?chat_id= works as a FILTER WITHIN the sender's allowed chats: +# - omitted -> delivered to all of the sender's chats +# - ?chat_id= -> delivered only to those chats (alias<->UUID equivalent) +# - chat outside scope -> 403 Forbidden, nothing sent +# - ?chat_id= (empty) -> 400 Bad Request, nothing sent +# So team B (chats: team-b, team-b-alerts) can send a specific webhook only to +# team-b-alerts with ?chat_id=team-b-alerts, but ?chat_id=team-a gives 403. +# # The default `secret` stays fully functional (mixed mode): webhooks using it # keep the ordinary behaviour (?chat_id -> routes -> default_chat_id -> ...). # It may also be omitted entirely — then only the sender tokens authenticate. diff --git a/internal/server/api/openapi.yaml b/internal/server/api/openapi.yaml index 1dac3e2..1ed23dc 100644 --- a/internal/server/api/openapi.yaml +++ b/internal/server/api/openapi.yaml @@ -664,11 +664,15 @@ paths: (multi-tenant): besides the default `server.gitlab.secret`, each entry of `server.gitlab.senders` defines its own token bound to an isolated set of chats. A request authenticated with a sender token is delivered - **only** to that sender's chats (fan-out, `MultiSendResponse`); - `?chat_id=`, `?bot=`, `routes` and `default_chat_id` are ignored for it, while - the `events` filter, templates and `error_events` apply as usual. The - response behaviour is otherwise unchanged; any token that matches - neither the default secret nor a sender secret gets `401`. + to that sender's chats (fan-out, `MultiSendResponse`); `?bot=`, `routes` + and `default_chat_id` are ignored for it, while the `events` filter, + templates and `error_events` apply as usual. `?chat_id=` acts as a + **filter within the sender's allowed chats**: omitted delivers to every + sender chat; a non-empty comma-separated subset delivers only to those + chats (aliases and the UUIDs they resolve to are treated as equivalent); + any requested chat outside the sender's scope is refused with `403` + (nothing is sent); an explicitly-empty `?chat_id=` is a `400`. Any token + that matches neither the default secret nor a sender secret gets `401`. Chat resolution priority (default-token requests): 1. `?chat_id=` query parameter (overrides routing entirely; @@ -678,8 +682,9 @@ paths: 4. Global default chat (`default: true` in chats section) 5. Single chat alias from config - Sender-token requests skip this list: the targets are always the - sender's configured chats. + Sender-token requests skip this list: the targets are the sender's + configured chats, optionally narrowed by `?chat_id=` to a subset of them + (a chat outside the scope yields `403`). **⚠️ Breaking change (response format).** Every delivered event now returns a `MultiSendResponse` (`results`/`errors` envelope), including single-chat @@ -690,12 +695,17 @@ paths: parameters: - name: chat_id in: query - description: Target chat UUID or alias; comma-separated for multi-chat fan-out (overrides config default) + description: | + Target chat UUID or alias; comma-separated for multi-chat fan-out + (overrides config default). For a sender token it instead filters + within the sender's allowed chats: a subset delivers only to those + chats, a chat outside the scope yields `403`, and an explicitly-empty + value yields `400`. schema: type: string - name: bot in: query - description: Bot name (required in multi-bot mode unless chat has a default bot binding) + description: Bot name (required in multi-bot mode unless chat has a default bot binding). Ignored for sender-token requests. schema: type: string requestBody: @@ -737,7 +747,7 @@ paths: - $ref: "#/components/schemas/MultiSendResponse" - $ref: "#/components/schemas/GitlabIgnoredResponse" "400": - description: Invalid request (malformed JSON or template error) + description: Invalid request (malformed JSON, template error, or an explicitly-empty `?chat_id=`) content: application/json: schema: @@ -748,6 +758,14 @@ paths: application/json: schema: $ref: "#/components/schemas/ErrorResponse" + "403": + description: | + Sender token requested a `?chat_id=` outside its allowed chats + (`server.gitlab.senders[].chats`); nothing is sent. + content: + application/json: + schema: + $ref: "#/components/schemas/ErrorResponse" "502": description: | Delivery to every target chat failed. Body is a `MultiSendResponse` diff --git a/internal/server/handler_gitlab.go b/internal/server/handler_gitlab.go index a5a829b..120907f 100644 --- a/internal/server/handler_gitlab.go +++ b/internal/server/handler_gitlab.go @@ -306,11 +306,31 @@ func (s *Server) handleGitlab(w http.ResponseWriter, r *http.Request) { status = "error" } - // A sender-token request is delivered only to that sender's chat scope - // (team isolation): ?chat_id, ?bot, Routes and DefaultChatID do not apply. + // A sender-token request stays inside that sender's chat scope (team + // isolation): ?bot, Routes and DefaultChatID never apply. ?chat_id acts as a + // filter *within* the allowed scope — omitted delivers to every sender chat + // (previous behaviour), a non-empty subset delivers only to those chats, and + // any chat outside the scope is refused (403) rather than silently ignored. // The filter/template/status logic above is shared with the default path. if isSender { - s.gitlabDeliver(w, r, "", senderChats, message, status, view.EventKey) + queryChats := parseChatIDs(r.URL.Query().Get("chat_id")) + // An explicitly-present but empty chat_id (?chat_id= / ?chat_id=,,) is a + // request error, not a fall-back to the full sender scope. + if len(queryChats) == 0 && r.URL.Query().Has("chat_id") { + writeError(w, http.StatusBadRequest, "chat_id is empty: provide at least one chat, or omit chat_id to deliver to all allowed chats") + return + } + targets := senderChats + if len(queryChats) > 0 { + canonical, bad, ok := s.senderScopeTargets(queryChats, senderChats) + if !ok { + vlog.V1("gitlab: sender requested out-of-scope chat %q -> 403", bad) + writeError(w, http.StatusForbidden, fmt.Sprintf("chat %q is outside this token's allowed chats", bad)) + return + } + targets = canonical + } + s.gitlabDeliver(w, r, "", targets, message, status, view.EventKey) return } @@ -395,6 +415,57 @@ func (s *Server) resolveGitlabAuth(token string) (chats []string, isSender, ok b return nil, false, false } +// senderScopeTargets checks that every requested chat_id stays within a sender's +// allowed chat scope, treating aliases and the UUIDs they resolve to as +// equivalent, and maps each in-scope request back to the sender.chats entry the +// operator configured. Delivering via that configured entry (rather than the raw +// request string) preserves the entry's bot binding in multi-bot mode, so a +// request that names a chat by the UUID its alias resolves to still delivers to +// the right bot instead of failing with "bot is required". +// +// It builds a lookup from every recognised key — each raw sender.chats value and +// the UUID it resolves to — back to the configured entry (raw values that fail to +// resolve are kept as-is). For each requested value it accepts the value itself, +// or the UUID it resolves to, when either is a known key, and emits the matching +// configured entry. Requests that collapse to the same entry are de-duplicated, +// preserving first-request order. +// +// It returns the canonical targets with ok=true, or on the first out-of-scope +// request (a value that fails to resolve and is not literally configured) returns +// (nil, that value, false). This gives alias↔UUID equivalence regardless of +// whether sender.Chats stores aliases or UUIDs. +func (s *Server) senderScopeTargets(requested, allowed []string) (targets []string, bad string, ok bool) { + entryOf := make(map[string]string, len(allowed)*2) + for _, a := range allowed { + if _, seen := entryOf[a]; !seen { + entryOf[a] = a + } + if res, err := s.chats(a); err == nil && res.ChatID != "" { + if _, seen := entryOf[res.ChatID]; !seen { + entryOf[res.ChatID] = a + } + } + } + seen := make(map[string]struct{}, len(requested)) + for _, q := range requested { + entry, in := entryOf[q] + if !in { + if res, err := s.chats(q); err == nil && res.ChatID != "" { + entry, in = entryOf[res.ChatID] + } + } + if !in { + return nil, q, false + } + if _, dup := seen[entry]; dup { + continue + } + seen[entry] = struct{}{} + targets = append(targets, entry) + } + return targets, "", true +} + // singleGitlabChat returns the single fallback delivery chat, following the // precedence default_chat_id -> global default chat -> the sole configured chat // alias. It is empty when none is configured. diff --git a/internal/server/handler_gitlab_test.go b/internal/server/handler_gitlab_test.go index 7eeec51..5a42065 100644 --- a/internal/server/handler_gitlab_test.go +++ b/internal/server/handler_gitlab_test.go @@ -12,6 +12,117 @@ import ( "github.com/lavr/express-botx/internal/config" ) +// scopeTestServer builds a bare Server whose chat resolver maps a fixed set of +// aliases to UUIDs. "bad-alias" returns a resolve error; other unknown values +// pass through as themselves, mirroring how a UUID resolves to itself. It is used +// to unit-test senderScopeTargets without going through the HTTP handler. +func scopeTestServer(t *testing.T) *Server { + t.Helper() + aliases := map[string]string{ + "team-a-alerts": "uuid-a", + "team-a-dev": "uuid-b", + "team-b-alerts": "uuid-c", + } + chatFn := func(chatID string) (ChatResolveResult, error) { + if chatID == "bad-alias" { + return ChatResolveResult{}, fmt.Errorf("unknown chat alias %q", chatID) + } + if u, ok := aliases[chatID]; ok { + return ChatResolveResult{ChatID: u}, nil + } + return ChatResolveResult{ChatID: chatID}, nil + } + return New(Config{Listen: ":0", BasePath: "/api/v1"}, okSend, chatFn) +} + +// TestSenderScopeTargets covers alias/UUID membership equivalence, out-of-scope +// detection, and canonicalisation to the configured sender.chats entry for the +// sender-scoped ?chat_id filter. +func TestSenderScopeTargets(t *testing.T) { + s := scopeTestServer(t) + tests := []struct { + name string + requested []string + allowed []string + wantTargets []string + wantBad string + wantOK bool + }{ + { + name: "alias matches same alias", + requested: []string{"team-a-alerts"}, + allowed: []string{"team-a-alerts"}, + wantTargets: []string{"team-a-alerts"}, + wantOK: true, + }, + { + name: "alias request against UUID in scope canonicalises to UUID", + requested: []string{"team-a-alerts"}, + allowed: []string{"uuid-a"}, + wantTargets: []string{"uuid-a"}, + wantOK: true, + }, + { + name: "UUID request against alias in scope canonicalises to alias", + requested: []string{"uuid-a"}, + allowed: []string{"team-a-alerts"}, + wantTargets: []string{"team-a-alerts"}, + wantOK: true, + }, + { + name: "foreign alias is a violation", + requested: []string{"team-b-alerts"}, + allowed: []string{"team-a-alerts"}, + wantBad: "team-b-alerts", + wantOK: false, + }, + { + name: "foreign UUID is a violation", + requested: []string{"uuid-c"}, + allowed: []string{"team-a-alerts"}, + wantBad: "uuid-c", + wantOK: false, + }, + { + name: "unresolvable requested chat is a violation", + requested: []string{"bad-alias"}, + allowed: []string{"team-a-alerts"}, + wantBad: "bad-alias", + wantOK: false, + }, + { + name: "subset of several is allowed", + requested: []string{"team-a-dev"}, + allowed: []string{"team-a-alerts", "team-a-dev"}, + wantTargets: []string{"team-a-dev"}, + wantOK: true, + }, + { + name: "alias and its own UUID collapse to one target", + requested: []string{"team-a-alerts", "uuid-a"}, + allowed: []string{"team-a-alerts"}, + wantTargets: []string{"team-a-alerts"}, + wantOK: true, + }, + { + name: "one out-of-scope among valid returns that one", + requested: []string{"team-a-alerts", "team-b-alerts"}, + allowed: []string{"team-a-alerts", "team-a-dev"}, + wantBad: "team-b-alerts", + wantOK: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + targets, bad, ok := s.senderScopeTargets(tt.requested, tt.allowed) + if ok != tt.wantOK || bad != tt.wantBad || !slices.Equal(targets, tt.wantTargets) { + t.Errorf("senderScopeTargets(%v, %v) = (%v, %q, %v), want (%v, %q, %v)", + tt.requested, tt.allowed, targets, bad, ok, tt.wantTargets, tt.wantBad, tt.wantOK) + } + }) + } +} + // newGitlabTestServer builds a server with the gitlab endpoint enabled and a // send function that records the last SendPayload it received. func newGitlabTestServer(t *testing.T, cfg *GitlabConfig) (*Server, *captureSend) { @@ -1317,21 +1428,181 @@ func TestGitlab_SenderFanout(t *testing.T) { } } -// TestGitlab_SenderIgnoresQueryChat: ?chat_id must not let a sender token break -// out of its chat scope — delivery still goes only to the sender's chats. -func TestGitlab_SenderIgnoresQueryChat(t *testing.T) { +// TestGitlab_SenderChatFilterSubset: ?chat_id listing a subset of the sender's +// own chats delivers to exactly that subset, not the full scope. +func TestGitlab_SenderChatFilterSubset(t *testing.T) { + srv, cap := newGitlabFanoutServer(t, senderTestConfig(), okSend, nil) + w := doRequest(srv, "POST", "/api/v1/gitlab?chat_id=team-a-chat2", strings.NewReader(mrOpenPayload), gitlabHeaders("team-a-token")) + if w.Code != 200 { + t.Fatalf("status = %d, body: %s", w.Code, w.Body.String()) + } + if cap.count() != 1 { + t.Fatalf("send count = %d, want 1 (filtered to the requested chat)", cap.count()) + } + if got := cap.last().ChatID; got != "team-a-chat2" { + t.Errorf("chat = %q, want team-a-chat2", got) + } + var resp MultiSendResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) + } + if !resp.OK || len(resp.Results) != 1 || resp.Results[0].Chat != "team-a-chat2" { + t.Errorf("response = %+v, want single team-a-chat2 result", resp) + } +} + +// TestGitlab_SenderChatFilterOutOfScope: ?chat_id naming a chat the sender is not +// allowed to reach is refused with 403 and delivers nothing (scope breakout). +func TestGitlab_SenderChatFilterOutOfScope(t *testing.T) { srv, cap := newGitlabFanoutServer(t, senderTestConfig(), okSend, nil) w := doRequest(srv, "POST", "/api/v1/gitlab?chat_id=chat1", strings.NewReader(mrOpenPayload), gitlabHeaders("team-a-token")) + if w.Code != 403 { + t.Fatalf("status = %d, want 403 (out-of-scope chat); body: %s", w.Code, w.Body.String()) + } + if cap.count() != 0 { + t.Fatalf("send count = %d, want 0 (nothing sent on scope violation)", cap.count()) + } + var resp map[string]any + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) + } + if ok, _ := resp["ok"].(bool); ok || !strings.Contains(fmt.Sprint(resp["error"]), "chat1") { + t.Errorf("response = %+v, want ok:false with an error naming chat1", resp) + } +} + +// TestGitlab_SenderChatFilterPartialOutOfScope: one allowed + one foreign chat in +// ?chat_id is refused wholesale (403), delivering to neither. +func TestGitlab_SenderChatFilterPartialOutOfScope(t *testing.T) { + srv, cap := newGitlabFanoutServer(t, senderTestConfig(), okSend, nil) + w := doRequest(srv, "POST", "/api/v1/gitlab?chat_id=team-a-chat1,chat1", strings.NewReader(mrOpenPayload), gitlabHeaders("team-a-token")) + if w.Code != 403 { + t.Fatalf("status = %d, want 403 (partial out-of-scope); body: %s", w.Code, w.Body.String()) + } + if cap.count() != 0 { + t.Fatalf("send count = %d, want 0 (all-or-nothing on scope violation)", cap.count()) + } +} + +// TestGitlab_SenderChatFilterEmpty: an explicitly-present but empty ?chat_id is a +// 400 request error, not a silent fall-back to the full sender scope. +func TestGitlab_SenderChatFilterEmpty(t *testing.T) { + for _, q := range []string{"?chat_id=", "?chat_id=,,", "?chat_id=+,+"} { + t.Run(q, func(t *testing.T) { + srv, cap := newGitlabFanoutServer(t, senderTestConfig(), okSend, nil) + w := doRequest(srv, "POST", "/api/v1/gitlab"+q, strings.NewReader(mrOpenPayload), gitlabHeaders("team-a-token")) + if w.Code != 400 { + t.Fatalf("status = %d, want 400 (empty chat_id); body: %s", w.Code, w.Body.String()) + } + if cap.count() != 0 { + t.Fatalf("send count = %d, want 0 (nothing sent on empty chat_id)", cap.count()) + } + }) + } +} + +// TestGitlab_SenderChatFilterAliasUUID: alias↔UUID equivalence — a sender scoped +// by alias accepts the UUID it resolves to (and vice versa) in ?chat_id. +func TestGitlab_SenderChatFilterAliasUUID(t *testing.T) { + aliases := map[string]string{"team-a-alerts": "uuid-a", "team-a-dev": "uuid-b"} + chatFn := func(chatID string) (ChatResolveResult, error) { + if u, ok := aliases[chatID]; ok { + return ChatResolveResult{ChatID: u}, nil + } + return ChatResolveResult{ChatID: chatID}, nil + } + + t.Run("UUID request against alias scope", func(t *testing.T) { + cfg := &GitlabConfig{ + SecretToken: "secret", + Senders: []GitlabSender{{Secret: "team-a-token", Chats: []string{"team-a-alerts", "team-a-dev"}}}, + } + srv, cap := newGitlabFanoutServer(t, cfg, okSend, chatFn) + w := doRequest(srv, "POST", "/api/v1/gitlab?chat_id=uuid-a", strings.NewReader(mrOpenPayload), gitlabHeaders("team-a-token")) + if w.Code != 200 { + t.Fatalf("status = %d, want 200 (UUID matches alias scope); body: %s", w.Code, w.Body.String()) + } + if cap.count() != 1 || cap.last().ChatID != "uuid-a" { + t.Fatalf("sends = %d, last = %q, want 1 to uuid-a", cap.count(), cap.last().ChatID) + } + }) + + t.Run("alias request against UUID scope", func(t *testing.T) { + cfg := &GitlabConfig{ + SecretToken: "secret", + Senders: []GitlabSender{{Secret: "team-a-token", Chats: []string{"uuid-a", "uuid-b"}}}, + } + srv, cap := newGitlabFanoutServer(t, cfg, okSend, chatFn) + w := doRequest(srv, "POST", "/api/v1/gitlab?chat_id=team-a-alerts", strings.NewReader(mrOpenPayload), gitlabHeaders("team-a-token")) + if w.Code != 200 { + t.Fatalf("status = %d, want 200 (alias matches UUID scope); body: %s", w.Code, w.Body.String()) + } + // gitlabDeliver resolves each target alias to its UUID before sending, so + // the recorded chat is the resolved uuid-a even though ?chat_id used the + // alias. + if cap.count() != 1 || cap.last().ChatID != "uuid-a" { + t.Fatalf("sends = %d, last = %q, want 1 to uuid-a", cap.count(), cap.last().ChatID) + } + }) +} + +// TestGitlab_SenderChatFilterMultiSubset: ?chat_id naming several in-scope chats +// fans out to exactly those chats, in request order, leaving the rest untouched. +func TestGitlab_SenderChatFilterMultiSubset(t *testing.T) { + cfg := &GitlabConfig{ + SecretToken: "secret", + Senders: []GitlabSender{{Secret: "team-a-token", Chats: []string{"team-a-c1", "team-a-c2", "team-a-c3"}}}, + } + srv, cap := newGitlabFanoutServer(t, cfg, okSend, nil) + w := doRequest(srv, "POST", "/api/v1/gitlab?chat_id=team-a-c1,team-a-c3", strings.NewReader(mrOpenPayload), gitlabHeaders("team-a-token")) if w.Code != 200 { t.Fatalf("status = %d, body: %s", w.Code, w.Body.String()) } if cap.count() != 2 { - t.Fatalf("send count = %d, want 2 (sender chats only)", cap.count()) + t.Fatalf("send count = %d, want 2 (only the requested subset)", cap.count()) } - for _, p := range cap.calls { - if p.ChatID == "chat1" { - t.Errorf("delivered to %q via ?chat_id override; sender scope must ignore it", p.ChatID) + var resp MultiSendResponse + if err := json.Unmarshal(w.Body.Bytes(), &resp); err != nil { + t.Fatalf("decode: %v (body: %s)", err, w.Body.String()) + } + got := []string{resp.Results[0].Chat, resp.Results[1].Chat} + if !slices.Equal(got, []string{"team-a-c1", "team-a-c3"}) { + t.Errorf("results = %v, want [team-a-c1 team-a-c3] in request order", got) + } +} + +// TestGitlab_SenderChatFilterUUIDMultiBot: in multi-bot mode a sender scoped by a +// bot-bound alias may name that chat by the UUID it resolves to; the request is +// canonicalised back to the configured alias so its bot binding is preserved and +// delivery succeeds (rather than failing with "bot is required"). +func TestGitlab_SenderChatFilterUUIDMultiBot(t *testing.T) { + cfg := &GitlabConfig{ + Senders: []GitlabSender{{Secret: "team-a-token", Chats: []string{"team-a-alerts"}}}, + } + tmpls, err := ParseGitlabTemplates(nil) + if err != nil { + t.Fatalf("parse default templates: %v", err) + } + cfg.Templates = tmpls + cap := &captureSend{} + send := func(ctx context.Context, p *SendPayload) (string, error) { + cap.record(p) + return "sync-1", nil + } + chatFn := func(chatID string) (ChatResolveResult, error) { + if chatID == "team-a-alerts" { + return ChatResolveResult{ChatID: "uuid-a", Bot: "bot-a"}, nil } + return ChatResolveResult{ChatID: chatID}, nil + } + srv := New(Config{Listen: ":0", BasePath: "/api/v1", BotNames: []string{"bot-a", "bot-b"}}, send, chatFn, WithGitlab(cfg)) + + w := doRequest(srv, "POST", "/api/v1/gitlab?chat_id=uuid-a", strings.NewReader(mrOpenPayload), gitlabHeaders("team-a-token")) + if w.Code != 200 { + t.Fatalf("status = %d, want 200 (UUID canonicalises to bound alias); body: %s", w.Code, w.Body.String()) + } + if cap.count() != 1 || cap.last().ChatID != "uuid-a" || cap.last().Bot != "bot-a" { + t.Fatalf("send = %+v, want single delivery to uuid-a via bot-a", cap.last()) } } diff --git a/internal/server/handler_send.go b/internal/server/handler_send.go index cab2c1a..80a9116 100644 --- a/internal/server/handler_send.go +++ b/internal/server/handler_send.go @@ -127,7 +127,7 @@ func (s *Server) handleSend(w http.ResponseWriter, r *http.Request) { // alternative would give per-command retry: on a partial failure the retry // re-sends to the chats that already succeeded (duplicates) or the failed // chat is lost with the ack. The worker stays "one chat = one message" and - // is not touched. See docs/plans/20260708-multi-chat-fanout.md. + // is not touched. targets := parseChatIDs(payload.ChatID) if len(targets) == 0 { writeError(w, http.StatusBadRequest, "chat_id is required") diff --git a/internal/server/multisend.go b/internal/server/multisend.go index 74cecc6..9c02c3c 100644 --- a/internal/server/multisend.go +++ b/internal/server/multisend.go @@ -4,8 +4,7 @@ package server // send surface (/send, /alertmanager, /grafana, /gitlab and the CLI). The // contract is deliberately uniform: a chat_id may list several chats separated by // commas (chat_id=a,b,c), the message is delivered best-effort to each, and the -// response is always a MultiSendResponse — even for a single chat. See -// docs/plans/20260708-multi-chat-fanout.md for the rationale. +// response is always a MultiSendResponse — even for a single chat. import ( "context" diff --git a/internal/server/server_test.go b/internal/server/server_test.go index a80f5c5..a51a821 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -2221,6 +2221,46 @@ func TestSend_AsyncMode_MissingBotID(t *testing.T) { } } +// TestSend_AsyncMode_MultiChatValidationAllOrNothing: async routing validation is +// request-level — if any target in a multi-chat request fails validation the whole +// request is rejected with 400 and nothing is enqueued (no partial publish). +func TestSend_AsyncMode_MultiChatValidationAllOrNothing(t *testing.T) { + var enqueued int + cfg := Config{ + Listen: ":0", + BasePath: "/api/v1", + Keys: []ResolvedKey{{Name: "t", Key: "k"}}, + AsyncMode: true, + DefaultRoutingMode: "direct", + } + sendFn := func(ctx context.Context, p *SendPayload) (string, error) { + enqueued++ + return "req-id", nil + } + chatResolver := func(chatID string) (ChatResolveResult, error) { + return ChatResolveResult{ChatID: chatID}, nil + } + srv := New(cfg, sendFn, chatResolver) + + // First chat is a valid UUID; second is not — direct mode rejects the second. + body := `{"bot_id":"00000000-0000-0000-0000-000000000001","chat_id":"00000000-0000-0000-0000-000000000002,not-a-uuid","message":"hi","routing_mode":"direct"}` + w := doRequest(srv, "POST", "/api/v1/send", strings.NewReader(body), map[string]string{ + "X-API-Key": "k", + "Content-Type": "application/json", + }) + + if w.Code != 400 { + t.Fatalf("status = %d, want 400 (whole request rejected); body: %s", w.Code, w.Body.String()) + } + resp := parseResponse(t, w) + if resp.OK || !strings.Contains(resp.Error, "chat_id must be a valid UUID") { + t.Errorf("response = %+v, want ok:false naming the invalid chat", resp) + } + if enqueued != 0 { + t.Errorf("enqueued = %d, want 0 (nothing published on validation failure)", enqueued) + } +} + func TestSend_AsyncMode_Multipart(t *testing.T) { var capturedPayload *SendPayload cfg := Config{