> Создан 2026-09-28. Статус: **✅ ВЛИТО В АПСТРИМ (2026-09-28T13:22:13Z).** > Мой коммит: **`d4ab923`** (4 файла, +411/−1), ветка `fix/hosted-toolsets-duplicate-endpoint`. > **PR normahq/runtime#5** — **MERGED** (merge-коммит `6775d71`). > Ответ ревьюеру: https://github.com/normahq/runtime/pull/5#issuecomment-5867224210 > Репо: `~/Developer/runtime-local`. Связано: [[personal/plans/runtime-pr4-openai-toolcalls-fixes]]. --- ## 0а. 🔴 ПОСТ-МЕРЖ: мейнтейнер нашёл 2 дефекта в МОЁМ фиксе После моего `d4ab923` и **до** мержа в ветку прилетели 3 коммита мейнтейнера (`f8011c1`, `e888d5e`, `facb0d0` — head PR стал `facb0d0`). Это **code review моего кода**, и два замечания — настоящие дефекты: | Коммит | Что нашёл | Почему это мой дефект | |---|---|---| | `f8011c1` `fix(agentfactory): preserve MCP dedup metadata` | **`hydrateMCPServerConfig` не переносил `DedupKey`/`DedupPreferred`** в гидратированную копию | 🔴 **Фикс был бы мёртвым в продовом пути `Build()`.** `hostedToolsets` читает гидратированные конфиги → ключ всегда пуст → дедуп никогда не срабатывает. Мой юнит-тест звал `hostedToolsets` **напрямую**, минуя гидратацию, поэтому дефект не поймал | | `e888d5e` `fix(agentconfig): validate preferred MCP dedup` | `DedupPreferred: true` без `DedupKey` не отвергался | Бессмысленная конфигурация проходила молча | | `facb0d0` `fix(agentfactory): keep MCP dedup silent` | Убрал мой `fmt.Printf` из библиотеки; добавил ошибку при **нескольких** `DedupPreferred` в одной группе | Библиотека не должна писать в stdout; молчаливый выбор победителя делал активную авторизацию зависимой от порядка id | | — | **`strings.TrimSpace(key)` — нормализация ключа** | 🔴 Ключ обязан быть **opaque**: `"tenant-a"` и `" tenant-a "` — разные namespace'ы. Нормализация склеила бы независимо сконфигурированные namespace'ы и воссоздала бы cross-tenant merge, который этот API и должен предотвращать | **Заменённые строки в моём коде:** ```go // было (мой вариант) // стало (upstream/main) key := strings.TrimSpace(cfg.DedupKey) key := cfg.DedupKey if strings.TrimSpace(key) == "" { continue } skip[id] = true skip[id] = true // без fmt.Printf ``` + `hydrateMCPServerConfig` теперь копирует `DedupKey`/`DedupPreferred`. 📌 **ГЛАВНЫЙ УРОК (durable):** юнит-тест, вызывающий внутреннюю функцию **напрямую**, не покрывает продовый путь, где конфиг проходит через **гидратацию/нормализацию**. Мейнтейнер добавил `TestFactoryBuildKeepsPreferredDedupEndpoint` — он идёт через `f.Build()` (registry lookup → hydration → hostedToolsets). **Правило: тестировать сквозь композиционный путь, а не только целевую функцию.** Этот дефект дал бы «зелёные гейты и неработающий фикс» — ровно тот класс ошибки, против которого был весь PR. --- ## 0. ИТОГ — что сделано Коммит **`d4ab923`** `fix(agentfactory): dedup MCP toolsets by explicit key`. **Гейты локально — все зелёные:** ``` gofmt -l agentfactory/ agentconfig/ → чисто go build ./... → OK go vet ./... → OK go test ./... → все 10 пакетов ok go test -race ./... → все ok go tool golangci-lint run ./... → 0 issues ``` | # | Дефект ревью | Фикс | |---|---|---| | 1 | **P1** ключ стирал query и игнорировал `Headers` → слияние разных тенантов/авторизаций | `mcpEndpointKey` + `mcpURLIsScoped` **удалены**. Инференс из URL убран. Схлопывание только по явному непустому `DedupKey`; пусто → **никогда** не схлопывается | | 2 | **P2-1** `selectScopedOnly` копировал алгоритм прода | **удалён**. Тесты пишут endpoint каждого конфига, реально дошедшего до `mcpTransportFactory` (инъектируемая переменная), и ассертят на нём | | 3 | **P2-2** `ProcessRequest(ctx any, req any)` — вакуумный | Точная продакшн-сигнатура `(agent.Context, *model.LLMRequest)` + позитивный контроль `TestRequestProcessorInterfaceIsSatisfiable` | **Новые поля `agentconfig.MCPServerConfig`** (аддитивно, по умолчанию выключено): - `DedupKey` — явный ключ эквивалентности (opaque, задаёт вызывающий); - `DedupPreferred` — кто выживает в группе, когда записи не взаимозаменяемы. Приоритет победителя: помеченный `DedupPreferred` > лексикографически меньший id (детерминированно, не зависит от порядка map). **Balda — composition layer (как просил ревьюер):** - `sessionmemorymcp.BundledDedupKey = "balda-bundled-mcp"` — общий ключ; - `internal/apps/balda/internalmcp/manager.go:213` — bundled-запись получает `DedupKey`; - `internal/apps/balda/sessionapp/module.go:130` — scoped-binding получает `DedupKey` + `DedupPreferred: true`. **Тесты (8 в `agentfactory`):** `TestHostedToolsetsCollapsesScopedEndpointDuplicate`, `TestDistinctQueryParamsNeverCollapse`, `TestDistinctHeadersNeverCollapse`, `TestSameDedupKeyCollapsesThreeEntries`, `TestDedupPreferredDecidesPlaceholderWinner`, `TestDedupWithoutPreferenceIsLexicographicallySmallest`, `TestEmptyDedupKeyNeverCollapses`, `TestHostedToolsetsKeepsDistinctEndpoints`, `TestMcpToolsetRequestProcessorSignature`, `TestRequestProcessorInterfaceIsSatisfiable`. ### 🔴 Негативные контроли (доказано, что тесты не вакуумные) Проверено скриптом `/tmp/negctl.sh`: 1. Вернул инференс из URL (стрип query) → `TestDistinctQueryParamsNeverCollapse` **FAIL**: `expected 2 toolsets for distinct tenants, got 1`. Лог: `skipping duplicate toolset "tenant-b": dedup key "http://h:1/mcp" already served by "tenant-a"`. 2. Вернул `ctx any, req any` → **build failed**: `*stubRequestProcessor does not implement requestProcessor (wrong type for method ProcessRequest)`. 3. Файлы восстановлены **байт-в-байт** (`diff` чистый), ре-верификация зелёная. --- ## 1. 🔴 КЛЮЧЕВОЕ ОТКРЫТИЕ: scoped-запись ОБЯЗАНА выживать Изначально планировался лексикографический победитель (`balda` < `balda-session-memory-*`), т.е. **plain** запись. При проверке кода брокера выяснилось — это сломало бы session-memory: `internal/apps/balda/sessionmemorymcp/broker.go`: - `contextQueryParameter = "balda_context"` — токен читается **из query** (`:124`); - `Wrap` (`:118-150`): без токена запрос идёт дальше без инъекции `HeaderSessionLocator`/ `HeaderSessionID`/`HeaderSessionBinding`; - комментарий в коде: *«session-memory calls still fail closed in HeaderSessionResolver»*. → Токен в URL **функциональный**, а не косметический. Поэтому: 1. нужен был `DedupPreferred` (порядок id не подходит); 2. заметка в тесте исправлена: `?ctx=` → реальный `?balda_context=`. ⚠️ **Вывод на будущее:** старая эвристика `mcpURLIsScoped` («есть query → scoped») давала **правильного** победителя случайно. Замена на «лексикографически меньший» без `DedupPreferred` была бы регрессией. Проверка кода, а не симметрия с ревью, её поймала. --- ## 2. Что говорит ревью (дословно, 3 пункта) Ревьюер **metalagman**, `CHANGES_REQUESTED`, 2026-09-28T09:04:10Z (опубликовано в PR). | # | Severity | Суть | |---|---|---| | 1 | **P1 — blocking** | `mcpEndpointKey` выбрасывает все query-параметры, группировка игнорирует `Headers`. `/mcp?tenant=a` vs `?tenant=b`, или один URL с разными `Authorization`, схлопываются в один произвольный конфиг → доступ к чужому tool-set / пересечение границы доступа. Просьба: *«keep generic endpoint identity exact»* + *«resolve the Balda-specific duplicate in the Balda composition layer, or introduce explicit alias/replacement metadata»* | | 2 | **P2** | `selectScopedOnly` повторяет алгоритм продакшена и вычисляет победителя сам — не проверяет фактически удержанный toolset | | 3 | **P2** | `ProcessRequest(ctx any, req any)` — Go требует точных типов, реальный метод интерфейс не удовлетворит; ассерт не может сработать | **Validation ревьюера:** `go test -race ./...`, `go vet ./...`, `golangci-lint run ./...` — проходили. `no check runs/statuses` — тот же fork-блокер, что в PR #4. --- ## 3. ⛔ CI: внешний блокер (не наш дефект) `gh pr checks 5` → `no checks reported on the 'fix/hosted-toolsets-duplicate-endpoint' branch`. Причина та же, что в PR #4 (§0в в [[personal/plans/runtime-pr4-openai-toolcalls-fixes]]): **fork-PR** (`mallexxx/runtime` → `normahq/runtime`), workflows ждут одобрения мейнтейнера. Починить не можем. Локальные гейты эквивалентны CI (`test.yml` + `lint.yml`). --- ## 4. Питфоллы, найденные в работе 1. 🔴 **`write_file` с относительным путём пишет НЕ в vault.** Первый `write_file` с путём `personal/plans/runtime-pr5-...md` создал файл в `/Users/admin/Developer/balda/personal/...`. Для vault — **только `mcp_obsidian_write_note`** (vault root `/Users/admin/obsidian`). 2. ⚠️ **`rm -rf` в терминале требует подтверждения** — таймаут апрува заблокировал команду. Лишнее дерево `/Users/admin/Developer/balda/personal/` **не удалено** (см. §6). 3. ⚠️ **LSP-диагностика после `patch` бывает устаревшей** — показывала `undefined: mcpEndpointKey` после того, как `go build` уже был чист. Верить `go build`/`go test`, не LSP-снапшоту. 4. ⚠️ Правки — **только `patch`**, не sed/in-place (правило Alex). Ни одного sed не использовано. 5. ⚠️ Untracked `tools/` в репо — **не мой** файл, не коммитил. --- ## 5. Осталось сделать 1. ✅ **Ре-ревью пройдено, PR влит** (`6775d71`). 2. 🔴 **Синхронизация копии рантайма в Balda — ОТКРЫТО, блокирует боевой стенд.** `~/Developer/balda/runtime-local` — **не клон runtime**, а **снапшот кода эпохи PR #4** (3 файла отличаются от `upstream/main`, все три — в пользу апстрима; `DedupKey` — **0 вхождений**). Каталог **git-ignored** в balda и подключён через `replace github.com/normahq/runtime/v2 => ./runtime-local`. Правки Balda-слоя из §0 **не компилируются** против него (`unknown field DedupKey in struct literal of type agentconfig.MCPServerConfig`). ✅ **Потери при замене НЕТ** — проверено `diff -rq`: отличаются только `agentfactory/agentfactory.go`, `hostedagent/openai.go`, `hostedagent/openai_test.go` (все — устаревшие версии того, что уже в апстриме). ⚠️ В апстриме мой код **уже исправлен мейнтейнером** (§0а) — синхронизировать надо с `upstream/main` (`c41939f`), а не с моим `d4ab923`. План и развилка (убрать `replace` совсем vs заменить копию) — [[personal/plans/balda-runtime-rebuild-mm-validation]] §1/§3. 3. ⚠️ `/tmp/negctl.sh`, `/tmp/control1.py`, `/tmp/control2.py`, `/tmp/af.bak`, `/tmp/dt.bak`, `/tmp/commitmsg.txt`, `/tmp/reply.md`, `/tmp/pr5_af.go`, `/tmp/pr5_test.go` — временные. 4. ✅ Ветки PR #4/#5 больше не «утекли» — обе влиты. --- ## 6. Требует решения Alex Удалить лишнее дерево, созданное ошибочным `write_file` (содержит только пустые `plans/` + `tech/`, ничего ценного): ``` /Users/admin/Developer/balda/personal/ ``` Удаление требует явного подтверждения — не трогал. --- ## 7. Контекст PR (исходная проблема) `hostedagent`/`agentfactory` — ADK разворачивает каждый toolset в один плоский список инструментов, а упаковка **не идемпотентна**: два сервера с одинаковым именем инструмента аварийно завершают ход с `duplicate tool: "balda.control.shutdown"`. Balda регистрирует bundled MCP-сервер дважды: как `"balda"` и как session-scoped binding того же listener'а с токеном в query. Оба отдают один список инструментов → каждый tool-using ход падал. Исходный фикс (`b01bdc6`) схлопывал по host/path — что и открыло P1. ## Связанные заметки - [[personal/plans/runtime-pr4-openai-toolcalls-fixes]] — PR #4, fork-CI-блокер, вопрос синхронизации копии - [[personal/tech/adk-hostedagent-openai-mcp]] — архитектура MCP/toolset, `runtime.mcp_servers` - [[personal/tech/balda-go-fx-pitfalls]] — метод дельт, питфоллы