302 lines
15 KiB
Markdown
302 lines
15 KiB
Markdown
# Balda — Go/fx pitfalls и метод правок (зеркало скилла)
|
||
|
||
> Создано 2026-09-26. Обновлено 2026-09-26 (ночь) — добавлены lint/arch-lint и закрытие Фазы 2.
|
||
> Зеркало секции скилла `devops/docker-balda-service`, добавленной 2026-09-26.
|
||
> Источник: работа над Mattermost-адаптером Балды (Фаза 2).
|
||
> Проектные доки: [[personal/projects/balda/balda-mattermost-connector]],
|
||
> [[personal/projects/balda/balda-mm-delegation-plan]].
|
||
|
||
---
|
||
|
||
## 🔴 `go build` = exit 0 ≠ рабочий транспорт
|
||
|
||
Компилятор не видит ошибок DI-графа. `go build ./...` проходит, а приложение падает
|
||
на старте с `fx.ValidateApp`. Проверять транспорт **только** реальным запуском валидации:
|
||
|
||
```go
|
||
prepared, err := loadBaldaCommandConfig(true) // нужен os.Chdir в tempdir с .config/balda/config.yaml
|
||
err = validateBaldaApplication(prepared) // ← вот это и есть проверка графа
|
||
```
|
||
|
||
Эталонный тест: `TestValidateCommandRunsValidationWithMattermostTransport`
|
||
в `cmd/balda/command_test.go`. На нём поймано 3 дефекта, которые `go build` пропустил.
|
||
|
||
---
|
||
|
||
## 🔴 Дефект 1: `fx.In` + внешний `fx.ParamTags`
|
||
|
||
**Симптом:**
|
||
```
|
||
Failed: invalid annotation function func(mattermostfx.IngressParams) (*mattermost.Ingress, error):
|
||
fx.In structs cannot be annotated with fx.ParamTags or fx.From
|
||
```
|
||
|
||
**Причина:** `fx.Annotate(NewIngress, fx.ParamTags(...))`, где `IngressParams` **уже** содержит `fx.In`.
|
||
Теги обязаны жить внутри структуры, снаружи аннотировать нельзя.
|
||
|
||
**Неправильно:**
|
||
```go
|
||
fx.Annotate(NewIngress,
|
||
fx.ParamTags(``, ``, ``, `name:"balda_mattermost_enabled"`,
|
||
`name:"balda_mattermost_bot_user_id"`,
|
||
`name:"balda_mattermost_bot_username"`, ``),
|
||
),
|
||
```
|
||
|
||
**Правильно:**
|
||
```go
|
||
// структура — теги на полях
|
||
type IngressParams struct {
|
||
fx.In
|
||
Processor mattermost.InboundProcessor `optional:"true"`
|
||
Client *mattermost.Client
|
||
Commands *commandcmd.Registry // обязательный, не optional
|
||
Enabled bool `name:"balda_mattermost_enabled"`
|
||
BotUserID string `name:"balda_mattermost_bot_user_id"`
|
||
BotUsername string `name:"balda_mattermost_bot_username"`
|
||
Logger zerolog.Logger
|
||
}
|
||
|
||
// предоставление — голое, без fx.Annotate
|
||
fx.Provide(NewIngress),
|
||
```
|
||
|
||
Эталон в апстриме: `zulip.ServerParams` + `zulip.NewServer` в `zulipfx/module.go` (строка 24) —
|
||
`zulip.NewServer` объявлен **без** `fx.Annotate`.
|
||
|
||
⚠️ Поле-порт (`*commandcmd.Registry`) делать **обязательным**. `optional:"true"` на нём — лишнее:
|
||
в графе он есть всегда, а optional маскирует ошибки конфигурации.
|
||
|
||
---
|
||
|
||
## 🔴 Дефект 2: нет провайдеров зарегистрированных строк
|
||
|
||
**Симптом:**
|
||
```
|
||
missing types:
|
||
- string[name="balda_mattermost_server_url"] (did you mean to Provide it?)
|
||
- string[name="balda_mattermost_token"]
|
||
- string[name="balda_mattermost_bot_user_id"]
|
||
```
|
||
|
||
**Причина:** модуль читает значения через `name:`-теги, а провайдеров этих имён в composition root нет.
|
||
У Zulip они есть (`balda_zulip_*`), для нового транспорта забыли.
|
||
|
||
**Правило: каждый `name:`-тег в модуле = обязательный провайдер в `app.go`.**
|
||
|
||
```go
|
||
fx.Provide(
|
||
fx.Annotate(
|
||
func() string { return strings.TrimSpace(cfg.Balda.Mattermost.ServerURL) },
|
||
fx.ResultTags(`name:"balda_mattermost_server_url"`),
|
||
),
|
||
),
|
||
```
|
||
|
||
`strings.TrimSpace` для отсутствующего ключа YAML даёт `""` — без паники, как у Zulip.
|
||
Образец: блок `balda_zulip_*` в `internal/apps/balda/app.go` (~строки 576-606).
|
||
|
||
---
|
||
|
||
## 🔴 Дефект 3: забытый импорт корневого пакета
|
||
|
||
`app.go:965: undefined: mattermost` при вызове `mattermost.ValidateConfig`, если импортирован
|
||
только `mattermostfx`. Транспорт требует **оба** импорта:
|
||
```go
|
||
"github.com/baldaworks/balda/internal/apps/balda/channel/mattermost"
|
||
"github.com/baldaworks/balda/internal/apps/balda/channel/mattermost/mattermostfx"
|
||
```
|
||
|
||
---
|
||
|
||
## ⚠️ Env-override работает только по ключам, уже присутствующим в YAML
|
||
|
||
`appconfig/loader.go:367` — `SetEnvPrefix("BALDA")` + `SetEnvKeyReplacer(".", "_")` +
|
||
`AutomaticEnv()` **по leaf-ключам конфига**. Поэтому блок нового транспорта обязан быть
|
||
в `cmd/balda/balda.yaml` — иначе `BALDA_<X>_*` переменные **молча игнорируются**.
|
||
|
||
Порядок для нового транспорта:
|
||
1. блок в `cmd/balda/balda.yaml` (иначе env мёртв)
|
||
2. `MattermostConfig`-подобная структура + поле в `BaldaConfig`
|
||
3. провайдеры в `app.go`
|
||
4. `validate<X>Config` + вызов рядом с `validateSlackConfig`
|
||
5. включить в `validateBaldaChannelConfiguration` (`cmd/balda/command.go`) —
|
||
иначе деплой только на этом транспорте отбивается «at least one channel is required»
|
||
6. `.go-arch-lint.yml` — компоненты `<x>_transport`/`<x>_wiring` + права у
|
||
`composition_root` и `ingress_wiring` (см. §go-arch-lint выше)
|
||
7. тест на `validateBaldaApplication` с `<x>.enabled=true` (см. §«go build ≠ …» выше)
|
||
8. тест env-override в `cmd/balda/command_test.go` (по образцу
|
||
`TestLoadConfigDocument_AppliesZulipEnvOverrides`)
|
||
9. если добавлял маршрут в `BuiltinRoutes()` — обновить `TestBuiltinRoutes`
|
||
10. доки: `docs/reference/configuration.md` (ключи), `docs/commands.md` (колонка транспорта)
|
||
|
||
**Финальные quality gates (все обязательны, из `AGENTS.md`):**
|
||
```bash
|
||
go build ./... # 0
|
||
go vet ./... # 0
|
||
go tool golangci-lint run # 0 issues
|
||
go tool go-arch-lint check --project-path . # OK
|
||
go test -race ./...
|
||
```
|
||
⚠️ `go-arch-lint` **не входит** в `golangci-lint` — запускать отдельно, иначе ошибка
|
||
архитектуры всплывёт только на CI/ревью.
|
||
|
||
---
|
||
|
||
## 🔴 `go-arch-lint` — обязательный шаг для нового транспорта
|
||
|
||
⚠️ **Это не «опциональная придирка»: `AGENTS.md` прямо требует обновлять
|
||
`.go-arch-lint.yml` в том же изменении, что и архитектуру.** Без правки линтер падает
|
||
с двумя классами ошибок:
|
||
|
||
```
|
||
Component composition_root shouldn't depend on .../channel/mattermost
|
||
File /internal/apps/balda/channel/mattermost/adapter.go not attached to any component in archfile
|
||
```
|
||
|
||
**Что добавить в `.go-arch-lint.yml` (4 правки) — по образцу `zulip`:**
|
||
|
||
1. **Секция `components:`** — пары `in:`-компонентов:
|
||
```yaml
|
||
# Provider-owned Mattermost transport. It may depend only on
|
||
# transport-neutral contracts; application/runtime adapters live in wiring.
|
||
mattermost_transport:
|
||
in:
|
||
- internal/apps/balda/channel/mattermost
|
||
- internal/apps/balda/channel/mattermost/presentation
|
||
|
||
# Composition-root adapters for Mattermost delivery, prompt, and structured renderers.
|
||
mattermost_wiring:
|
||
in:
|
||
- internal/apps/balda/channel/mattermost/mattermostfx
|
||
```
|
||
⚠️ Root-пакет и `presentation/` идут в **один** компонент `_transport`, а `_fx`-пакет —
|
||
отдельно в `_wiring`. Именно так у `zulip`/`telegram`/`slackagent`.
|
||
|
||
2. **Правила `mayDependOn`:**
|
||
- `mattermost_transport` → `foundation, contracts, neutral_registry, mattermost_transport`
|
||
- `mattermost_wiring` → `foundation, contracts, neutral_registry, application, delivery,
|
||
storage, mattermost_transport, mattermost_wiring`
|
||
|
||
3. **`composition_root`** → добавить `mattermost_transport`, `mattermost_wiring`
|
||
(иначе «composition_root shouldn't depend on …» — это ожидаемый отказ, там
|
||
перечислены все транспорты).
|
||
|
||
4. **`ingress_wiring`** → добавить `mattermost_transport` (соседствует с `telegram_transport`,
|
||
`zulip_transport`).
|
||
|
||
Проверка: `go tool go-arch-lint check --project-path .` → должно быть `OK - No warnings found`.
|
||
|
||
---
|
||
|
||
## 🔴 `golangci-lint` на новом транспорте: мёртвый typing-throttle
|
||
|
||
Первый прогон дал 4 замечания — все в новом коде. Важный урок: **не гейтить линтер
|
||
nolint-комментарием, а удалять мёртвый код.**
|
||
|
||
| Замечание | Что было | Фикс |
|
||
|---|---|---|
|
||
| `inamedparam` ×2 | `Start(context.Context) error` в interface-assertion | `Start(ctx context.Context) error` |
|
||
| `unused` | `shouldSendTyping()` — недостижим, т.к. `SendTyping` всегда no-op | удалён |
|
||
| `unused` | `websocketReceiveTimeout` — константа без использования | удалена |
|
||
| `unlambda` | `func(client, logger) *Adapter { return NewAdapter(...) }` | заменено на `mattermost.NewAdapter` |
|
||
|
||
**Корневая причина `unused`:** механизм typing-throttle (`shouldSendTyping`, поля
|
||
`typingThrottleInterval`/`typingLastSentAt`, `typingMu sync.Mutex`,
|
||
`SetTypingThrottleInterval`) был скопирован из Zulip по аналогии. Но у Mattermost
|
||
**ботов нет typing API** (`SendTyping` осознанно возвращает `nil`), поэтому весь механизм
|
||
недостижим. Удалён целиком — не заглушён.
|
||
|
||
**Правило: при копировании транспорта с Zulip проверять каждую фичу на применимость.**
|
||
Zulip-специфичное, что НЕ переносится в транспорт без поддержки:
|
||
- typing-throttle (нужен интерактивный typing API)
|
||
- логика `sendWithPlainFallback` — переносится, но только на content-rejection
|
||
|
||
⚠️ **Каскад после удаления полей:** удаление `typingThrottleInterval` потянуло `SetTypingThrottleInterval`
|
||
→ `sync` (импорт) → `zerolog` (импорт в `module.go`). Удалять надо **всей группой**,
|
||
иначе серия `undefined`/`imported and not used`.
|
||
|
||
---
|
||
|
||
## ✅ Тест-регрессия: `BuiltinRoutes` и длина списка
|
||
|
||
Добавление маршрута в `BuiltinRoutes()` (`deliveryfmt/routes.go`) **ломает** `TestBuiltinRoutes`
|
||
в `deliveryfmt/registry_test.go`, если не обновить ожидание:
|
||
|
||
```
|
||
len(BuiltinRoutes()) = 5, want 4
|
||
```
|
||
|
||
Фикс — добавить строку в map `want`:
|
||
```go
|
||
{transport: TransportMattermost, deliveryFormat: DeliveryFormatNone}: NamePlainText,
|
||
```
|
||
📌 Нашлось только сравнением «с правками / без правок» через `git stash` — в списке
|
||
предсуществующих падений апстрима этого теста **не было**, значит это наша регрессия.
|
||
|
||
---
|
||
|
||
## ✅ 13 пакетов падают в апстриме изначально
|
||
|
||
Не пытаться «чинить» как часть своей задачи — это дефект апстрима. Проверять `git stash`
|
||
перед выводами о регрессии.
|
||
|
||
Падающие в `baldaworks/balda@de273d8f`:
|
||
- `TestInitCommand_NonInteractiveAutoSelectsRootAndGeneratesDetectedAgents` (cmd/balda)
|
||
- `TestMigrateSessionMemoryLayout_*` ×4 (state)
|
||
- `TestWorkspaceMCPImport/Export_UsesPersistedSessionMetadata` (session)
|
||
- `TestPluginResolverResolvesContainedArgvFilteredEnvAndLaunchSecrets` (mcpruntime)
|
||
- `TestResolveWorkspaceBaseBranch_ConfigPreferredWhenValidVal`, `TestCanonicalSessionMemory*` (apps/balda)
|
||
|
||
**Метод проверки регрессии:**
|
||
```bash
|
||
git stash push -q -m baseline --include-untracked
|
||
go test ./<пакет>/ -count=1 # смотрим падения БЕЗ правок
|
||
git stash pop -q # правки возвращаются
|
||
```
|
||
⚠️ `git stash` может задеть чужие стэши — в форке лежат `whale-20260623` и
|
||
`feat/zulip-events-polling`. Проверять `git stash list` до и после.
|
||
|
||
---
|
||
|
||
## Метод правок: дельта-апплаер вместо sed
|
||
|
||
Правило Alex: **никаких in-place скриптов и sed**. Инструмент в `~/Developer/balda/tools/`:
|
||
|
||
| Файл | Роль |
|
||
|---|---|
|
||
| `apply-project-deltas.py` | движок (JSON-дельты + состояние) |
|
||
| `run-project-deltas.sh` | обёртка `check` / `apply` / `list` |
|
||
| `deltas-<topic>.json` | группы правок |
|
||
| `.deltas-state.json` | sha256 применённых якорей |
|
||
|
||
```bash
|
||
cd ~/Developer/balda
|
||
bash tools/run-project-deltas.sh check tools/deltas-mattermost.json tools/.deltas-state.json
|
||
bash tools/run-project-deltas.sh apply tools/deltas-mattermost.json tools/.deltas-state.json
|
||
```
|
||
|
||
**Формат дельты:**
|
||
```json
|
||
{ "file": "путь/от/корня", "note": "что делаем",
|
||
"search": "уникальный фрагмент текущего текста",
|
||
"replace": "чем заменить" }
|
||
```
|
||
|
||
**Три исхода:** `replace` уже есть → `skipped` (идемпотентно); `search` ровно 1 раз → `applied`;
|
||
ни того ни другого → `error: ЯКОРЬ НЕ НАЙДЕН`.
|
||
|
||
📌 `check` **обязателен перед `apply`** — на нём поймано, что дельта валидации целилась
|
||
в несуществующий `validate()` в `config.go` (валидация живёт в `app.go`), и переписана до записи.
|
||
|
||
⚠️ Дельты с длинным `replace` писать в JSON через `write_file`, **не** heredoc — кавычки и `\n`
|
||
ломаются молча. Это тот же питфолл, что у SSH-heredoc с вложенными кавычками.
|
||
|
||
---
|
||
|
||
## Связанные заметки
|
||
|
||
- [[personal/projects/balda/balda-mattermost-connector]] — Фаза 2, где всё это найдено
|
||
- [[personal/projects/balda/balda-mm-delegation-plan]] — общий план
|
||
- [[personal/tech/hermes-secret-redaction-bypass]] — маскировка секретов в tool-выводе
|