git_admin/obsidian-vault
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity
Files
e414cf985f53ba2fcb8bf1feea2d5ed71ed37198
obsidian-vault/personal/tech/adk-hostedagent-openai-mcp.md
T

28 KiB
Executable File
Raw Blame History

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):

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:

// 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 добавляет привязку в набор серверов агента:

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 (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 toolset». Искать строго 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

// ❌ нормализация на сравнении — 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> для этого не требуется — достаточно доступа аккаунта к репозиторию.

# где реально живёт ветка 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 дал неверную версию. Один тест на присутствие интерфейса убил её за секунду:

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:

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":...} → исполнитель не получал аргументов

📌 Как проверить фактом, а не глазами (использовать при любой правке схем):

// пробник: как реально сериализуется 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 до одобрения мейнтейнером. Диагностика:

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.

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

Reference in new issue View Git Blame Copy Permalink