fix: scope load_memory/search_memory to a reopened conversation's reset boundary - #75
Closed
mateusbellozupko wants to merge 44 commits into
Closed
mateusbellozupko wants to merge 44 commits into
mateusbellozupko wants to merge 44 commits into
Conversation
When the model called transfer_to_human with only `reason` (no explicit team_id/assignee_id — the common case, since it only knows team NAMES from the docstring, not their opaque IDs), the tool silently picked whichever transfer rule happened to be first in the configured list, regardless of the actual reason. Every escalation for an agent with multiple transfer rules was routed to rule #1 — e.g. an "Imposto de Renda" request got sent to "Dep. Contábil/Fiscal" (rule #1) instead of the dedicated IRPF team (rule evolution-foundation#4), because the code never evaluated rule conditions at all (the old comment literally said "In the future, this could be enhanced to evaluate rule conditions"). Fix: - Add a `rule_index` parameter and instruct the model (via the numbered rule list already in its docstring) to always pass it when transfer_rules are configured. - If rule_index is omitted, fall back to matching `reason` against each rule's own `instructions` text by keyword overlap. - Only if neither yields a match, fall back to the first configured rule (previous behavior), now logged as a warning so misroutes are visible. Reproduced live: an "Imposto de Renda Pessoa Física" request was transferred to "Dep. Contábil/Fiscal" instead of the dedicated team.
Conversation tagging (acts_as_taggable_on) accepts any free-form string with no relation to the account's actual Label model (Settings > Labels). The tool was letting the model tag conversations with invented label titles that never existed in that catalog — invisible in Settings, uncolored, and absent from any label-based filter. `add` now fetches the account's label catalog and only applies titles that already exist there (case-insensitive match); anything else is skipped and reported back via `rejected` instead of silently tagging the conversation with an ad-hoc string.
…rring Every transfer rule's own instructions ask the agent to notify the customer before transferring, but that depended entirely on the model also producing reply text in the same turn as its tool calls — and when the model decides to call tools, it frequently skips the customer-facing text entirely (observed live: manage_conversation_labels + transfer_to_human fired together with a null/empty assistant message, so the transfer happened completely silently). Add a message_to_customer parameter: when provided, the tool sends it as a real outgoing message BEFORE performing the team/agent assignment, so "notify then transfer" is one atomic tool action instead of two separate things the model has to remember to do in sequence. Docstring now instructs the model to always pass it when the matched rule's instructions ask for a customer notice.
- transfer_to_human: an explicit but out-of-range rule_index now returns an error instead of silently falling through to keyword matching or the first rule (hid the model's contract violation before). Keyword fallback now scores rules by count of shared meaningful, non-stopword tokens and picks the best match instead of stopping at the first rule containing any word longer than 3 chars — a single generic word like "cliente" or "para" no longer misroutes to an unrelated rule. - llm_agent_builder: the agent-level system prompt describing configured transfer rules said "the tool will automatically use the configured transfer rules, so you don't need to specify assignee_id or team_id" — directly contradicting the new rule_index requirement and explaining why models kept omitting it. Now numbers the rules (matching the tool's own docstring) and explicitly requires rule_index + message_to_customer. - manage_conversation_labels: _fetch_catalog_labels now returns (fetch_succeeded, titles) so a legitimately empty label catalog is no longer indistinguishable from a failed fetch. Also fixed the add-result message to distinguish "already present" from "rejected" instead of claiming none of the requested labels existed when some were already attached. Found via Sourcery review on the corresponding upstream PRs (evolution-foundation#55, evolution-foundation#56, evolution-foundation#57).
Builds and pushes this repo's own image to registry.bellosoft.dev on every push to main, plus workflow_dispatch for manual builds. Replaces depending on the superrepo's monolithic release.yml (kept as a manual fallback) for routine releases, so a change to one service no longer requires rebuilding all six. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Merge upstream v1.1.0 (custom tool name sanitization)
docker-publish.yml logs into Docker Hub with DOCKERHUB_USERNAME/ DOCKERHUB_TOKEN, secrets this org never configured -- it's upstream's own release pipeline for evoapicloud/*, carried over by the fork. It's been failing on every push to main with "Username and password required" since day one; harmless (bellosoft-release.yml is the real pipeline, pushing to registry.bellosoft.dev), but it's pure noise on every CI run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ublish-workflow chore(ci): remove the inherited Docker Hub publish workflow
… knowledge search call
The knowledge preload block in standard_runner.py and streaming_runner.py
referenced settings.KNOWLEDGE_SERVICE_URL, a setting that has never existed
in this codebase, so the feature silently failed and was swallowed by a
broad except Exception. Extract the logic into a standalone, testable
knowledge_preload module that calls the real CRM endpoint
(POST {EVO_AI_CRM_URL}/api/v1/internal/knowledge/search, X-Service-Token
auth) built in evo-ai-crm-community, removing ~90 lines of duplicated
inline logic from each runner.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…bled + error paths The prior test only asserted "knowledge/search" appeared in the call args, a substring shared by the old broken settings.KNOWLEDGE_SERVICE_URL reference, so it would not have caught a regression back to the broken path. Strengthen it to assert the exact CRM URL, X-Service-Token header, and JSON payload fields, and add coverage for the disabled (load_knowledge/preload_knowledge False) path and for HTTP failures propagating out of preload_knowledge so the runner's own try/except can log and continue instead of crashing the turn. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… instead of a dead URL The tool previously hand-rolled a raw httpx GET against settings.KNOWLEDGE_SERVICE_URL / KNOWLEDGE_SERVICE_API_TOKEN, neither of which exist in this codebase's settings module, so memory preload always failed. It now delegates to the already-correct memory_service.search_memory(...) (HttpMemoryService), matching how the rest of the memory pipeline (add_event_to_memory, compress_memory) is wired. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…e instead of a dead URL Task 6: replace the hand-rolled httpx call against non-existent KNOWLEDGE_SERVICE_URL/KNOWLEDGE_SERVICE_API_TOKEN settings with a call to HttpMemoryService.compress_memory (added in Task 4). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…rking memory_service The inline memory-preload block in both runners referenced settings.KNOWLEDGE_SERVICE_URL, which does not exist in this codebase's settings module, so preload silently no-opped (swallowed by a broad except). Extract the logic into a shared, tested memory_preload.py that calls the memory_service singleton's search_memory() instead, mirroring the fix already applied to preload_memory_tool.py and compress_memory_tool.py. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…uery
Preload was calling search_memory(query="") -> POST /memory/search, which
returns medium-term summaries mixed with raw short-term events, polluting the
preloaded context and defeating compression. GET /memory/load exists precisely
for this case and returns summaries only, but nothing called it.
- Add HttpMemoryService.load_memory(). http_client.do_get_json takes no
`params` argument, so the query string is urlencoded into the URL.
- Point memory_preload.preload_memory() and preload_memory_tool's
preload_memory_with_client() at load_memory, consuming plain dicts
(mem["content"] / mem.get("timestamp")) instead of pydantic MemoryEntry.
- Drop the unused `session` parameter from preload_memory().
- Replace memory_preload.py's non-standard box-drawing header with a plain
module docstring (it carried no @author/license block and its border was
misaligned).
- Fold the duplicated x-memory-base-config-id header block into
_get_headers(memory_base_config_id), removing five copies and two dead
isinstance branches whose arms were identical.
Tests run:
tests/unit/services/test_memory_service_load.py
tests/unit/services/adk/runners/test_memory_preload.py
tests/unit/services/adk/tools/test_preload_memory_tool.py
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Both runners had the preload_memory() call and its append_event inside the
same outer try whose only handler logs at debug, so a memory preload failure
was near-invisible in production AND skipped the knowledge preload that runs
after it. Restore the nested try/except with a warning-level log, as existed
before the preload block was extracted.
Also adds the missing runner-level wiring tests (no test previously covered
the runner -> preload_memory -> append_event integration point) and two minor
compress_memory_tool cleanups:
- Remove the unused `import uuid`.
- Thread the per-agent memory_medium_term_compression_interval through
create_compress_memory_tool() into memory_service.compress_memory(), which
previously always passed None. The value is available at the only call site
(llm_agent_builder), so no invented parameter is involved.
- Correct the tool docstrings to list only the statuses the code can produce
("success" | "error"). The internal endpoint reports "not enough events" and
a real failure with shape-identical dicts, so a distinct "not_ready" status
cannot be derived cleanly; the docstring now says "message" carries that
distinction instead of promising unreachable statuses.
Tests run:
tests/unit/services/adk/runners/test_runner_memory_preload_wiring.py
tests/unit/services/adk/tools/test_compress_memory_tool.py
plus the full tests/unit/ suite as a regression check
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… independently-landed knowledge_preload.py Another concurrent branch already extracted a real knowledge_preload.py module and fixed its own KNOWLEDGE_SERVICE_URL bug (commit f441570) between when this branch diverged and now. That rewrite dropped the specific "Preloading knowledge for agent X" log line the wiring test asserted on. The underlying behavior this test verifies (a memory-preload failure does not skip knowledge preload) is unchanged and still holds — only the assertion needed to catch up to the new, real implementation.
…emory namespace
HttpMemoryService.__init__ defaulted self.base_url to settings.CORE_SERVICE_URL,
which every deployment config points at the Go core service (no /memory/* routes)
instead of the Rails CRM. Every memory HTTP call (event, search, load, compress)
built off that base_url 404s in production as a result.
Default base_url is now settings.EVO_AI_CRM_URL.rstrip("/") + "/api/v1/internal",
matching the pattern already used by knowledge_preload.preload_knowledge for the
sibling /api/v1/internal/knowledge/search route, and matching the real
Api::V1::Internal::MemoryController routes on the CRM.
Explicit base_url arguments (used throughout the existing test suite) are
unaffected - only the default changes.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ring selection Keyword matching and the first-valid-rule fallback both need a rule with a usable userId/teamId for its transferTo, but the keyword-scoring loop selected purely on instructions-text overlap without that check. A misconfigured rule (e.g. transferTo: team with no teamId) could win the keyword match on its instructions alone, then leave both effective_assignee_id and effective_team_id unset — a hard "assignee_id or team_id is required" error instead of falling through to the next, actually-usable rule. Extracted the existing valid-target check (already used by the first-valid-rule fallback) into _rule_has_valid_target and applied it to the keyword-scoring loop too. An explicit rule_index still errors on its own malformed rule rather than silently substituting another — that's the model's own contract failure, not something to paper over. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ure too The customer notice and the assignment are two separate, already-committed POSTs. message_to_customer_sent/message_to_customer_error were only included in the success response — if the notice succeeded but the subsequent assignment POST failed, the error response gave no indication the customer had already been told a transfer was imminent, so a caller or model retry had no way to know not to resend the same notice. Computed once right after the notice attempt so every return path (success, assignment error, and the outer unexpected-error handler) reports the same outcome. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Switch GitHub-hosted ubuntu-latest runners to the self-hosted bellosoft runner ([self-hosted, X64, Linux, bellosoft]), matching the label already used by the working Release (Bellosoft) job. GitHub Actions billing on the org has been blocking every ubuntu-latest job (guards, tests, migration checks) while only jobs already on the self-hosted runner kept running. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… just add_to_pipeline (evolution-foundation#3) The pipeline_manipulation tool accepted a custom_fields parameter (src/services/adk/tools/evo_crm/pipeline_manipulation.py:118), but it was only wired into _add_to_pipeline (card creation, lines 370-451). The _move_to_stage action (lines 454-543) — the one used for the normal case of an ongoing conversation, per the tool's own docstring — never received or applied custom_fields at all, so there was no code path to persist a custom field on an existing card. This forced the agent to fall back to update_contact, writing the attribute onto the Contact instead of the pipeline_item (card), which breaks the intended data model: a contact can represent more than one deal over time, so per-deal attributes belong on the card, not the contact. Fix: _move_to_stage now accepts custom_fields and, when provided, resolves the real pipeline_item_id from conversation_id (same lookup pattern _create_task already used against GET /pipelines/{id}/pipeline_items, lines 584-598) and calls PATCH /pipelines/{pipeline_id}/pipeline_items/{pipeline_item_id}/update_custom_fields with {"custom_fields": {...}}, in addition to the existing move_to_stage PATCH. That resolution step is required because update_custom_fields does not accept conversation_id/display_id in place of the pipeline_item's own id. Extracted the shared lookup into _find_pipeline_item_id() and reused it in _create_task too. When custom_fields is absent/None/empty, _move_to_stage's behavior is unchanged. _add_to_pipeline is untouched — creation already worked correctly. Updated tests/unit/test_pipeline_tool_context_ids.py's _move_to_stage mock to accept the new custom_fields kwarg. Not tested against a live CRM instance (no live environment available in this sandbox); tests/unit passes in full (418 passed) against the existing suite. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…talog The "not added" branch (label already present or rejected by the catalog check) returned silently with no logger call, unlike the success and exception paths — making it impossible to tell from logs why a requested add produced no visible progress. Added a logger.info call there. Also cache the label catalog fetch (GET /labels) for 30s per tool instance, since every single `add` call re-fetched the full catalog even when it had just been fetched moments earlier by a previous add in the same agent run. Found while investigating a 2026-09-22 incident (CRM-236): a lead_qualifier turn issued 4 manage_conversation_labels calls ~8-10s apart (2 catalog fetches each on the failed attempts) before one add finally succeeded, pushing the whole agent run past the AI_CALL_TIMEOUT_SECONDS ceiling and triggering a client-disconnect cancellation mid-turn. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The use_emojis config toggle only ever injected a prompt instruction when set to true (encouraging emoji use); when false it silently added nothing, leaving emoji suppression entirely up to the agent's own free-text instruction or the model's default behavior. Operators turning the toggle off saw no effect. Now the false/unset path explicitly instructs the model not to use emojis. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
OpenRouter's default provider routing balances price and speed, which
can pick a cheaper-but-slower endpoint even when a faster one is
available for the same model. Added `extra_body={"provider": {"sort":
"latency"}}` to the LiteLLM kwargs for openrouter-routed agents —
LiteLLM's documented mechanism for forwarding OpenRouter-specific
request fields (see litellm/main.py's OpenrouterConfig handling).
Verified against OpenRouter's official TypeScript SDK
(OpenRouterTeam/ai-sdk-provider) that `provider.sort` is a real field
on the standard /chat/completions schema — as opposed to
`preferred_max_latency`, which only exists on the separate Decisions
API and would have had no effect here.
Found while investigating a 2026-09-22 incident where a single GLM
generation on GMICloud (normally our fastest/most reliable provider)
took ~104s, exceeding the agent-run timeout mid-turn.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
GET /pipelines/{id}/pipeline_items wraps its list under "data"
(`{"success": true, "data": [...], "meta": {...}}`), not "payload" —
that key belongs to a different endpoint shape (conversation labels).
Reading items_response.get("payload", []) silently returned an empty
list every time, so _find_pipeline_item_id always returned None even
when the pipeline item existed, breaking the custom_fields-on-move
fix from evolution-foundation#3 and create_task's item lookup end to end since they were
introduced — the existing test suite mocked _move_to_stage itself and
never exercised this function's real response parsing.
Confirmed live 2026-09-22: a card had already moved stage successfully
(the PATCH before this lookup) while the custom_fields patch failed
with "could not find the pipeline item to update custom fields on".
Added tests/unit/test_find_pipeline_item_id.py exercising the real
response shape end to end (would have caught this before merge).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
GLM (via OpenRouter) called manage_conversation_labels with labels='["qualificado_ntba", "reuniao_confirmada", "etapa_concluida"]' — a single string, not a real JSON array. _coerce_input_list treated the whole bracketed string as one literal label, which never matched the catalog and was silently rejected. This surfaced to the model (and then the user, via its own private note) as "these labels don't exist" — a hallucinated explanation for what was actually a malformed tool argument, never a real catalog lookup. _coerce_input_list now detects a string shaped like a JSON array and parses it back into a real list before doing anything else. Falls back to the previous literal-string behavior if the bracketed string isn't valid JSON, so a genuinely odd label name doesn't silently vanish either. Found while investigating a 2026-09-22 conversation where qualified labels were never applied to the conversation. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…(CRM-572) TenantContextMiddleware treated every exception from current_context_id as a resolution failure and ran the request unbound, so a refusal the extension point decided on (a 403) surfaced downstream as a 500 or as an empty read. The class moves to src/middleware/tenant_context.py so it can be tested without importing the app.
Trivial lint cleanup on the just-cherry-picked CRM-572 fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
O processor entrava em crash-loop no boot com `DuplicateTableError: relation "evo_agent_processor_execution_metrics" already exists` durante o `alembic upgrade head` da chain community — antes de chegar no create_all e na chain do licensing. Causa: a unica migration (down_revision=None) fazia um CREATE TABLE cru sem guard. Quando a tabela ja existe fisicamente (criada por um run anterior, ou pelo Base.metadata.create_all de uma imagem mais antiga) mas o alembic_version desta alembic esta vazio, o upgrade re-roda do zero e colide. Fix: guard `to_regclass` em upgrade (pula o CREATE se a tabela existe; a revisao ainda e' stampada, reconciliando o drift) e simetria no downgrade. Prova RED/GREEN contra Postgres real (banco driftado: tabela presente + alembic_version vazio): sem o fix, `alembic upgrade head` estoura DuplicateTable; com o fix, roda como no-op e registra a versao. Unit test novo (roda na lane unit da CI, sem DB) cobre o guard via mock do bind — falha sem o fix. Debito conhecido (follow-up, fora deste escopo): divergencia de nomes — o model ExecutionMetrics usa `evo_ai_agent_processor_execution_metrics` enquanto esta migration e o usage_reporter usam `evo_agent_processor_ execution_metrics` (rename feito so no model). Reconciliar exige decisao sobre dados legados.
Review follow-ups on the guard: - The skip was silent, so an operator could not tell a stamped revision from an applied one. Log it on alembic's own logger, next to the "Running upgrade" lines in the boot log. - Drop the unreferenced module-level sa.Table block: it re-hardcoded the literal the new TABLE constant exists to hold. - Trim the upgrade and test docstrings to what the code cannot say. - Cover the fourth branch: downgrade drops when the table is present. - usage_reporter's docstring named evo_agent_processor_execution_metrics, but execution metrics are persisted through the ExecutionMetrics model into evo_ai_agent_processor_execution_metrics; the file holds no SQL.
…volution-foundation#66) Old custom tools stored each body param as a plain string instead of the {type, required, description} schema. Building the tool docstring called .get on that string and raised AttributeError, breaking tool creation for both CustomToolBuilder and ToolBuilder. Add _normalize_body_param to coerce a string body param into a required string schema whose description is the string, and route both docstring loops through it. A string-shaped body param no longer crashes the build and its param is still exposed to the LLM. Claude-Session: https://claude.ai/code/session_018zYu3wd6KQkuG76B9qTXSA
The ADK builds the tool declaration the model sees from the function signature, never from the docstring. http_tool(**kwargs) therefore reached the LLM as a tool taking no arguments: it called with none, body_data came out empty and the endpoint answered 400 body cannot be empty. That is the symptom CRM-527 reports, and it survived the docstring fix. Synthesize the signature from body_params before handing the function to FunctionTool, in both builders. A required param is declared as its type, an optional one as Optional[T] with a default — a bare None default makes the ADK drop the required list and every param turns optional. A name that is not a Python identifier stays docstring-only instead of breaking the build. Also drops the underscore from normalize_body_param, which tool_builder already imports across module boundaries. Claude-Session: https://claude.ai/code/session_019CSZZDzLqvHC19nybzcHtV
get_agent/get_agents_by_account sanitizavam o nome do agente com um guard inline
baseado em str.isalnum() -- que e True para acentuados ("cafe".isalnum() -> True,
com acento). O acento passava, e quando o agente e anexado como tool de outro
(llm_agent_builder -> AgentTool) o nome vira tools[].function.name; o provedor
exige ^[a-zA-Z0-9_-]+$ -> 400 -> 500 opaco no chat (mesma familia do CRM-499).
Reusa o helper que o CRM-499 deixou no mesmo repo (src/utils/tool_naming.py:
sanitize_tool_name -- coage p/ ^[a-zA-Z0-9_-]+, cap 64, passthrough p/ nomes ja
validos, fallback "tool"). Preserva o db.commit() so quando o nome muda.
Exposicao: cadastro pela UI passa por sanitizeAgentName no front (e o a2a tem o
sanitizeName em Go); o furo e o agente criado por API/seed/import, onde o
fallback do backend era a unica barreira -- e ela deixava o acento passar.
QA:
- prova standalone: o sanitizador velho produz 'Cafe_Acao' com acento
(invalido); o helper produz 'Caf_A_o' (valido). "cafe(acento)".isalnum() True.
- teste novo tests/unit/test_agent_name_sanitization.py (roda no ci.yml =
pytest tests/unit, py3.11): nome acentuado via get_agent/get_agents_by_account
-> function.name valido + persiste; nome ja valido fica intacto.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JpWaS9MUbLhMK8QGy3GKML
…m (CRM-501) Sourcery apontou 2 furos reais: 1. nome vazio/None escapava (guard `if agent.name`) e continuava invalido p/ tools[].function.name. Agora chama sanitize_tool_name incondicional -- o fallback "tool" cobre vazio/None. Persiste so quando muda. 2. o passthrough do helper usava `^...$`, e em Python `$` casa ANTES de um `\n` final -> "valid\n" passava direto (ainda invalido). Troca p/ `\A...\Z` (ancora absoluta) no _VALID_NAME. Corrige tambem o CRM-499 (custom tools), que compartilha o helper. Testes: os asserts passam a usar .fullmatch (.match tinha o mesmo furo do `$`); +2 casos -- nome vazio -> "tool" (persiste); "valid\n" -> "valid". Prova standalone reconfere todos os casos. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JpWaS9MUbLhMK8QGy3GKML
…tion.name (CRM-501) sanitize_tool_name keeps "-", because the providers accept it in tools[].function.name. An agent name goes through the ADK first, and google-adk's BaseAgent validates name.isidentifier() -- "-" fails it. So routing agent names through the tool helper traded the accent 400 for an ADK ValidationError on names that used to work: the old inline sanitizer turned "-" into "_", the helper does not. It is the everyday path, not an import edge: the UI's own sanitizeAgentName keeps "-" on purpose, so "Suporte - N1" is stored as "suporte_-_n1". Proved against google-adk 1.19.0: 'suporte_-_n1' old-> 'suporte___n1' OK | new-> 'suporte_-_n1' REJECTED 'brave-search' old-> 'brave_search' OK | new-> 'brave-search' REJECTED Every agent type is affected, not just llm: LlmAgent, A2ACustomAgent, WorkflowAgent and TaskAgent all extend BaseAgent. Adds sanitize_agent_name next to sanitize_tool_name -- the intersection of the two contracts: the provider alphabet plus a leading-digit prefix and "-" folded to "_", so the result is also an identifier. A name the ADK already accepts still comes back untouched. Kept in the same module (a rename would conflict with the open PRs on custom_tools.py/tool_builder.py); its docstring now describes both consumers instead of claiming custom HTTP tools are the only one. Also logs the rename. get_agent rewrites the name in the database, and the 64-char cap made that reach names that were previously left alone, so the log is the only trace the user has of a rename they cannot undo. Tests: the assertions now build a real LlmAgent instead of restating our own regex -- that restatement is what let the hyphen through. New cases for the hyphen, the leading digit and the over-64 truncation, plus a standalone contract test for sanitize_agent_name.
…d via a task-local ContextVar Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…dary Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
add_event_to_memory and compress_memory now fall back to the same task-scoped min_timestamp as load_memory/search_memory, closing the gap where the write-side compression trigger could fold pre-reset events into a fresh summary that then passed the read-side filter under its own (now) created_at. The Rails-side compression scoping (already merged) is the enforcement point; this lets it see the same boundary via the request the processor already sends. Found in code review of the EVO-2241 fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Adds an asyncio.gather-based isolation test against the real ContextVar-reading load_memory (not a mock of preload_memory), so the contextvars.ContextVar design choice - the property that makes it safe to scope the ADK-owned load_memory tool via a shared HttpMemoryService singleton - is actually verified rather than only argued for in a comment. Found in code review of the EVO-2241 fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @mateusbellozupko, your pull request is larger than the review limit of 150,000 diff characters
Author
|
Closing: this fix depends on the ai_session_epoch mechanism (EVO-2241), which only exists in the Bellosoft-Limited fork and not in evolution-foundation. Re-targeting this fix at Bellosoft-Limited/evo-ai-processor-community main instead. |
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
3 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.
Summary
preload_memory(run automatically every turn) and the ADK-owned, model-invokedload_memorytool both callHttpMemoryService.load_memory/search_memory, keyed only byapp_name/user_id- with no awareness of the CRM'sai_session_epochreset boundary, so a reopened-after-resolved conversation still had pre-resolution memory re-injected even though its ADK session is correctly empty.HttpMemoryService.load_memory/search_memory/add_event_to_memory/compress_memorynow accept an optionalmin_timestamp, falling back to a task-localcontextvars.ContextVar(set_memory_min_timestamp) when not passed explicitly - this is what scopes the on-demandload_memorytool without modifying that library-provided tool.standard_runner.py/streaming_runner.pyset that ContextVar once per turn from the request metadata'smemoryMinTimestamp(sent by the CRM PR above), before any memory read or write happens in the turn.add_event_to_memory/compress_memoryalso thread the floor through, so the write-side compression trigger can't fold pre-reset events into a fresh summary and have it slip past the read-side filter (the CRM PR's compression-scoping fix is the enforcement point; this lets it see the boundary).Test plan
pytest tests/unit/services/test_memory_service_load.py tests/unit/services/test_memory_service_search.py tests/unit/services/test_memory_service_compress.py tests/unit/services/test_memory_service_add_event.pypytest tests/unit/services/adk/runners/test_runner_memory_preload_wiring.py(incl. anasyncio.gather-based test proving the ContextVar does not leak between two concurrently running turns)pytest tests/unit/suite green (492 passed; one pre-existing, unrelatedtest_exception_handlers.pyimport error predates this branch and is excluded)🤖 Generated with Claude Code