0 Fork 0

Files

T

Taiga 5d3ea0a4ee [2026-09-28 06:10:57] syncthing-out: personal/plans/balda-callee-prism-plan.md personal/plans/balda-mm-pr8-review-fixes.md

2026-09-28 06:10:57 +00:00

17 KiB

Executable File

Raw Blame History

Balda — PR #8 review fixes: slash commands, /locator, group DM session

Создан 2026-09-28. Статус: ПЛАН — ожидает подтверждения Alex. Источник: ревью Alex на baldaworks/balda#8 (Mattermost transport). Часть personal/plans/balda-mm-pr-plan (PR #8). Ветка feat/mattermost-channel. Ветки / кода ещё не трогал — только чтение и сверка.

0. TL;DR

Ревью право по всем трём пунктам — проверено в коде, не на слово.

#	Дефект	Подтверждение в коде (коммит `9f1e6c9c`)
Б1	Slash-команды не доходят: `/start` и прочие обрабатываются только из WS-события `posted`	`channel/mattermost/ingress.go` — `switch event.Event` знает только `posted`; отдельного HTTP-приёмника slash нет вовсе. У Slack он есть: `slackagent/server.go` → `mux.HandleFunc(commandsPath, h.handleCommands)`
Б2	`/locator` всегда падает: `locatorref.Parse` не знает `mattermost`	`locatorref/locatorref.go:52-62` — `switch channelType` покрывает только `telegram` / `zulip` / `slackagent`; ветки `mattermost` нет → `unsupported channel type`
В3	Групповой DM расщепляется по отправителям	`locator_codec.go:102-118` — `dmAddressKey` = `d:<ChannelID>` (один чат = один ключ), а `dmSessionID` при непустом `UserID` = `mm-dm-<hash(UserID)>` (автор, не чат)

🔴 Ключевой вопрос по Б1 — не «как», а «что именно». См. §2: две реализации, они архитектурно разные. Нужно решение Alex до написания кода.

1. Сверка с докой (что именно было заявлено и оказалось неправдой)

docs/commands.md:20-22 (коммит 58f27120) утверждает:

Mattermost uses one slash command per action, such as /locator, and accepts them in direct messages and in channels where the bot is mentioned.

И таблица (docs/commands.md:26+) ставит /help, /topic, /goalkeeper, /auto, /usage, /close, /cancel, /user → «yes» для Mattermost.

⚠️ Всё это ложь текущей реализации — ни одна slash-команда в MM не обрабатывается. /start в таблице стоит no, но это тоже неверно: в MM /start — единственный документированный путь первичной привязки владельца, и он не работает.

📌 Правка доков — обязательная часть фикса, а не «потом». Колонка Mattermost в таблице доступности должна стать следствием кода, который реально есть.

2. Б1 — slash-команды. Решение: вариант A (HTTP-приёмник). Вопрос снят.

Решение Alex 2026-09-28: «Тебе что, не очевидно в прод-качестве?» — выбора нет. B (убрать slash из доки) = откат функциональности, а не фикс. Прод-качество = A. Спрашивать было нечего. Ниже сохранён разбор обоих путей как обоснование.

Mattermost не публикует slash-команды как посты (в отличие от Telegram/Zulip) — поэтому текущий WS-путь их физически не видит. Есть два штатных механизма, и они взаимоисключающие по архитектуре:

📌 Факт, подтверждённый докой MM (docs.mattermost.com/integrations-guide/slash-commands): «if you type in a slash command it will execute without posting a message, whereas an outgoing webhook is only triggered by posted messages». То есть WS-события posted для slash не будет — ревью право на 100%.

🔴 Что уже работает и что отсутствует — точно:

Компонент	Статус
`mattermostInboundHandler.HandleCommand` (`handlersfx/mattermost_ingress.go`) — публикация в `commandcmd`	✅ написан и корректен
`ParseCommand` (`inbound.go:223`) — разбор `/name args` из текста	✅ написан
вызов `ParseCommand` из `processPosted` (`ingress.go`) — mention-путь	✅ работает
HTTP-путь для slash (MM POST → наш эндпоинт)	❌ отсутствует целиком

Значит фикс Б1 — не переписывать command-машину, а добавить к ней второй вход. Command-машина исправна; нет только способа донести до неё slash-запрос.

Вариант A — Slack-style: исходящий webhook + HTTP-приёмник (ВЫБРАН)

Slash-команда MM настраивается на Request URL — MM делает POST в наш HTTP-эндпоинт. Это ровно то, что уже сделано для Slack, значит есть рабочий образец в репо.

Что дублируется из slackagent/:

HTTP-сервер с отдельным путём /mattermost/commands (server.go:129-аналог);
handleCommands → decodeCommandRequestWithRegistry → commandcmd.Request → common handler;
ответ в MM — JSON {"response_type":"ephemeral"|"in_channel","text":"..."} (MM-специфично, отличается от Slack {"text":...});
верификация: у MM slash-команда подписывается полем token в теле запроса (не HMAC-подпись, как у Slack) — сравнение с токеном из конфига;
lifeзycle: тот же appports.TransportLifecycleStage, что у WS-ingress (либо расширить существующий Ingress, либо отдельный Server — см. §6).

Плюсы: /start, /topic, /goalkeeper, /auto, /usage, /close, /cancel, /user работают в полном объёме, как в доке и таблице. Минусы: нужен публичный HTTP-эндпоинт для MM и новый ключ конфига (slash-токен) + правки доков по настройке.

Вариант B — «mention-форма»: команды только текстом в посте (ОТКЛОНЁН)

Оставить один WS-путь, а из доки/таблицы убрать slash-обещания: писать команды как @balda /locator (текст поста, ParseCommand их уже разбирает — ingress.go/inbound.go).

Почему отклонён: это не фикс, а удаление уже обещанной функциональности. /start owner=<token> в MM-развёртывании — штатный вход владельца, и docs/mattermost.md:117-119 прямо описывает его как slash-команду в DM. Вариант B = переписать доку под урезанный код. Для «прод-качества» — неприемлемо.

Итог

A. В доке уже обещаны все slash-команды, у Slack есть готовый образец (slackagent/server.go:129 → mux.HandleFunc(commandsPath, h.handleCommands) + handleCommands → decodeCommandRequestWithRegistry → commandcmd.Request), а /start owner=<token> — штатный вход владельца в MM-развёртывании.

3. Б2 — `/locator`: добавить `mattermost` в `locatorref`

Механически, по образцу slackagentLocatorFromAddressKey (там та же форма c:/t:):

Что	Куда
ветка `case baldamattermost.ChannelType:` в `switch channelType`	`locatorref/locatorref.go:52-62`
`mattermostLocatorFromAddressKey(addressKey string)` — разбор `c:<channel>`, `c:<channel>:<root>`, `d:<channel>`	рядом с `slackAgentLocatorFromAddressKey`
конструкторы `NewMattermostChannelLocator` / `NewMattermostDMLocator` (по образцу `NewSlackAgent*`)	там же
импорт `channel/mattermost` — ⚠️ проверить архитектурное правило	см. ниже

🔴 Питфолл архитектуры. AGENTS.md прямо требует: «Keep locatorref independent from concrete transport adapter packages». Импорт channel/mattermost в locatorref это нарушает. Как это решено сейчас: locatorref импортирует telegramref (отдельный тонкий пакет-ref), а для zulip/slack — дублирует константы строкой:

channelTypeZulip       = "zulip"
channelTypeSlackAgent  = slackagentref.ChannelType   // ← тонкий ref-пакет

📌 Значит правильная форма для MM — тонкий пакет mattermostref (или константа-строка в locatorref), не импорт транспорта. Решение сверить с slackagentref/telegramref, прежде чем писать код. Это тот случай, где go-arch-lint даст красное на наивной реализации.

Ещё два следствия Б2, которые надо закрыть тем же изменением:

internalmcp/manager.go:409 — locatorref.Parse(channelType + ":" + addressKey). Любой MM-таргет там сейчас падает.
envelopetarget/resolve.go:62 — locatorref.Parse(target.Key). То же самое → balda.scheduler.jobs с target: locator для MM не работает.

⚠️ Это и есть вторая половина фразы ревью «также исключает Mattermost из config targets, использующих тот же парсер». Правка только handler.go задачу не закрывает.

4. В3 — групповой DM: ключ чата вместо ключа автора

Сейчас:

func dmAddressKey(a LocatorAddress) string { return fmt.Sprintf("d:%s", url.PathEscape(a.ChannelID)) }  // ЧАТ
func dmSessionID(a LocatorAddress) string {
    if a.UserID != "" { return fmt.Sprintf("%s-dm-%s", prefix, shortHash(a.UserID)) }   // АВТОР ← баг
    return fmt.Sprintf("%s-dm-%s", prefix, shortHash(a.ChannelID))
}

DM (D) приходит с UserID → сессия по автору. Групповой DM (G) канал общий, а авторов много → у каждого участника своя сессия. Ревью право.

Правильная форма — различать D и G явно, а не «если есть UserID»:

Тип канала MM	AddressKey	SessionID
`D` (1:1)	`d:<channel_id>`	`mm-dm-<hash(channel_id)>` (чат = 2 участника, стабильно)
`G` (групповой)	`d:<channel_id>`	`mm-gdm-<hash(channel_id)>` — по чату

⚠️ Соображения, которые надо учесть при реализации:

Обратная совместимость: существующие 1:1 DM-сессии на Kraken имеют id mm-dm-<hash(UserID)> (перепривязка сломает их → новые сессии, старые «осиротеют»). Либо принять (тестовая инсталляция), либо оставить для D форму по UserID.
IsDirectChannelType (inbound.go) уже знает D/G — переиспользовать.
Точка входа — LocatorForPost (inbound.go), она и должна различать D/G.
Правка ведёт к обновлению locator_codec_test.go (тест на изоляцию thread↔channel придётся дополнить тестом «два автора в G → одна сессия»).

5. Порядок работ (порядок важен: сначала решается A/B)

#	Шаг	Файл	Тип
0	Решение Alex: вариант A или B для Б1	—	⛔ блокирует всё остальное
1	Б2: `mattermost` в `locatorref` (через ref-пакет, не импорт транспорта)	`locatorref/`, новый `mattermostref`?	код
2	Б2-хвост: тесты на `internalmcp/manager.go` и `envelopetarget/resolve.go` путях	тесты	код
3	В3: `D`/`G` различать в `dmSessionID` + `LocatorForPost`	`locator_codec.go`, `inbound.go`	код
4	В3: тест «два автора в групповом DM → одна сессия»	`locator_codec_test.go`	тест
5	Б1 (если A): HTTP-приёмник slash + `token`-верификация + DI + конфиг-ключ	`channel/mattermost/`, `mattermostfx/`, `app.go`, `balda.yaml`	код
6	Б1 (если A): `docs/mattermost.md` — настройка slash Request URL и токена	доки	доки
7	Правка таблицы команд `docs/commands.md` — колонка Mattermost = реальность, не обещание	доки	доки
8	Гейты: `go build` / `vet` / `golangci-lint` / `go-arch-lint` / `go test -race`	—	проверка
9	Push в `feat/mattermost-channel` → дождаться CI 8/8	—	проверка
10	Обновить Obsidian: personal/plans/balda-mm-pr-plan, personal/projects/balda/balda-mattermost-connector	доки	доки

📌 Коммиты — Conventional Commits, правки — дельта-апплаером (tools/run-project-deltas.sh, check → apply), не sed/in-place (правило Alex).

📌 Ветка та же (feat/mattermost-channel) — правки уедут в открытый PR #8 и сбросят пройденный CI. Это ожидаемо: PR не смержен, ревью просит исправлений.

6. Риски / питфоллы, которые уже видны

🔴 locatorref не должен импортировать channel/mattermost — go-arch-lint даст composition_root/boundary-ошибку. Форма фикса — тонкий ref-пакет, как slackagentref.
🔴 Slash-команды MM требуют публичного HTTPS для Request URL. Если стенд на Kraken без публичного адреса — нужен туннель либо вариант B. Проверить до выбора A.
⚠️ MM slash-запросы возвращаются в течение 3 с, иначе MM показывает timeout. Тяжёлые команды (/goalkeeper) обязаны отвечать сразу, а результат доставлять постом — как у Slack.
⚠️ token в slash-запросе — не HMAC. Другая модель верификации, чем у Slack; копировать signedSlackRequest-логику нельзя.
⚠️ Обратная совместимость DM-сессий (§4) — смена id осиротит существующие сессии.
⚠️ Изменение Б3 меняет поведение transport_user_id в persisted-метаданных сессии — перепроверить тест TestLocatorForPostScopesReplyToThreadRoot и «чужой user_id» из §«Два не-блокера» personal/plans/balda-mm-pr-plan.

Связанные заметки

personal/plans/balda-mm-pr-plan — PR #8, состояние, деплой, приёмка
personal/projects/balda/balda-mattermost-connector — Фаза 2 (код коннектора)
personal/tech/balda-go-fx-pitfalls — fx-питфоллы, дельта-апплаер, arch-lint
personal/projects/balda/balda-mm-delegation-plan — общий план

17 KiB Executable File Raw Blame History Unescape Escape