release: corte de agosto — develop para main (36 commits) - #54
Open
gomessguii wants to merge 48 commits into
Open
release: corte de agosto — develop para main (36 commits)#54gomessguii wants to merge 48 commits into
gomessguii wants to merge 48 commits into
Conversation
…-2163/2164/2165) Three chained bugs in the Google Calendar tools that stopped the AI agent from listing free slots and scheduling meetings. Single umbrella PR (product decision recorded on the cards) since all three live in the same two files with a real dependency (#2/#3 only become observable after #1). EVO-2163 (#1) businessHours.enabled was tested at the ROOT level, but `enabled` only exists per-day -> every day was skipped -> "Found 0 available time slots". Fix: check_availability.py reads the per-day config directly. EVO-2164 (#2) credentials reach the tools SANITIZED (no refresh_token/client_id/ client_secret) via agent.config.integrations, breaking the Google token refresh (RefreshError). Fix: both tools reload the full credentials from evo_core_agent_integrations (short-lived psycopg2 connection) when the secrets are missing; explicit error if still incomplete. EVO-2165 (#3) slot search made one Google API call PER candidate slot (~60 calls, ~42s) -> blew past the bot_runtime 30s timeout, so the answer never reached WhatsApp. Fix: prefetch the whole window in ONE call and test slots in memory. Mirrors the fix validated end-to-end in the client's production (0 -> 76 slots, ~0.37s tool time, event created). Adds tests/unit/test_google_calendar_availability.py (6 tests). Documented follow-ups (not applied): docstring builder + base.py is_within_business_hours share bug #1's root cause but were not part of the validated production fix (base.py would be a behavior change).
… silent 401
validate_agent_api_key swallowed ANY exception into {"valid": False}, which the
auth middleware turned into a 401. A transient DB blip (dead pooled connection,
timeout, DB overload) therefore masqueraded as an auth failure — a silent,
non-retryable 401 that dropped the customer's message with no signal it was
transient. This survives EVO-2141 (pool_pre_ping): pre-ping cuts the dead-conn
case, but any other infra exception still became a 401.
- agent_service.py: new AgentValidationError; validate_agent_api_key now RAISES
it on SQLAlchemyError / unexpected errors instead of returning valid:False.
Auth decisions (agent not found, no config, key mismatch) still return
valid:False -> 401 as before.
- evo_auth.py: the three agent-key validation sites (/sync/, token-fallback,
main) map AgentValidationError to 503 (retryable) instead of 401.
- tests: infra error -> AgentValidationError / middleware 503; invalid key ->
valid:False / middleware 401 (unchanged).
Complements EVO-2167 (bot-runtime retry): the bridge can now retry the 5xx.
The agent had only check_availability and create_event, so when a customer asked to cancel a meeting the LLM hallucinated a cancellation — no tool ran and the event stayed on the calendar. - cancel_event.py: new cancel_calendar_event tool. Searches the window in one API call (via client.check_availability), optionally filters by title, deletes via events().delete(sendUpdates="all"). Disambiguates on multiple matches (needs_clarification + list), returns an informative message on zero matches (no hallucination), supports direct event_id deletion. Same sanitized-credentials DB reload as the other tools; same calendar (default primary) as create_event. - __init__.py / tool_builder.py: register the tool (log now lists cancel_event). - tests: single match cancels + notifies; multiple -> disambiguation; none -> informative message; title filter; event_id direct; sanitized creds reloaded. Validated in the client's production (create+cancel ok, Google shows cancelled, non-existent -> "not found", no hallucination). Note: create/check/cancel use the `primary` calendar, not the UI-configured one — a separate calendar_id bug, out of scope here (follow-up).
…ting) Completes the calendar toolset: the agent could create and cancel, but not edit / reschedule an existing meeting. - edit_event.py: new edit_calendar_event tool. Locates the event by event_id (events().get) or by the search window (client.check_availability) + optional title. Applies only the provided fields (new_start_date/new_end_date to reschedule, new_title, new_description) via events().patch(sendUpdates="all"). When only new_start_date is given, the original duration is preserved. Requires at least one change; disambiguates on multiple matches; informative message on none. Same sanitized-credentials DB reload and default calendar as the others. - __init__.py / tool_builder.py: register the tool (log lists edit_event). - tests: reschedule preserves duration; title-only; explicit start+end; no-change error; multiple -> disambiguation; none -> informative; event_id via get; sanitized creds reloaded. Validated in the client's production (14:00-15:00 -> rescheduled to 16:30 -> Google confirmed 16:30-17:30, 1h preserved, title changed). Independent of EVO-2169 (#42, cancel): both add their own tool file. When both merge, the only overlap is the tools-registered log line — a trivial resolution.
…ty + create_event The tools defaulted calendar_id to 'primary' and never read the calendar the user picks in the UI (settings.selectedCalendarId), so the agent operated on the OAuth account's primary calendar instead of the configured one. Resolve the calendar from config (config selection is authoritative; fall back to the tool arg / primary) and use it in every API call. Tests assert the selected calendar is used and that an empty/whitespace selection falls back to primary. Part of EVO-2171 (also applied to cancel_event #42 and edit_event #43).
Resolve the calendar from config (selectedCalendarId, else the tool arg / primary) so cancel searches and deletes on the same calendar create_event uses, not always primary. Removes the now-outdated comment that assumed create_event ignores the selection. Test asserts search + delete target the selected calendar.
Resolve the calendar from config (selectedCalendarId, else the tool arg / primary) so edit locates (events().get / search) and patches the event on the calendar the event lives on, not always primary. Test asserts search + patch target the selected calendar.
…unexpected infra error Review follow-ups on PR #41: - agent_service: unexpected-exception branch now logs with exc_info=True so a genuine code bug masked as a retryable 5xx stays diagnosable. - test_agent_auth_infra_5xx: add middleware coverage for the main (validated agent token) path — the path named in the incident root-cause — asserting infra error -> 503 and invalid key -> 401. Guards against a stray inner except regressing it back to a silent 401.
…uth-infra-5xx fix(EVO-2166): agent key validation returns 5xx on infra error, not a silent 401
…y-scheduling fix(google-calendar): AI agent lists free slots and schedules (EVO-2163/2164/2165)
…event-tool feat(EVO-2169): add cancel_calendar_event tool for the AI agent
…event-tool # Conflicts: # src/services/adk/tool_builder.py # src/services/adk/tools/google_calendar/__init__.py
…ent-tool feat(EVO-2170): add edit_calendar_event tool (edit / reschedule)
process_files built an inline_data Blob for every file and saved it to artifacts,
but appended it to file_parts only `if is_audio`. So an image was blobbed + saved
yet never sent to the LLM, and create_content("", file_parts) returned None ->
the agent replied "No content to process". Append EVERY file part (image/audio/
video/...) to file_parts; keep the is_audio branch only for the log label. Audio
behavior is unchanged (it was already appended). Part of EVO-2178 (image end-to-end).
- runner_utils.py: unconditional file_parts.append after the artifact save.
- tests/unit/test_media_file_parts.py: image appended; audio still appended; both;
create_content("", [image]) is not None (regression guard); create_content("", []) None.
The unconditional append from 84aab90 fixed the image but opened a worse failure: every file now goes to the model as inline_data, and google-adk's LiteLlm -- which every LLM agent is built on (llm_agent_builder) -- raises ValueError for a mime type it cannot carry. That ValueError reaches standard_runner's handler and becomes an InternalServerError, so an ordinary WhatsApp document (docx/xlsx/zip) now costs the whole turn: before 84aab90 the same message was answered, the file was just dropped. A caller that omits `mimeType` hits the same path -- a2a_routes.extract_files_from_message defaults to application/octet-stream. - runner_utils.py: `_inline_skip_reason` gates the append on what ADK actually converts (text//image//audio//video/ + application/pdf + application/json) and on per-file / per-request byte ceilings mirroring the bot-runtime bounds (ai_adapter.go). A skipped file is still saved as an artifact and logged with the reason; the rest of the message still gets an answer. - runner_utils.py: the append moved ahead of save_artifact, so a failing artifact store can no longer swallow the file and bring the original bug back. - test_media_file_parts.py: unreadable types stay out (docx/zip/octet-stream/ empty) while the caption still reaches the model; an unreadable file does not drop the image beside it; pdf/text still travel; mime parameters ("audio/webm;codecs=opus") are normalized for the check and verbatim on the Blob; the image survives an artifact store failure; both byte ceilings; and a contract test running every forwarded type through the installed ADK's _get_content, so an ADK bump that narrows the set fails here, not in front of a customer. Unit suite: 239 passed (was 227).
…eaches-model fix(EVO-2181): let images reach the model (not only audio)
Review follow-up to EVO-2181. _inline_skip_reason normalized the content type before checking it (lowercase, parameters dropped) while the Blob kept it verbatim, so the guard and ADK were answering different questions. _get_content matches the raw value with a case-sensitive startswith and an exact-match set, which means "IMAGE/PNG" and "application/pdf; charset=binary" cleared the guard and then raised ValueError inside ADK — the 500 that costs the customer the whole turn, caption included, and precisely what the guard was added to prevent. Checking the verbatim value closes the gap without changing any accepted case: "audio/webm;codecs=opus" still matches the audio/ prefix with its parameter attached. The new tests assert both halves — that those types are skipped, and that ADK really does raise on them — so an ADK bump that widens what it accepts fails here instead of quietly dropping readable media. Also adds .github/workflows/ci.yml: nothing ran pytest on a PR, including the PR this fixes. tests/unit/test_exception_handlers.py is excluded there with the reason written down — it fails at collection because generic_exception_handler no longer exists, which is a separate regression.
…mime-guard fix(EVO-2181): judge the inline mime exactly as ADK does
…om fallback inline O builder de agente externo passa a resolver o segredo pelo cofre quando a integração carrega `credential_id`, e a manter o valor inline quando não carrega. Nenhuma instalação precisa migrar nada para esta mudança entrar: o caminho antigo continua intacto até a 2.7 remover o fallback. A resolução aqui é POR ID, deliberadamente. Precedência entre escopos tem um dono só, o resolvedor do CRM (story 2.2); percorrer cadeia no runtime criaria uma segunda verdade sobre qual credencial vence. O que o processor faz é buscar valor, não decidir hierarquia. O mapa de campo por provedor fica num lugar só, porque errar um nome aqui produz auth vazia em silêncio, não erro: dify, flowise e openai leem `apiKey`; n8n lê o par `basicAuthUser`/`basicAuthPass`, que vem do envelope composto do cofre e precisa ser traduzido; typebot não tem credencial nenhuma e está registrado com tupla vazia, para a ausência ler como decisão e não como esquecimento que alguém tenta "consertar" depois. Ordem de precedência: referência resolvida vence, senão inline, senão erro explícito. O erro só dispara quando o usuário PEDIU o cofre e não há inline para usar: mandar chave vazia ao provedor falharia mais longe e com mensagem pior. A razão da falha viaja na mensagem, porque "é oauth" e "não existe" pedem correções diferentes de quem configurou o agente. O vault lê a tabela pela sessão que o builder já tem, com query parametrizada e escopada a uma linha. Não abre conexão própria: o padrão de psycopg2 cru que existe nas tools de Calendar ignora ORM e tenant, e a própria story manda não copiá-lo. Prova negativa verificada: removendo o fallback inline, três testes falham (referência não resolvida, referência oauth e valor indecifrável). Nota: o módulo é livre de imports pesados de propósito, e o teste o importa por caminho, porque `src.services.__init__` puxa a stack ADK inteira e o ambiente local não tem essas dependências instaladas. Refs EVO-2250 (story 2.3)
…amento do os.environ Custom tools, MCPs remotos, MCPs oficiais e o Knowledge Nexus passam a resolver seus segredos pelo cofre quando há referência, e a manter o valor inline quando não há. Nada quebra antes da migração 2.6. A referência é um MAPA (nome do header ou da env var → id da credencial): a regra do épico é uma credencial por segredo, então uma tool com dois headers de auth referencia duas credenciais. O Nexus tem um segredo só, então o credential_id escalar do diálogo é adaptado para o mesmo formato de mapa. Os DOIS caminhos de injeção de header foram tratados (custom_tools.py e tool_builder.py). Endereçar só um deixaria o hardening pela metade, que é o alerta explícito da story. DECISÃO REGISTRADA (o ou/ou que a story obriga a resolver): o vazamento do os.environ em mcp_context.py foi CORRIGIDO aqui, não adiado. Cada env var de MCP era gravada no os.environ do processo do processor além de ser passada ao filho pelo env= da linha seguinte. A escrita era redundante e nunca desfeita: token de um agente vazava para todo subprocesso MCP posterior e, no enterprise, entre tenants. Como este é exatamente o ponto por onde os segredos resolvidos pelo cofre passam agora, deixá-lo desfaria o ganho do cofre uma linha depois. Junto: os dois logs que despejavam o mapa de headers inteiro antes do helper de máscara passam a registrar só os NOMES. Bearer token não volta a aparecer em log. Provas negativas verificadas: reintroduzir a escrita no os.environ quebra test_mcp_context_no_longer_pollutes_os_environ, e remover o fallback inline quebra os testes de referência não resolvida. Nota de ambiente: os testes rodam isolados (importação por caminho), porque src.services.__init__ puxa a stack ADK e as dependências não estão instaladas aqui. A suíte do repo já não coletava antes deste trabalho. Pareia com o commit do core (schema credential_refs + redação de header) e o do CRM (bot de canal) da mesma story. Refs EVO-2250 (story 2.4)
Corrige os bloqueadores 1 e 2 da reprovação (review de 2026-07-29) e o achado 13.
BLOQUEADOR 1 — a resolução de header de MCP remoto era CÓDIGO MORTO.
`_resolve_mcp_headers` existia, tinha teste unitário e ZERO chamadores: um
`git grep` devolvia só a definição. O ponto real de montagem seguia com
`"headers": custom_server.headers or {}` cru, então um MCP remoto configurado
com `credential_refs` e sem header inline saía SEM AUTENTICAÇÃO. Agora a
montagem chama o resolvedor.
BLOQUEADOR 2 — env var de MCP oficial nunca resolvia pelo cofre. O ponto de
montagem fazia `update(server.get("envs", {}))` verbatim. Entra
`_resolve_mcp_envs`, chamado no mesmo lugar, lendo `credential_refs` como mapa
(nome da env var → id da credencial) e reusando o mesmo `resolve_credential_refs`
de tools e MCPs remotos.
⚠️ A LEITURA está ligada, mas a AC7 NÃO fecha só com isso: nada persiste
`credential_refs` na entrada de MCP do agente (ponta de escrita, no front). Até
existir, todo install segue no caminho verbatim. O contrato foi enviado ao
Reviewer; o estado real está registrado no docstring, não escondido.
ACHADO 13 — valor de header ia para o log. Os dois sites de `mcp_context.py`
mascaravam SÓ `authorization`, então `X-API-Key` e qualquer header de auth
customizado saíam em claro. A máscara passa a ser derivada de uma ALLOWLIST de
nomes seguros, espelhando o `safeHeaderNames` do secretmerge no Go: denylist de
nomes que "parecem auth" deixa passar `X-Tenant-Auth` e afins.
OS TESTES SÃO DE CAMINHO, não da função. Foi exatamente teste de função isolada
que deixou o defeito passar: ele passava enquanto ninguém chamava. Os novos
afirmam sobre o ponto de montagem e falham se a chamada for removida — provado
removendo-a.
Prova de chamador (o que a reprovação pediu):
src/services/adk/mcp_service.py:754 "headers": _resolve_mcp_headers(...)
src/services/adk/mcp_service.py:393 ...update(_resolve_mcp_envs(server, db))
Refs EVO-2250
Achado pelo Reviewer ao revisar minha própria correção do bloqueador 2, e ele
está certo: eu tinha ligado a chamada no lugar certo lendo a chave ERRADA, então
a resolução continuava inerte mesmo com a chamada no ponto de montagem. Conferi
o pipeline inteiro antes de aceitar:
1. FRONT grava `environments` (MCPConfigDialog é o único escritor). `envs` tem
zero ocorrências reais no front.
2. CORE valida por `environments` (config_processor.go:266, com erro
"server environments must be a dictionary") e REESCREVE a entrada persistida
com exatamente {id, environments, tools} (:278-282).
Consequência dupla que o Reviewer nomeou: (a) o guard em `server.get("envs")`
nunca era verdadeiro para agente configurado pela tela, então
`_resolve_mcp_envs` não rodava; (b) mesmo que rodasse, a allowlist do core
descartaria `credential_refs` antes de chegar ao processor — essa metade é do
Reviewer e ele está fazendo nesta rodada.
Meu lado, os três pontos: o guard e o resolvedor passam a ler `environments`,
tolerando `envs` para entradas escritas antes da reconciliação, e o
`MCPServerConfig` do schema ganha `environments` e `credential_refs` (declarava
só `envs`, mesma divergência).
O teste novo afirma sobre a CHAVE QUE O PIPELINE ESCREVE, não sobre a existência
da chamada: uma chamada no lugar certo com a chave errada passava no teste
anterior. Prova negativa: voltar para `envs` quebra o teste.
Ressalva de escopo mantida: a coluna `evo_core_mcp_servers.environments` do
CATÁLOGO continua sendo schema de chaves obrigatórias, não valor. O cofre entra
só na ponta do agente.
Refs EVO-2250
O ajuste de 2 linhas que o Reviewer pediu já estava no commit e33de81 (guard e resolvedor lendo `environments`, com tolerância a `envs` para config antiga, e o `MCPServerConfig` do schema com os dois campos). O que faltava era guardar a cadeia INTEIRA agora que as três pontas existem. Cadeia conferida no código, não suposta: 1. front 9f63077 — grava credential_refs junto de environments 2. core 62830b7 — deixa credential_refs atravessar o processamento 3. processor — resolve na montagem (mcp_service.py:399) O teste novo afirma sobre a LINHA QUE LÊ os valores, não sobre a existência da função: a primeira versão que escrevi passava mesmo com o resolvedor voltando a ler só `envs`, ou seja, não protegia nada. Corrigi e a prova negativa agora falha de verdade quando a chave regride — que é o defeito exato desta rodada. Observação sobre o ramo OAuth do core: ele monta a entrada com {id, environments, tools} e descarta credential_refs. Conferi a lista de provedores (github, notion, stripe e afins) e isso está CERTO por desenho: são conexões OAuth, cujo token vive no store dono e entra no cofre por referência (2.5), não como valor. Não é uma segunda allowlist esquecida. Refs EVO-2250
O MCPConfigDialog passou a persistir credential_refs e o core carrega o mapa pelo processMCPServers. O aviso deixava o próximo leitor concluir que a AC7 seguia aberta.
Sai o que pertencia ao relatório de review e não ao fonte: reconstrução do defeito, citação de card e severidade, referência a quem revisou. Fica o porquê não-óbvio — o mapa em vez de escalar porque uma credencial é um segredo, o allowlist de header no log, e o `environments` vs `envs` que deixava a resolução inerte. Só comentário e docstring: a AST dos 11 arquivos .py, com docstring removida, é idêntica à de antes. Os 32 testes seguem verdes.
…s-externos-cofre feat(vault): agentes externos e tools/MCPs resolvem pelo cofre (EVO-2250)
The lead says something, the agent should move the card, and nothing happens.
Not because the model refuses: the configured rules never reached the prompt.
The frontend saves PipelineRule { pipelineId, pipelineName, generalInstructions,
stages: StageRule[] } with StageRule { stageId, stageName, instructions }
(PipelineRules.tsx:17-32). The prompt builder read stageName/instructions off
the RULE, where they do not exist, so a two-stage funnel rendered as a single
line with the funnel name — no stage names, no stage ids, and none of the
per-stage "when to move" instructions the operator configured. The tool always
read rule["stages"] correctly; only the prompt side was wrong.
Since the same prompt says "do not move conversations between stages without a
matching rule", the model had nothing to match. Live check with Gemini
(gemini-3.5-flash), lead message "Fechado, pode gerar o pedido!":
before: {"stage_name": "Fechamento", "pipeline_id": "Vendas", ...}
^ stage hallucinated (real ones are Qualificado/Fechado), and the
funnel NAME sent where the id belongs
after: {"pipeline_id": "pipe-1", "stage_id": "stg-fechado", ...}
With the hallucinated args _move_to_stage looks the rule up by
pipelineId == "Vendas", finds nothing, and answers "stage_id or stage_name is
required. Available stages: none" — the card stays put and the operator sees
no reason, because the error dies inside the tool's return value.
Extracted _format_pipeline_rules_for_prompt: walks rule["stages"] and emits one
line per stage with name, id and its move-here-when instructions, plus the
funnel's generalInstructions. Legacy flat rules (stage on the rule itself)
still render, so configs saved by an older UI keep working.
Tests: 8 examples in tests/unit/test_pipeline_rules_prompt.py pinning the exact
shape the UI writes. Negative proof: restoring the old rule-level read makes 5
of them fail. Suite: 274 passed vs 266 on the clean develop baseline (+8, the
new ones); the 3 failures and 7 collection errors are identical to the baseline
and pre-existing (test_exception_handlers, mcp_headers_call_path).
…odel (CRM-237)
`conversation_id` and `contact_id` are declared as tool parameters, so the model
fills them in — and the contact UUID sits in the ContactInfo block injected into
the prompt. In a live run the model sent the CONTACT id as `conversation_id`:
Extracted contact_id from metadata: c6d3efd5-…
Adding conversation c6d3efd5-… to pipeline
CRM API response: 400 {"code":"CONVERSATION_NOT_FOUND"}
The card never moved. Note the missing "Extracted conversation_id from
metadata" line: the model had filled the field, and the tool trusted it.
The prompt already promises the opposite ("The conversation_id will be
automatically extracted from the context", llm_agent_builder), so this makes the
code keep that promise: when the metadata carries the id, it wins; the model's
argument only stands when the context is silent (a call made outside a
conversation). A mismatch is logged as a warning rather than silently dropped.
Why the first live run passed: the model chose move_to_stage and OMITTED
conversation_id, so the metadata fallback kicked in. The second chose
add_to_pipeline and filled it. Same code, outcome depending on what the model
decides to fill — fragility, not flakiness.
Verified in the running stack by reproducing the model's exact mistake:
[CRM-237] ignoring conversation_id='c6d3efd5-…' from the model;
the current conversation is 16e5207a-…
CRM API response: 200 {"success":true,...}
Tests: 7 examples in tests/unit/test_pipeline_tool_context_ids.py. Negative
proof: without the fix the regression test fails (the contact id reaches the
CRM). Suite: 281 passed vs 266 on the clean develop baseline (+15 = 8 from
CRM-235 + 7 here); the 3 failures and 7 collection errors are identical to the
baseline and pre-existing.
Code review follow-ups on the same defect, at the edges the fix left open. A stage with no stageId was still rendered as "- Stage: unnamed stage — move here when: ...". The UI creates stages with stageId="" (PipelineRules.tsx) and AgentEditPage saves pipeline_rules verbatim, so an operator who writes the criteria and forgets to pick the stage in the select produces exactly that. The model then gets a rule that looks actionable with no id and no name, and invents a stage_name — the failure this card is about. Such stages are now skipped with a warning. The legacy flat branch dropped the stageId when the rule also carried a stageName, so it rendered the same id-less line the fix removes; and a flat rule cannot be recovered by name, because the tool resolves a name through rule["stages"], which a flat rule does not have. It now emits the id, and a flat rule without one is skipped for the same reason. When the formatter renders nothing (a pipeline_rules list holding no usable rule), the prompt used to say "Configured pipeline rules:" followed by an empty list and then "do not move without a matching rule" — advertising the tool and disabling it in the same breath. It now falls back to the generic text the no-rules branch already had. Comments trimmed to what the code does not say; the bug history lives in the PR and the card. Tests: 10 examples (8 kept, 2 added for the skipped shapes, 1 renamed to pin the id). Suite 294 passed vs 284 on the clean develop baseline (+10); the 2 failures and 1 collection error are identical to the baseline and pre-existing.
…-rules-prompt fix(agent): send the pipeline rules' STAGES to the model (CRM-235)
…ncidente O relato da execucao que originou o fix (400 CONVERSATION_NOT_FOUND, bloco de ContactInfo, card parado) ja vive no card e na PR. No codigo fica so a regra: o contexto e fato, o argumento do modelo so vale com o contexto silencioso.
…ids-win fix(agent): the conversation/contact come from the context, not the model (CRM-237)
…-prompt-guidance fix(agent): tell the model when to act and stop offering ids it must not fill (CRM-238)
…ed (CRM-236) (#52) * fix(a2a): stop running an agent nobody waits for, and say why it failed (CRM-236) Two defects left over from the degraded-provider incident, both on the processor side. 1. The run outlived the caller. bot-runtime gives up at its own ceiling and closes the socket, but the processor never noticed: 04:12:47 bot-runtime -> processor 04:13:17 bot-runtime gives up, socket closed 04:18:26 processor: "Agent execution completed successfully" Five minutes of model calls with nowhere to go — burning the very quota whose exhaustion caused the timeout. run_unless_client_disconnects() now races the run against the ASGI disconnect and cancels it. Verified on the live stack: client aborted at 12:38:38.789, cancelled at 12:38:38.755, and no "completed successfully" followed. Detection deliberately uses receive(), NOT is_disconnected(). Polling is_disconnected() never fires under uvicorn once the body has been read: pause_reading() takes the socket off the selector, so connection_lost() is never called. The first version of this fix did exactly that, passed its unit tests, and still let the agent run 27s past the client abort on the live stack. Awaiting receive() calls resume_reading() and the disconnect arrives as an event. A2A_CANCEL_ON_DISCONNECT=false restores the old behaviour. 2. Every failure was a 500. standard_runner wraps everything in InternalServerError(str(e)), so a quota refusal and a NameError produced byte-identical responses and the only way to tell them apart was reading container logs. classify_provider_error() reads the cause chain and maps the recognised conditions to 429/503/502/413 with a distinct JSON-RPC code, leaving genuine bugs on 500. map_status_to_error_code() had no 429/413/499 entries, so without the catalog additions the envelope would have kept saying INTERNAL_ERROR. Recognition is conservative in both directions: our own InternalServerError name and a bare "timeout" are NOT markers — matching them would classify every bug we write as a provider outage, inverting the bug being fixed. Credentials are redacted and the detail is capped, since provider errors echo request URLs containing the key. Live verification, both paths, against the running stack: - invalid provider key -> 502 / EXTERNAL_SERVICE_ERROR / -32004, through the real double-wrapped runner chain (was 500 / INTERNAL_ERROR / -32603) - the incident's verbatim quota string -> 429 / RATE_LIMIT_EXCEEDED / -32001 - NameError still classifies as None (stays a 500) Retry parity: bot-runtime already retries 429/500/502/503/504, so moving off 500 changes nothing there. 499 and 413 are correctly not retryable. Tests: 15 provider_errors + 12 client_disconnect. Negative proof for both — with cancellation disabled the disconnect test fails AND the work runs the full 30s; without the catalog entries map_status_to_error_code(429) returns INTERNAL_ERROR. Baseline unchanged (80 passed / 3 pre-existing failures / 22 pre-existing collection errors from missing local deps). * fix(a2a): anchor classification to the provider and stop colliding with A2A codes (CRM-236 review) Addresses the two CRITICAL findings on the processor side. 1. Our own infrastructure was reported as an LLM provider outage. _status_code_of walked the whole cause chain and matched a status with no provider anchor, and status took precedence over text. The runner calls raise_for_status() against internal services (standard_runner.py:222 memory, :311 evo-kb-service), so their httpx errors entered the chain: evo-kb-service down (503) -> "The model provider is unavailable" wrong internal token (401) -> "The model provider rejected our credentials" That is the exact inversion of the bug this module exists to fix. The care had gone into the text markers and none into the status match. Classification now requires positive evidence that the exception came from a provider (module, class name, or text fingerprint). Chose an anchor over a blacklist of internal hosts: a blacklist rots — every new internal service has to be remembered, and forgetting one silently reintroduces the inversion. Refined once while testing: wording that ONLY a provider produces ("resource_exhausted", "maximum context length", "api key not valid", "model is overloaded") anchors on its own, because requiring an SDK fingerprint on top of it lost legitimate cases. Ambiguous markers ("rate limit", "too many requests", a bare 429/503) still need separate evidence — those are exactly what an internal service produces too. 2. The new JSON-RPC codes collided with this repo's own A2A catalogue. src/schemas/a2a_types.py already owns -32001 TaskNotFound, -32002 TaskNotCancelable, -32003 PushNotificationNotSupported, -32004 UnsupportedOperation and -32005 ContentTypeNotSupported, and a2a_routes.py emits them (:958, :2213, :2307). Reasoning about the reserved RANGE was not enough: an exhausted quota went on the wire as "Task not found" to any conforming A2A client — the opposite of this card's requirement. Moved to -32010..-32013, leaving -32006..-32009 free for that catalogue to grow. A test now derives the taken codes from a2a_types and asserts no intersection, so the next addition cannot collide silently. Also (MEDIUM 5): the 500 fallback is the COMMON path, because classification is conservative on purpose — so it is the path most likely to carry a credential. It emitted str(e) raw into the log and into data.error, leaking `?key=AIza…`. redact_secrets already existed in this PR; it is now applied there too. Also (MEDIUM 8): A2A_CANCEL_ON_DISCONNECT and A2A_DISCONNECT_POLL_SECONDS are documented in .env.example — the operator's escape hatches were undiscoverable. Tests: 39 (was 27). New coverage for internal-service failures at 503/502/401/ 403/429, an internal auth failure, provider SDK status without text markers, and the code-collision guard. One existing test was REWRITTEN rather than kept: it asserted that any exception carrying status_code = 429 classified as a rate limit, which is precisely the defect. It now pins that the status wins over text WITHIN a provider exception, with a counterpart asserting that a bare status on an unknown exception classifies nothing. * fix(a2a): classify and redact on the streaming route too (CRM-236 review 7) message/stream reported every failure as a generic -32603 and echoed str(e) raw, so a quota exhaustion was as opaque there as it used to be on message/send — and the raw text carries `?key=AIza…`. On the other half of that finding I reached a different conclusion, and it changes what the fix should be. The review says both defects survive on this route. The error one does. The disconnect one does NOT, and adding the guard there would have introduced a bug: sse-starlette runs _listen_for_disconnect inside cancel_on_finish, which cancels the entire task group — including _stream_response, the consumer of this generator — as soon as http.disconnect arrives. Cancellation already works here, natively. Worse, run_unless_client_disconnects awaits receive() itself. Putting it on this route would leave TWO consumers on the same receive channel, and whichever won the race would swallow the disconnect message the other was waiting for. The mechanism that makes the guard correct on message/send is exactly what makes it wrong here. Verified by reading the installed sse-starlette 3.0.2, not from memory. So this commit applies only the error half: provider classification plus redaction on the fallback. * style(comments): cut the PR narrative out of the code (CRM-236 review 14) Incident timelines, before/after tables and the review's own reasoning belong in the PR body, where they already are. Here they were a 20-line module docstring above eight imports, a 19-line block above a tuple of strings and a 13-line comment above one dataclass field. Kept is what the code cannot say: why polling is_disconnected() fails under uvicorn, why the codes sit at -3201x, why an anchor is required instead of a blacklist, and why the streaming route must not get the disconnect guard. 132 added comment lines to 88, no behaviour touched. --------- Co-authored-by: Matheus Pastorini <matheus.pastorini@etus.com.br> Co-authored-by: Guilherme Gomes <guinomotec.dev@gmail.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @gomessguii, your pull request is larger than the review limit of 150,000 diff characters
…corrente (CRM-424)
classify_provider_error tratava 429/5xx/401-403/context, mas NÃO o 404 de "modelo não
encontrado". Um agente configurado num modelo que o provider retirou (ex.: um preview
Gemini datado) caía no 500 genérico e falhava em TODO turno sem pista do motivo — a
falha silenciosa recorrente do card.
Nova categoria model_not_found (http 404, jsonrpc -32014): status 404 de um provider OU
marcadores de texto ("is not found for API version ... generateContent" do Gemini, "the
model ... does not exist" da OpenAI/Anthropic) → mensagem acionável ("modelo indisponível,
pode ter sido depreciado; escolha um modelo vigente"). Mantida ANTES do ramo auth pra um
404 nunca ler como problema de credencial. Complementa o AC1 (o catálogo já não OFERECE
modelos mortos; isto cobre agentes configurados antes da retirada).
3 testes novos (Gemini 404, OpenAI does-not-exist, status 404 ≠ auth) com red/green.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Achados do code review sobre a categoria model_not_found:
- `"does not exist"` casava com erro NOSSO: `_provider_anchored` aceita
qualquer texto que carregue nome de modelo, então um `KeyError` citando
`gpt-4.1-mini` virava falha de provider — a inversão que este módulo
existe para impedir. Ficam só as frases verbatim do provider (a cláusula
de acesso da OpenAI, as duas do Gemini e o código `model_not_found`).
- `status == 404` era avaliado ANTES de auth e de context_length, e status
vencia texto: key revogada que chega com 404 era reportada como "troque o
modelo", e estouro de contexto também. O 404 nu passa a ser o último
recurso, com mensagem que admite o api_base como causa possível.
- `http_status` 404 -> 502: a rota `POST /a2a/{agent_id}` já responde 404 +
NOT_FOUND para agente inexistente, então o 404 do provider deixava modelo
morto indistinguível de agente morto. Mesmo motivo que levou `auth` a 502.
- `"is not found for api version"` era subconjunto de `"not found for api
version"` na mesma tupla; removida.
- Testes: o guard do 500 agora tem um bug nosso citando modelo, e o teste de
precedência passa a exercitar de fato o conflito status x texto.
…ed-model-actionable-error fix(errors): modelo obsoleto vira erro acionável, não 500 recorrente (CRM-424 · AC2)
…M-464) Mesma correção do core-service, nos dois call sites que o Python tem: get_agent e get_agents_by_account repetem o reparo do agente sequential/parallel/loop sem sub_agents, e ambos atribuíam gpt-4.1-nano quando o agente não tinha modelo próprio. A OpenAI desliga esse modelo em 23/10/2026, e o reparo faz db.commit() — o id morto era gravado no agente do cliente, não apenas tentado uma vez. Passa a apontar para openai/gpt-5.6-luna, com prefixo de provider. Aqui o prefixo importa mais do que no Go: é este serviço que monta o LiteLlm, e a LiteLLM só adivinha um nome cru quando ele casa com uma família que ela já conhece, levantando BadRequestError em qualquer outro. O valor sai de uma constante de módulo para que a próxima varredura ache os dois pontos de uma vez. Modelo só é atribuído quando o agente não tem nenhum — o que o cliente já escolheu nunca é reapontado, e isso agora tem teste.
…4 review)
Das tres portas de reparo, a de listagem era a unica sem asserir persistencia —
e e justo a que o PR chama de pior ("gravado no agente do cliente so por alguem
abrir a listagem"). test_get_agents_by_account_repair_stamps_the_same_model
passa a aferir db.commit.called, como o teste de get_agent ja fazia.
test_repair_keeps_a_model_the_agent_already_has passava a vazio pelo mesmo
motivo do lado Go: sem aferir que o agente foi coagido para llm, um guard de
sub_agents que parasse de casar deixaria o modelo intocado e o teste verde.
Provado por mutacao: removendo o db.commit() do bloco de reparo de
get_agents_by_account, so o teste novo cai; anulando o guard de sub_agents nas
duas portas, os tres testes caem (antes, o de preservacao passava).
O comentario da constante cai de 6 para 2 linhas e o docstring do teste perde o
id do card — o commit e a PR ja carregam CRM-464.
…64-default-de-reparo-modelo-vigente fix(agent): o reparo carimbava um modelo com desligamento marcado (CRM-464)
…review) test_repair_keeps_a_model_the_agent_already_has passa a aferir db.commit.called, como os outros dois testes do arquivo. Aqui nao cabe a assercao de valor persistido que a PR irma ganhou — o SQLAlchemy comita o proprio objeto mutado, nao ha copia de onde os dois valores possam divergir. O que se documenta e que o reparo ESCREVE e o modelo do cliente sobrevive a escrita.
…ia-teste-preservacao test(agent): simetria com o lado Go no teste de preservação (CRM-464)
…M (CRM-499)
A custom HTTP tool whose name contains a space/accent/punctuation was sent
verbatim as tools[].function.name, which OpenAI/Anthropic reject (must match
^[a-zA-Z0-9_-]+$). The 400 surfaced as an opaque 500 and left the agent unable
to answer, with no hint of which tool was at fault.
Add sanitize_tool_name() and apply it where the ADK function name is set
(tool_builder.py, custom_tools.py). We dispatch these tools by __name__, so the
sanitized name is self-consistent; the readable label stays in the description.
This also repairs existing agents with invalid tool names, no operator action
needed.
MCP tools are intentionally NOT sanitized: their function names come from the
MCP protocol (already valid) and are the key the toolset uses to route
execution back to the server, so renaming would break the call. The seeded MCP
display names ("Brave Search", "Sequential Thinking") are only used for
routing/logging, never as function.name.
Tests: tests/unit/test_tool_name_sanitization.py (16 cases) proves every input
maps to ^[a-zA-Z0-9_-]+$, including the client's "testando ferramenta".
…e (CRM-499) Integration guard over CustomToolBuilder and ToolBuilder: a tool named "testando ferramenta" must build with __name__ == "testando_ferramenta", and a valid name is preserved. Reverting the sanitize call at either _create_http_tool site makes these fail (the raw space survives). Runs in the container/CI lane (imports google.adk), alongside the standalone sanitizer unit test.
Sanitizing __name__ made it diverge from the raw name the expanded config still
carries, and the dedup compared the two spellings. An agent holding both
custom_tool_ids and the http_tools those ids expand to therefore built the tool
twice under one function name — the same payload the provider rejects, so the
sanitization was undone by the guard it walked past.
Both builders now compare on the sanitized name, and each _create_http_tool is
handed the names already taken so two tools never answer to the same one:
distinct names can collapse alike ("minha ferramenta" and "minha_ferramenta") or
share their first 64 chars. The name is settled before FunctionTool captures it,
since the declaration is generated from the live __name__ while dispatch uses
the captured one — renaming later would split them.
sanitize_tool_name also returns a name the providers already accept untouched:
squeezing "my__tool" into "my_tool" renamed tools that work today for no gain.
A rename is logged with both spellings so the name the LLM sees can be traced
back to the catalog.
…-tool-name fix(agents): sanitize custom tool names before sending to the LLM (CRM-499)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Corte de release: leva a
developpara amain— 36 commits, 11 cards.Faz parte da cascata que leva o evolution-ecosystem para produção, parada em 2 de julho. A
maindo superprojeto aponta para SHAs que estão namainde cada submódulo, então cada repositório corta primeiro; o superprojeto bumpa os ponteiros por último.Merge testado com
git merge-treeantes de abrir: sem conflitos.Este repositório não depende dos demais dentro da cascata — pode ser mergeado em qualquer ordem em relação aos irmãos.
As migrations do conjunto foram ensaiadas contra uma cópia restaurada do banco de produção (Postgres 18.6, imagens por digest): cinco passos do
migrate-job, todos exit 0, dado intacto, idempotência confirmada.Runbook: https://claude.ai/code/artifact/3a9608d8-4126-4c9c-9934-8e0e9074fbff