obsidian-vault/personal/tech/adk-hostedagent-openai-mcp.md

# ADK hostedagent — OpenAI/MCP: `duplicate tool` и правда о `ProcessRequest`

> Создано 2026-09-27. Зеркало скилла [[software-development/adk-hostedagent-openai-mcp|`adk-hostedagent-openai-mcp`]].
> Здесь — **исправление к скиллу** и разбор дефекта, найденного на Балде.
> Проект: [[personal/plans/balda-mm-phase4-agents]] §19, [[personal/projects/balda/balda-kraken-mm-deploy]] §19.
>
> 🔴 **ОБНОВЛЕНО 2026-09-28:** первая версия фикса дедупа (§4) и два теста (§5, §6)
> **отклонены ревью PR #5**. §4a объясняет причину и **описывает замену, реализованную в `d4ab923`**
> (явный `DedupKey`/`DedupPreferred` вместо вывода из URL). §5 и §6 помечены `✅ ИСПРАВЛЕНО`.
> Читать §4 только как историю — не копировать.
>
> 🔴🔴 **ОБНОВЛЕНО ВТОРИЧНО (2026-09-28, post-merge):** PR #4 и PR #5 **ВЛИТЫ** в апстрим
> (`c6dfc6d`, `6775d71`). **Мейнтейнер нашёл 2 настоящих дефекта в моём фиксе** — см. §4б.
> **Обязательно читать перед следующей правкой конфиг-поля.** Урок: фикс может пройти все
> гейты и быть **мёртвым** в продовом пути.

---

## 1. ⚠️ Исправление к скиллу (было неверно до 2026-09-27)

Скилл (v3.2.0) утверждал:

> «**MCP tools implement `RequestProcessor`** → их `ProcessRequest()` вызывает
> `toolutils.PackTool(req, tool)`, поэтому инструменты пакуются и в `toolPreprocess`, и в
> `toolsetPreprocess`».

**Это ложь.** Проверено исполняемым тестом:

```
go test ./agentfactory/ -run TestMcpToolsetImplementsRequestProcessor -v
→ MCP toolset "mcp_tool_set" does NOT implement ProcessRequest
```

В `adk/v2@v2.4.0`:

| Тип | Файл | `ProcessRequest` |
|---|---|---|
| `mcpTool` | `tool/mcptoolset/tool.go:88` | ✅ есть |
| `set` (сам тулсет) | `tool/mcptoolset/set.go:144-185` | ❌ **НЕТ** (есть только `Tools()`) |

Поэтому `toolsetPreprocess` (`base_flow.go:798`) на MCP-тулсетах делает `if !ok { continue }` —
**пропускает их**. Единственный паковщик для MCP-инструментов — `toolPreprocess` (`:778`)
по плоскому `Flow.Tools`, который наполняет `toolProcessor` (`tools_processor.go:49`).

⛔ **Не диагностировать `duplicate tool` как «двойную паковку двух слоёв ADK» — этот путь
физически невозможен.** Полчаса чтения `base_flow.go` вели именно к этой ложной версии.

---

## 2. Как выглядит дефект

```
error … "agent run: duplicate tool: \"balda.control.shutdown\""
warn  … command failed with retryable error   (delivery_attempt 1..5)
warn  … command deadlettered                  (terminal:unclassified)
```

`PackTool` (`tool/toolutils/toolutils.go:47-48`):

```go
if _, ok := req.Tools[name]; ok {
    return fmt.Errorf("duplicate tool: %q", name)
}
```

`PackTool` **не идемпотентен**, а `toolProcessor` схлопывает **все** тулсеты в один плоский
`Flow.Tools`. Значит: **два MCP-сервера с одинаковыми именами инструментов = падение хода.**

🔴 **Ключевая сигнатура:** обычный текстовый ответ работает, падает **только** ход с явным
запросом инструмента. Это указывает на момент паковки, а не на конфиг.

---

## 3. Настоящий корень (найден 2026-09-27)

Balda регистрировала bundled MCP-сервер **ДВАЖДЫ на один и тот же URL**:

```go
// internal/apps/balda/internalmcp/manager.go
// :206 — база для session-scoped брокера
m.contextBroker.SetBaseURL("http://" + res.Addr + baldaRoute)
// :213 — и тот же эндпоинт в реестр
m.registry.Set(bundledBaldaServerID, agentconfig.MCPServerConfig{
    Type: agentconfig.MCPServerTypeHTTP,
    URL:  fmt.Sprintf("http://%s%s", res.Addr, baldaRoute),
})
```

`sessionmemorymcp/broker.go`:
- `Bind()` (`:111`) → `ID: "balda-session-memory-"+token`, а URL (через `contextURL:216-225`) —
  **тот же эндпоинт** + `?<contextQueryParameter>=<token>`
- `Wrap()` (`:118-153`) — тот же handler, инжектит trusted-заголовки по токену

`agent/runtime_manager.go:344-345` добавляет привязку в набор серверов агента:

```go
registry.Set(binding.ID, binding.Config)
extraMCPServerIDs = append(extraMCPServerIDs, binding.ID)
```

→ `hostedToolsets` строит **два тулсета на один сервер** → одинаковые имена → `PackTool` падает.

**Триггер:** `balda.session_memory.enabled: true` создаёт scoped-привязку. Без неё дефекта нет.

---

## 4. Фикс — дедуп по эндпоинту ⚠️ (ОТВЕРГНУТ ревью PR #5, 2026-09-28)

> 🔴 **Статус: первая версия фикса отклонена ревьюer'ом metalagman в
> [[personal/plans/runtime-pr5-toolset-dedup-review-fixes|PR #5]] (`CHANGES_REQUESTED`).**
> Ниже — как было сделано; что именно неверно и чем заменяем — см. §4a. Не копировать
> этот раздел в новые реализации.

**Файл:** `runtime-local/agentfactory/agentfactory.go`, функция `hostedToolsets`.

Два конфига, адресующие **один host+path**, — один сервер с точки зрения пространства имён
инструментов. Различие в query (контекст-токен) не делает их разными.

Хелперы:
- `mcpEndpointKey(cfg)` — `url.Parse`, обнуляет `RawQuery` и `Fragment`.
  Покрывает **`http` и `sse`**; `stdio` возвращает `""` (общего эндпоинта нет) → никогда не схлопывается.
- `mcpURLIsScoped(cfg)` — есть ли query (признак session-scoped привязки Балды).

🔴 **Критично, какую запись оставлять.** Наивный «первый по алфавиту» оставляет `balda` и
**выбрасывает scoped-привязку** — краш уходит, но **session memory ломается** (теряется
контекст-токен). Поэтому keeper — запись **со** scoped-URL: она строгий супернабор
(без токена `Wrap` просто пропускает запрос в `next.ServeHTTP`).

Результат в логе:

```
[mcp] skipping duplicate toolset "balda": endpoint already served by "balda-session-memory-<token>"
```

---

## 4a. Почему версия из §4 неверна (ревью PR #5, 2026-09-28)

Три замечания ревьюer'а, все подтверждены фактами. Разбор и план — [[personal/plans/runtime-pr5-toolset-dedup-review-fixes]].

| # | Severity | Что не так |
|---|---|---|
| P1 | **blocking** | `mcpEndpointKey` стирает **все** query-параметры, а `Headers` в ключе не участвуют. Query и headers выбирают тенанта / авторизационный контекст. `/mcp?tenant=a` vs `/mcp?tenant=b`, или один URL с разными `Authorization` → схлопываются в один произвольный конфиг = **доступ к чужому tool-set**. Вывод идентичности из host/path — небезопасен. |
| P2 | test gap | `selectScopedOnly` (§5) **дублирует алгоритм продакшена** и сам вычисляет «ожидаемого» победителя → не проверяет фактически удержанный конфиг. Вакуумный ассерт. |
| P2 | test gap | Пробник `ProcessRequest(ctx any, req any)` (§6) — Go требует **точных** типов, реальная подпись `(agent.Context, *model.LLMRequest)` его **не** удовлетворяет → проверка не может сработать. |

📌 **Направление замены — РЕАЛИЗОВАНО 2026-09-28 в `d4ab923`** (PR #5):
- generic-слой (`agentfactory`) **перестал выводить** идентичность из формы URL —
  `mcpEndpointKey` и `mcpURLIsScoped` удалены;
- схлопывание только по **явному** ключу: `DedupKey string` в `agentconfig.MCPServerConfig`
  (+ `DedupPreferred bool` — кто выживает, когда записи не взаимозаменяемы);
- Balda объявляет свой дубль **сама** в composition layer:
  `internal/apps/balda/internalmcp/manager.go:213` (bundled) и
  `internal/apps/balda/sessionapp/module.go:130` (scoped-binding) ставят общий
  `sessionmemorymcp.BundledDedupKey = "balda-bundled-mcp"`;
- победитель — **детерминированный**: помеченный `DedupPreferred` > лексикографически меньший id.

📌 **Пустой `DedupKey` = дедуп выключен.** Это safety-свойство: конфиги, которые раньше
сливались по общему host/path, теперь **по умолчанию остаются разными**. Есть тест
`TestEmptyDedupKeyNeverCollapses`.

⚠️ Разные `?tenant=`/`Authorization` в общем слое остаются **разными серверами** — тесты
`TestDistinctQueryParamsNeverCollapse` и `TestDistinctHeadersNeverCollapse`.

⚠️ **Почему понадобился `DedupPreferred`, а не порядок id.** Лексикографически меньший id — это
`balda` (plain, **без** токена). Простая замена эвристики на сортировку была бы регрессией:
токен в URL функциональный (`contextQueryParameter = "balda_context"`, `Wrap` инжектит
`HeaderSessionLocator`/`HeaderSessionID`/`HeaderSessionBinding`, без токена session-memory
fail closed). Проверка кода брокера, а не симметрия с ревью, это поймала.

---

## 5. Проверка (что реально подтверждает фикс)

| Проверка | Команда | Ожидание |
|---|---|---|
| Тесты | `go test ./agentfactory/ -run 'Toolset\|MCPEndpoint\|McpToolset' -v` | 6/6 PASS |
| Ошибка ушла | `docker logs balda-mm \| grep -c "duplicate tool:"` | **0** |
| Инструмент вызван | лог: `function_response_part_count` | **1** |
| Ход завершён | `turn_complete` / `final_response` | `true` / `true` |
| Живая приёмка | пост владельца с явным вызовом инструмента | ответ с реальным результатом |

⚠️ **Питфолл своей же проверки:** `grep -c "duplicate tool"` даёт ложную **1**, потому что
собственная строка лога фикса — «skipping **duplicate tool**set». Искать строго
`grep -c "duplicate tool:"` (с двоеточием, как в формате `PackTool`).

⚠️ **Тест должен быть чувствительным.** Первая версия проверяла только *количество* тулсетов и
**проходила даже со сломанным фиксом** (отключение keeper-предпочтения через `if false && …` её
не ломало). Добавлен `selectScopedOnly` — теперь тест падает, если выживает не scoped-запись.
Слабый тест = ложная уверенность.

🔴 **НО: `selectScopedOnly` тоже признан вакуумным (ревью PR #5, §4a P2).** Он **копирует цикл
`hostedToolsets`** внутри теста и возвращает «ожидаемого» победителя сам — проверяется копия
алгоритма, а не реально удержанный конфиг (`hostedToolsets` отдаёт непрозрачные `tool.Toolset`;
тест считает только `len()`). Если поменять приоритет синхронно в проде и в копии — тест останется
зелёным. **Правильная форма:** ассертить фактически использованный endpoint — либо через
инъектируемый конструктор транспорта (тестовый хук, подмена в тесте + проверка `Transport.Endpoint`),
либо подняв реальные httptest-MCP-серверы с разными tool-листами. Урок усиливается: **копия логики
в тесте — тот же класс ошибки, что терпимый ассерт.** Тест обязан наблюдать **результат продакшена**.

✅ **ИСПРАВЛЕНО в `d4ab923`.** `selectScopedOnly` удалён. Реализован **тестовый хук**:
`var mcpTransportFactory = mcpTransportForConfig` — тест подменяет её на записывающую и ассертит
`Transport.Endpoint` конфига, реально дошедшего до сборки транспорта. Плюс два **негативных
контроля** (проверено скриптом):
1. возврат инференса из URL (стрип query) → `TestDistinctQueryParamsNeverCollapse` **FAIL**:
   `expected 2 toolsets for distinct tenants, got 1`;
2. возврат `ctx any, req any` → **build failed**
   (`*stubRequestProcessor does not implement requestProcessor`).

📌 **Правило, выведенное на практике:** новый тест на баг обязательно проверять **негативным
контролем** — вернуть баг и убедиться, что тест падает. Тест, который не падает на сломанном
коде, — это не тест, а его имитация. См. навык `batch-data-edit-review-loop`.

---

## 5a. 🔴 ПОСТ-МЕРЖ: мейнтейнер нашёл 4 дефекта в моём фиксе PR #5

До мержа в ветку прилетели 3 коммита мейнтейнера (`f8011c1`, `e888d5e`, `facb0d0`).
Два замечания — **настоящие дефекты моего кода**, не стилистика.

| Коммит | Что нашёл | Почему это дефект |
|---|---|---|
| `f8011c1` | **`hydrateMCPServerConfig` не переносил `DedupKey`/`DedupPreferred`** | 🔴 **Фикс был бы мёртвым в продовом пути `Build()`**: `hostedToolsets` читает гидратированные конфиги → ключ всегда пуст → дедуп не срабатывает **никогда**. Мой тест звал `hostedToolsets` напрямую, минуя гидратацию, поэтому дефект не поймал |
| `e888d5e` | `DedupPreferred: true` без `DedupKey` не отвергался | бессмысленная конфигурация проходила молча |
| `facb0d0` | убран мой `fmt.Printf` | библиотека не должна писать в stdout |
| — | 🔴 **`strings.TrimSpace(key)` на сравнении — нормализация** | ключ обязан быть **opaque**: `"tenant-a"` ≠ `" tenant-a "`. Нормализация склеила бы независимо сконфигурированные namespace'ы и **воссоздала бы cross-tenant merge**, который этот API и должен предотвращать |

### 🔴 Тест, обходящий продовый путь, не ловит дефект

Это **четвёртая форма** бесполезного теста (дополняет таблицу выше):

| Форма | Как выглядит | Почему бесполезна |
|---|---|---|
| Терпимый ассерт | `x != "A" && x != "a"` | валидное значение никогда не отклоняется |
| Копия логики в тесте | тест-хелпер переписывает цикл прода | проверяется копия, не результат |
| Вакуумный пробник | `ProcessRequest(ctx any, req any)` | типы не совпадут никогда, `t.Fatal` недостижим |
| **Юнит-тест внутренней функции** | тест зовёт `hostedToolsets(...)` напрямую, минуя `factory.Build()` | **проверяет половину конвейера**; дефект в гидратации/сборке конфига не виден |

📌 **Правило:** для фичи, живущей в конвейере «собрать конфиг → гидратировать → использовать»,
нужен **хотя бы один тест через публичную точку входа**. Эталон, появившийся в upstream после
ревью, — `TestFactoryBuildKeepsPreferredDedupEndpoint`: собирает фабрику через `New(...)`,
регистрирует MCP-серверы, зовёт `f.Build(...)` и ассертит, что до транспорта дошёл scoped-endpoint.

### 📌 Ключ дедупа обязан быть opaque

```go
// ❌ нормализация на сравнении — cross-tenant merge
key := strings.TrimSpace(cfg.DedupKey)
// ✅ ключ не трогаем; TrimSpace только для проверки «пустой ли»
key := cfg.DedupKey
if strings.TrimSpace(key) == "" { continue }
```

⚠️ И: несколько `DedupPreferred` в одной группе должны давать **ошибку**, а не выбирать
победителя по порядку id — активная авторизация не может зависеть от сортировки.

### 🔴 Осиротевший каталог чужого модуля ломает `go-arch-lint`

Пока в корне Balda лежал `runtime-local/` (копия runtime через `replace`), `go-arch-lint`
давал **56 notices** `File /runtime-local/taskmaster/core.go not attached to any component in
archfile` — хотя каталог не участвовал в сборке. После выноса → `OK - No warnings found`.

📌 Заметки arch-lint о «файлах вне компонентов» — сначала проверить, не осиротевший ли это
каталог, а не править `.go-arch-lint.yml`.

---

## 5б. 📌 PR создаётся через `gh` API — git-ремоут на базовый репо не нужен

Частая путаница: «как открыт PR, если ремоута на базовый репозиторий нет?».
PR идёт **из форка**: ветка пушится в `origin` (форк), затем
`gh pr create --repo <base> --head <owner>:<branch>`. Git-ремоут на `<base>` для этого
**не требуется** — достаточно доступа аккаунта к репозиторию.

```bash
# где реально живёт ветка PR
git ls-remote --heads <base-url> <branch>    # пусто → ветка в форке, так и должно быть
gh pr view <N> --repo <base> --json headRepositoryOwner,headRepository,headRefName,baseRefName
```

⚠️ `normahq/balda` и `baldaworks/balda` отдавали **идентичный** `main` (`c1fde3c4`) — это один
репозиторий (переименование/зеркало). **Не считать их разными апстримами.**

⚠️ База PR может отставать от `main`: у PR #8 база `de273d8f`, а `upstream/main` уже `c1fde3c4`
→ PR требует синхронизации перед мержем.


---

## 6. Метод: исполняемый пробник вместо чтения

Час чтения исходников ADK дал **неверную** версию. Один тест на присутствие интерфейса убил её
за секунду:

```go
import (
    "google.golang.org/adk/v2/agent"
    "google.golang.org/adk/v2/model"
)

type requestProcessor interface {
    ProcessRequest(ctx agent.Context, req *model.LLMRequest) error
}
if _, ok := toolset.(requestProcessor); ok {
    t.Fatal("unexpected: premise changed")
}
```
🔴 **Типы должны быть ТОЧНЫМИ (ревью PR #5, §4a P2).** Изначальная форма
`ProcessRequest(ctx any, req any) error` — **вакуумная**: Go требует точного совпадения типов,
поэтому реальный метод `(agent.Context, *model.LLMRequest)` такой интерфейс **не** реализует,
`ok` всегда `false`, `t.Fatal` **недостижим**. Пробник выглядел как проверка, но не проверял
ничего — ровно та же болезнь, что «терпимый ассерт» в §8. Полезное дополнение: **позитивный
контроль** — рядом проверить, что интерфейс реализуется заведомо подходящим типом; если сигнатура
разъедется, тест это покажет.

⚠️ Тип `agent.Context` в `adk/v2` — **интерфейс** (`github.com/google/uuid`-подобных полей нет).
Продакшн-подписи подтверждены в модульном кэше:
`adk/v2@v2.4.0/internal/toolinternal/tool.go:41` и `tool/tool.go:192`:
```go
ProcessRequest(ctx agent.Context, req *model.LLMRequest) error
```

📌 **Правило:** когда гипотеза касается **интерфейса или пути вызова** — писать исполняемый
пробник, а не читать дальше. Чтение даёт правдоподобные версии; тест даёт факт.

📌 **Искать ошибку в БАЗОВОМ модуле, а не в своём репо:** `grep -rn "duplicate tool"` по
репозиторию Balda **пуст** — текст живёт в `google.golang.org/adk` (`$GOMODCACHE`). Симптом
«в моём коде такого нет» означает «ищи в зависимостях».

---

## 7. Порядок сборки образа (урок сессии)

Сборка, запущенная **до** последней правки кода, даёт устаревший бинарник. В этой сессии build
стартовал, затем я доработал `mcpEndpointKey` (добавил `sse`), пришлось убивать процесс и
пересобирать.

**Правильный порядок:** все правки → финальный `go build` / `go test` → и только затем docker build.

---

## 8. Wire-контракт OpenAI tool_calls: 4 дефекта (2026-09-28)

`hostedagent/openai.go` — наш патч tool_calls поверх runtime v2. Ревью PR
**normahq/runtime#4** дало вердикт «Request changes» с 4 воспроизводимыми дефектами.
Все подтверждены фактами и закрыты коммитом **`0937011`**. Разбор —
[[personal/plans/runtime-pr4-openai-toolcalls-fixes]].

| # | Дефект | Суть |
|---|---|---|
| P1-a | `genai.Schema` маршалится напрямую | типы уходят как `"type":"OBJECT"`/`"STRING"` — **genai использует uppercase-константы**, а OpenAI требует JSON Schema lowercase → 400 |
| P1-b | `content,omitempty` у tool-message | пустой результат → `{"role":"tool","tool_call_id":"..."}` без `content`, а он **обязателен** по схеме OpenAI → 400 |
| P2-a | Ветка `toolResponses` | `Content{FunctionResponse + Text}` терял текст (ADK такой Content разрешает) |
| P2-b | Декод аргументов | не-объект (массив/скаляр) падал в мусорный `{"raw":...}` → исполнитель не получал аргументов |

📌 **Как проверить фактом, а не глазами** (использовать при любой правке схем):
```go
// пробник: как реально сериализуется genai.Schema
out, _ := json.Marshal(&genai.Schema{Type: genai.TypeObject, Properties: ...})
t.Logf("%s", out)   // → "type":"OBJECT"  ← вот дефект
```
📌 **`genai.Schema.Nullable` — это `*bool`**, не `bool`. Компилятор ловит:
`non-boolean condition in if statement`.

🔴 **Урок про маскирующие тесты.** Существовавший
`TestOpenAIToolsFromConfig_WithParameters` проверял
`params["type"] != "OBJECT" && params["type"] != "object"` — **принимал обе формы**, поэтому
uppercase-баг жил незамеченным через все гейты. **Терпимый ассерт = отсутствующая проверка.**
Ассерт должен пинить ровно одно ожидаемое значение.

📌 **Формы аргументов, которые надо переживать** (все встречаются у провайдеров):
`{"query":"x"}` · `"{\"query\":\"x\"}"` · `"\"{\\\"query\\\":\\\"x\\\"}\""` (двойное) ·
`"[...]"` (массив) · `"scalar"`. Корень бага: `json.Unmarshal` в `map[string]any` **падает**,
если после unwrap получился не объект.

📌 **`FunctionCall.Args` по контракту — объект.** Поэтому не-объектный результат оборачивается
под стабильный ключ, а не отбрасывается.

### ⛔ CI PR из форка: `action_required`, 0s

Все PR-раны для веток из форка (`isCrossRepository: true`) висят в статусе **`action_required`
с длительностью `0s`** — GitHub не запускает workflow до одобрения мейнтейнером.
Диагностика:
```bash
gh run list --repo normahq/runtime --limit 10     # → completed | action_required | 0s
gh pr view 4 --json isCrossRepository,headRepository
```
Контроль: PR из ветки **в самом upstream** проходит нормально (`1m5s`). Это **внешний блокер**,
не дефект кода. Обход — ветка в upstream при наличии write-доступа.

### ⚠️ Копия рантайма в balda — отдельная!

`~/Developer/balda/runtime-local` — **отдельная копия** (не симлинк), подключена через
`replace github.com/normahq/runtime/v2 => ./runtime-local`. Её HEAD — коммит **balda**, а не
runtime. Правки в `~/Developer/runtime-local` **не доезжают** до боевого стенда автоматически.
Проверять: `git -C ~/Developer/balda/runtime-local rev-parse --short HEAD`.

---

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

- [[personal/plans/runtime-pr5-toolset-dedup-review-fixes]] — **PR #5: 3 дефекта ревью дедупа MCP-тулсетов**, план замены (§4a)
- [[personal/plans/runtime-pr4-openai-toolcalls-fixes]] — PR #4: 4 дефекта, фиксы, тесты
- [[personal/plans/balda-mm-phase4-agents]] — §19 (корень, фикс, тесты, приёмка), §16.2 и §18 опровергнуты
- [[personal/projects/balda/balda-kraken-mm-deploy]] — Фаза 3, образ `balda-mm:0.1.13`
- [[personal/tech/balda-go-fx-pitfalls]] — fx-питфоллы Балды
- скилл `adk-hostedagent-openai-mcp` — источник (исправлен 2026-09-27)