0 Fork 0

Files

T

Alexey Martemyanov d6c3ef4ffa [2026-09-28] eagle: personal/plans/balda-mm-pr8-review-fixes.md

2026-09-28 13:41:56 +06:00

14 KiB

Executable File

Raw Blame History

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

Создан 2026-09-28. Статус: ✅ РЕАЛИЗОВАНО, ЗАПУШЕНО, CI ИДЁТ. Коммит cd4ce4bc в feat/mattermost-channel → PR baldaworks/balda#8 обновлён. 46 файлов, +6760/−27, база 0 позади / 8 впереди. Источник: ревью Alex на PR #8 + четвёртый дефект, найденный мной (§5). Часть personal/plans/balda-mm-pr-plan (PR #8).

0. Итог

Ревью Alex подтверждено в коде — все три пункта. Плюс найден четвёртый дефект, которого в ревью не было (§5). Все четыре закрыты одним коммитом.

#	Дефект	Статус
Б1	Slash-команды не доходят (только WS `posted`)	✅ HTTP-приёмник `command_server.go`
Б2	`/locator` падает: `locatorref` не знает `mattermost`	✅ ветка + декодер + тесты
В3	Групповой DM расщепляется по авторам	✅ свой тип/префикс/сессия + scope
Б4	Регистрация в `ScopeResolver` отсутствовала В PR	✅ `app.go` + `preflight.go`

1. Гейты — все зелёные (локально, на закоммиченном дереве)

gofmt (мои файлы)                            → чисто
go build ./...                               → exit 0
go vet ./...                                 → exit 0
go tool golangci-lint run                    → 0 issues
go tool go-arch-lint check --project-path .  → OK - No warnings found (без runtime-local)
go test -race (7 затронутых пакетов)         → все ok
go test ./cmd/balda/ (fx-граф)               → ok

Новые тесты: 19 (command_server_test.go 15 + locatorref/mattermost_test.go 4).

⚠️ go-arch-lint: 28 замечаний «not attached» — все из runtime-local/ (наша локальная папка с форком runtime, в PR её нет). Проверено mv runtime-local → OK - No warnings found. Это не наши правки.

⚠️ gofmt: в репо 80 файлов не отформатированы — унаследовано, проверено на HEAD (git show HEAD:<file> | gofmt -l /dev/stdin). Мои файлы чисты. Свои gofmt-правки в adapter.go/ingress_test.go откатил, чтобы не раздувать diff PR-ом чужих файлов.

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

📌 Факт из доки MM: «if you type in a slash command it will execute without posting a message» — WS-события posted для slash не будет. Ревью право на 100%.

Что уже было и что добавлено:

Компонент	Статус
`mattermostInboundHandler.HandleCommand` — публикация в `commandcmd`	✅ уже был, корректен
`ParseCommand` — разбор `/name args`	✅ уже был
вызов из `processPosted` (mention-путь: `@balda /locator`)	✅ работает
HTTP-путь для slash	❌ добавлен `channel/mattermost/command_server.go`

🔴 Правка Alex (важный урок): «в твоей ММ реализации взят существующий slack транспорт за основу? Они должны быть практически идентичны». Первая версия разошлась со Slack в пяти местах без причины — переписал по образцу slackagent/server.go:

Что	Slack	Было у меня	Стало
`invocationID`	хеш от тела	из `post_id`	`commandInvocationID(body)`
таймаут команды	`2500ms`	`30s`	`2500ms`
семафор	`processSem` 16	отсутствовал	16
`webhookMaxBodyBytes`	`1 << 20`	`64 << 10`	`1 << 20`
`ReadTimeout`/`Idle`	10s / 30s	15s / 60s	10s / 30s

Осталось ровно два отличия, и оба диктует провайдер (не наш выбор):

Верификация: у MM token в теле запроса (сравнение constant-time), у Slack — HMAC-подпись X-Slack-Signature. Копировать verifySignature нельзя.
Форма ответа: MM {"response_type":"ephemeral","text":"..."}, Slack {"text":...}.

Поведение:

принимает обе конфигурации команды: /locator (аргументы в text) и /balda locator (первое слово text = подкоманда) — как у Slack с /balda;
неизвестная команда → 200 + usage (не 400, не тишина) — тоже как у Slack;
тип канала (D/G/O/P) резолвится через GetChannel, т.к. slash его не несёт; при ошибке резолва — трактуем как не-DM (строгая авторизация);
отвечает сразу, результат уходит постом (MM показывает timeout автору за ~3 с).

Новые ключи конфига (balda.mattermost.*): commands_enabled, commands_listen_addr (:8093), commands_path (/mattermost/commands), commands_token. Все 4 + провайдеры name:-тегов в app.go.

3. Б2 — `/locator`: `mattermost` в `locatorref`

Механически, по образцу slackagentLocatorFromAddressKey.

🔴 Питфолл архитектуры ЗАКРЫТ правильно: AGENTS.md требует независимости locatorref от транспортов. Не импортировал channel/mattermost — использовал string(deliverycmd.ChannelTypeMattermost), ровно как для Slack (channelTypeSlackAgent = string(deliverycmd.ChannelTypeSlackAgent)). deliverycmd уже в allow-list locatorref. go-arch-lint зелёный.

🔴 Ошибка, которую поймал тестами: первая версия декодера использовала типы "channel"/"direct", а транспорт определяет "channel"/"dm"/"group". Несовпадение строк = локатор декодируется, но транспорт его отвергает. Сверить имена типов с locator_codec.go, не выдумывать.

Хвост Б2 (закрыт тем же изменением):

internalmcp/manager.go:409 — locatorref.Parse теперь работает для MM;
envelopetarget/resolve.go:62 — balda.scheduler.jobs target: locator для MM теперь работает. Это вторая половина фразы ревью «исключает Mattermost из config targets».

Тесты: locatorref/mattermost_test.go — 4 теста (канал/тред, DM/группа, Format-round-trip, отказ на кривом ключе).

4. В3 — групповой DM

Было (дефект):

dmSessionID: if a.UserID != "" { return "mm-dm-<hash(UserID)>" }   // АВТОР, не чат

G-канал общий, авторов много → у каждого своя сессия.

Стало:

Тип MM	AddressKey	SessionID	Scope
`D` (1:1)	`d:<channel_id>`	`mm-dm-<hash(channel_id)>`	personal
`G` (групповой)	`g:<channel_id>`	`mm-gdm-<hash(channel_id)>`	personal

🔴 Отдельный префикс ключа g: — не косметика. Общий d: не различим при LocatorFromAddressKey: round-trip d: восстанавливал любой групповой DM как 1:1. Свой префикс сохраняет тип.

Закрыто в 8 местах (§«Тип канала» — самый коварный): addressTypeGroupDM · NewGroupDMLocator · dmAddressKey · dmSessionID · IsDirectLocator · validateLocatorAddress · ClassifyLocatorScope · LocatorForPost + IsGroupChannelType.

⚠️ validateLocatorAddress и ClassifyLocatorScope — я их пропустил в первой версии. DecodeLocator возвращает ok=false для неизвестного типа, и тогда все下游-хелперы fail-closed: IsDirectLocator, ChannelIDFromLocator, ClassifyLocatorScope. Тест TestLocatorForPostUsesDirectLocatorForDMAndGroupChannels/G это и поймал.

⚠️ Обратная совместимость: старая форма D-сессии была mm-dm-<hash(UserID)>, стала mm-dm-<hash(channel_id)> → существующие 1:1 DM-сессии на Kraken осиротеют (новые создадутся, старые останутся мёртвыми). Тестовая инсталляция — принято.

5. 🔴 Б4 — четвёртый дефект, которого НЕ было в ревью

Регистрация MM в ScopeResolver отсутствовала в PR #8.

Диагностика: grep ClassifyLocatorScope internal/apps/balda/app.go — строка есть в рабочем дереве, но git show HEAD:...app.go | grep ClassifyLocatorScope — строка отсутствует. Значит она была WIP-правкой на диске и в коммиты не уехала. В preflight.go её не было вообще (обе ветки).

Почему это критично (питфолл #11 навыка): sessionmemoryapp.ScopeResolver строит карту классификаторов по channelType; резолвер fail-closed (capture.go: classifier == nil → CodeUnsupportedScope). Симптом при session_memory.enabled: true:

warn … "no session-memory classifier is registered for \"mattermost\""
→ в чате: "Could not reset this session."

MCP-инструменты не подключаются (function_call_part_count: 0). Компилируется, go build/vet/линтеры зелёные, отказ только в рантайме.

Зарегистрировано в обоих файлах (app.go и preflight.go) — карт две, правка одной даёт расхождение «preflight прошёл, рантайм упал».

📌 Урок метода: «зелёные гейты» ≠ «рабочий транспорт». Эта строка проходила все статическиe проверки, отсутствуя в PR. Сверять git show HEAD:<file> с рабочим деревом для критичных регистраций, а не доверять git status.

6. Что понадобилось дополнительно (для будущих транспортов)

🔴 Каждый name:-тег в mattermostfx = обязательный провайдер в app.go. Добавлены 4: balda_mattermost_commands_{enabled,listen_addr,path,token}. Иначе fx: missing types: string[name="balda_mattermost_commands_enabled"].
⚠️ Ключ обязан быть в cmd/balda/balda.yaml — env-override (BALDA_MATTERMOST_COMMANDS_*) применяется только к leaf-ключам, уже присутствующим в YAML (appconfig/loader.go). Молчаливое игнорирование иначе.
✅ DI-граф проверен реально — TestValidateCommandRunsValidationWithMattermostTransport прогоняет loadBaldaCommandConfig(true) + validateBaldaApplication(prepared) с включённым MM. В логе видно оба провайдера: NewCommandServer + NewIngress. go build этого не проверяет — только fx.ValidateApp.

Файлы (новые)

Файл	Строк
`channel/mattermost/command_server.go`	~409
`channel/mattermost/command_server_test.go`	~361
`locatorref/mattermost_test.go`	~94

Дельты (все применены, `tools/`)

deltas-pr8-b2-locatorref.json · -b2-locatorref-types.json · -p3-groupdm.json · -p3-groupdm-keys.json · -b1-wiring.json · -b1-config-keys.json · -lint-fixes.json · -docs-commands.json · -docs-mattermost.json

7. Доки

docs/commands.md — MM-колонка приведена к реальности: /start, /topic, /goalkeeper, /auto, /usage, /close, /cancel, /user, /locator = yes (было no — дока врала). Абзац про HTTP-природу slash + обе конфигурации команды.
docs/mattermost.md — новый раздел Slash command setup (Request URL, token, шаги), 4 ключа в таблице, env-образец, slash-специфичный troubleshooting.

8. Дальше

#	Шаг	Статус
1	Дождаться CI на `cd4ce4bc` (8 джобов)	⏳ идёт
2	Внешнее ревью PR #8 (`reviewDecision` пустой)	⛔ внешний блокер
3	Живая приёмка slash на Kraken (публичный HTTPS для Request URL!)	⏳ после 1
4	Фаза 4 — исполнитель (делегирование)	🎯 основной фронт

⚠️ Б1 требует публичного HTTPS для Request URL slash-команды. Стенд Kraken — проверить достижимость эндпоинта до приёмки; иначе нужен туннель.

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

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 — общий план

14 KiB Executable File Raw Blame History Unescape Escape