chore: drop stale diagnostic report from PR #77021
teknium1 flagged ISSUE_76870_RELATORIO_CAUSA_RAIZ.md as containing stale metadata (references an unrelated local branch) and asked to drop the standalone report, keeping only the focused server.py fix and regression test.
This commit is contained in:
joaomarcos
kshitij
parent
a93969dbd1
commit
3b53a3c560
|
|
@ -1,126 +0,0 @@
|
|||
# Relatório de validação e causa raiz — Issue #76870
|
||||
|
||||
## Identificação
|
||||
|
||||
- Issue: [#76870 — Model switch mid-session triggers history_version mismatch](https://github.com/NousResearch/hermes-agent/issues/76870)
|
||||
- Estado consultado em 2026-08-02: aberta, sem responsável, sem comentários.
|
||||
- Labels: `type/bug`, `comp/tui`, `P1`, `sweeper:risk-session-state`, `area/sessions`.
|
||||
- Superfície afetada: `tui_gateway`, usada por TUI/Desktop/Webapp via JSON-RPC.
|
||||
- Branch local analisada: `fix-76574-secret-scope-residuals`.
|
||||
|
||||
## Veredito
|
||||
|
||||
**Issue verídica. Implementação necessária e realizada.**
|
||||
|
||||
Foi reproduzido deterministicamente o defeito causal: uma troca escolhida enquanto um turno está rodando fica em `pending_model_switch`; no início do turno seguinte, o gateway capturava `history` e `history_version` antes de aplicar essa troca. A aplicação da troca adiciona o marcador de modelo ao histórico e incrementa a versão. Ao terminar, o próprio turno é classificado incorretamente como concorrente/stale e seu resultado não é gravado no histórico da sessão.
|
||||
|
||||
A evidência local confirma o primeiro turno após a troca. O relato de 75+ mensagens vazias no SQLite é evidência operacional fornecida pelo autor; esse volume completo não foi reproduzido localmente sem o banco e ambiente Docker citados. A correção elimina o desync inicial que inicia a cadeia, sem relaxar o guard defensivo.
|
||||
|
||||
## Evidência da issue
|
||||
|
||||
O autor relata:
|
||||
|
||||
- Docker com s6-overlay, perfil `default`.
|
||||
- Troca `minimax-m3` → `deepseek-v4-pro`, provider `ollama-cloud`.
|
||||
- Sessão `20260802_151208_b9d649`.
|
||||
- Log: `history_version mismatch (expected=1 current=2)`.
|
||||
- Respostas visíveis pelo stream WebSocket, mas histórico/DB inutilizável para busca, resume, rewind, undo e cron `context_from`.
|
||||
- Workaround: executar `/new` antes da troca.
|
||||
|
||||
## Causa raiz confirmada
|
||||
|
||||
Sequência defeituosa anterior:
|
||||
|
||||
1. `config.set model` durante turno ativo não troca cliente em uso; salva escolha em `session["pending_model_switch"]`. Isso é correto e evita race entre modelo/client/base URL.
|
||||
2. `_run_prompt_submit()` do próximo turno copiava `session["history"]` e `history_version` no dispatcher, antes de iniciar worker.
|
||||
3. Worker chamava `_apply_pending_model_switch()`.
|
||||
4. `_apply_model_switch()` chamava `_append_model_switch_marker()`.
|
||||
5. Marcador era anexado e `history_version` incrementava.
|
||||
6. `agent.run_conversation()` recebia snapshot antigo, sem marcador.
|
||||
7. Commit final comparava versão antiga com nova, concluía falsamente que histórico mudou externamente e recusava `result["messages"]`.
|
||||
|
||||
Defeito nasceu no commit `f27d45e2880b46a2239b184ecc8ab88ecfd2843d` (`feat(tui): ... switch mid-turn`, 2026-07-30). Intenção do commit era válida; erro foi somente ordenação entre aplicação adiada e snapshot.
|
||||
|
||||
## Solução implementada
|
||||
|
||||
Arquivo: `tui_gateway/server.py`.
|
||||
|
||||
Alteração: mover captura de `history` e `history_version` para dentro do worker, imediatamente após `_apply_pending_model_switch()` e `_sync_agent_model_with_config()`, ainda protegida por `history_lock` e antes de qualquer chamada ao modelo.
|
||||
|
||||
Nova ordem:
|
||||
|
||||
1. Aplicar mutações preparatórias pertencentes ao próprio turno.
|
||||
2. Capturar histórico e versão consistentes.
|
||||
3. Executar agente com esse snapshot.
|
||||
4. Manter comparação final de versão.
|
||||
|
||||
Assim, mutação interna esperada entra no baseline. Mutações externas reais ocorridas após snapshot continuam detectadas e rejeitadas pelo guard existente.
|
||||
|
||||
### Por que não aceitar qualquer mismatch
|
||||
|
||||
A alternativa sugerida pela issue de aceitar resposta mesmo com versão alterada seria insegura: undo, compressão, retry, rollback ou outra escrita concorrente podem invalidar contexto usado para gerar resposta. Sobrescrever histórico nesses casos ressuscitaria mensagens removidas ou perderia estado novo. Guard permanece intacto.
|
||||
|
||||
### Por que não remover marcador ou incremento
|
||||
|
||||
Marcador informa novo runtime ao modelo e precisa sobreviver em histórico. Incremento representa mutação real. Removê-los esconderia mudança e enfraqueceria invariantes. Problema era snapshot cedo demais.
|
||||
|
||||
### Escopo
|
||||
|
||||
- 7 linhas produtivas reposicionadas/adicionadas; nenhuma API nova.
|
||||
- Nenhuma mudança de schema, tool, config ou prompt global.
|
||||
- Sem alteração em persistência SQLite.
|
||||
- Sem atualização de Graphify, conforme instrução do projeto.
|
||||
|
||||
## Teste de regressão
|
||||
|
||||
Adicionado `test_prompt_submit_snapshots_history_after_pending_model_switch` em `tests/test_tui_gateway_server.py`.
|
||||
|
||||
O teste simula contrato real:
|
||||
|
||||
- sessão começa com versão 0;
|
||||
- troca adiada adiciona marcador e eleva versão para 1 no início do turno;
|
||||
- agente deve receber marcador no `conversation_history`;
|
||||
- resposta deve terminar no histórico;
|
||||
- `message.complete` não deve carregar warning de mismatch.
|
||||
|
||||
### Prova antes da correção
|
||||
|
||||
Resultado esperado e observado:
|
||||
|
||||
- teste falhou;
|
||||
- agente recebeu `conversation_history=[]`, não o marcador;
|
||||
- stderr: `history_version mismatch (expected=0 current=1)`;
|
||||
- histórico recusou resposta do agente.
|
||||
|
||||
### Prova depois da correção
|
||||
|
||||
- `pytest ... -k "history_version or pending_model_switch"`: **5 passed**.
|
||||
- `pytest ... -k "model_switch"`: **7 passed**.
|
||||
- `ruff check tui_gateway/server.py tests/test_tui_gateway_server.py`: **passou**.
|
||||
- `git diff --check`: **passou**.
|
||||
|
||||
Suite completa de `tests/test_tui_gateway_server.py`: **501 passed, 2 failed** na primeira execução. Falhas foram testes concorrentes não relacionados (`test_write_json_serializes_concurrent_writes` e `test_run_prompt_submit_requeues_all_unstarted_notifications_with_real_threading`). Ambos passaram juntos ao rerodar isoladamente (**2 passed**), classificando-os como flakes/interferência de suite, não regressão desta mudança.
|
||||
|
||||
## Invariantes preservados
|
||||
|
||||
- Prompt caching: nenhuma mensagem passada é reescrita durante chamada; snapshot continua estável para o turno.
|
||||
- Alternância/histórico: marcador existente continua sendo mensagem `user`, conforme compatibilidade com providers estritos.
|
||||
- Concorrência: `history_lock` cobre captura; guard de versão continua protegendo mutações posteriores.
|
||||
- Escopo por sessão: nenhuma variável global ou ambiente foi alterada.
|
||||
- Persistência: agente e gateway passam a compartilhar o mesmo baseline que contém marcador.
|
||||
|
||||
## Risco residual
|
||||
|
||||
Baixo. Mudança só desloca momento do snapshot alguns milissegundos para depois da preparação do turno. Se uma mutação externa legítima ocorrer depois da nova captura, mismatch ainda dispara. Se ocorrer antes, ela é incluída no contexto enviado ao agente, comportamento correto porque chamada ainda não começou.
|
||||
|
||||
A alegação de que todo turno posterior necessariamente falha não decorre isoladamente do contador: após consumir `pending_model_switch`, nova versão deveria estabilizar. Isso pode refletir efeito secundário do desync inicial ou particularidade do ambiente/banco do relator. Correção cobre causa confirmada sem alegar reprodução do banco original.
|
||||
|
||||
## Arquivos alterados
|
||||
|
||||
- `tui_gateway/server.py`: ordem correta do snapshot.
|
||||
- `tests/test_tui_gateway_server.py`: regressão causal.
|
||||
- `ISSUE_76870_RELATORIO_CAUSA_RAIZ.md`: este relatório.
|
||||
|
||||
## Conclusão
|
||||
|
||||
Não cancelar: bug existe no código atual e foi reproduzido. Correção mínima age na fronteira exata: operações preparatórias primeiro, snapshot depois. Não adiciona abstração, não enfraquece proteção anti-stale e preserva intenção da troca adiada.
|
||||
|
|
@ -1,218 +0,0 @@
|
|||
# Relatório técnico — Issue #69678 e PRs relacionados
|
||||
|
||||
**Data da análise:** 22 de julho de 2026
|
||||
**Repositório:** `NousResearch/hermes-agent`
|
||||
**Issue principal:** [#69678 — SQLite connections leaked in delivery, async delegation, and verification evidence ledgers](https://github.com/NousResearch/hermes-agent/issues/69678)
|
||||
**PR principal:** [#69681 — fix(gateway,tools,agent): close leaked SQLite connections in delivery](https://github.com/NousResearch/hermes-agent/pull/69681)
|
||||
|
||||
## Resumo executivo
|
||||
|
||||
A issue #69678 descreve um bug real: três ledgers SQLite usam a conexão como context manager, mas nunca a fecham explicitamente. Em processos de gateway de longa duração, as conexões e seus descritores de arquivo podem permanecer vivos até a coleta pelo garbage collector, acumulando descritores para o banco principal, `-wal` e `-shm`. O processo eventualmente pode atingir `RLIMIT_NOFILE` e começar a falhar com `[Errno 24] Too many open files` em componentes não relacionados.
|
||||
|
||||
A causa raiz apresentada está correta. O PR #69681 corrige os 21 call sites identificados e preserva as semânticas existentes de transação e locking.
|
||||
|
||||
**Conclusão:** #69678 não é duplicata exata de #69567. Ambas pertencem à mesma classe de bug, mas afetam módulos diferentes. O PR #69681 deve ser tratado como fix irmão do PR #69594, não como implementação duplicada.
|
||||
|
||||
## Escopo afetado
|
||||
|
||||
| Módulo | Call sites afetados | Operações que acionam o ledger | Risco |
|
||||
|---|---:|---|---|
|
||||
| `gateway/delivery_ledger.py` | 5 | Registro, atualização, recuperação, pruning e inspeção de entregas | Muito alto; executado no fluxo frequente de respostas finais |
|
||||
| `tools/async_delegation.py` | 13 | Dispatch, conclusão, recuperação, claim, release e confirmação de entrega | Alto durante delegações em background |
|
||||
| `agent/verification_evidence.py` | 3 | Resultado de terminal, edição do workspace e leitura de status | Cresce com operações de desenvolvimento e verificação |
|
||||
|
||||
Total confirmado: **21 call sites**.
|
||||
|
||||
## Causa raiz
|
||||
|
||||
Os módulos usam o seguinte padrão:
|
||||
|
||||
```python
|
||||
with _connect() as conn:
|
||||
...
|
||||
```
|
||||
|
||||
O context manager de `sqlite3.Connection` controla a transação:
|
||||
|
||||
- sucesso: commit;
|
||||
- exceção: rollback;
|
||||
- saída do bloco: não executa `conn.close()`.
|
||||
|
||||
Consequentemente, o bloco `with` transmite uma falsa impressão de gerenciamento completo do recurso. A transação termina, mas o lifecycle da conexão não termina de forma determinística.
|
||||
|
||||
Em modo WAL, uma conexão pode manter descritores associados a:
|
||||
|
||||
- arquivo principal do banco;
|
||||
- arquivo `-wal`;
|
||||
- arquivo `-shm`.
|
||||
|
||||
Em processo curto, encerramento ou coleta rápida pode mascarar o defeito. Em gateway long-lived, sob tráfego recorrente, acúmulo pode alcançar o soft limit de descritores e causar falhas em leituras de configuração, arquivos temporários, sockets e outros bancos SQLite.
|
||||
|
||||
## Relação com #69567 e PR #69594
|
||||
|
||||
[Issue #69567](https://github.com/NousResearch/hermes-agent/issues/69567) encontrou a mesma falha em `cron/executions.py`. Uma execução normal de cron abre conexões em `create_execution()`, `mark_execution_running()` e `finish_execution()`. O relato mediu crescimento de descritores até atingir limite do processo.
|
||||
|
||||
[PR #69594](https://github.com/NousResearch/hermes-agent/pull/69594) propõe um `_transaction()` que:
|
||||
|
||||
1. abre a conexão;
|
||||
2. preserva commit/rollback com `with conn:`;
|
||||
3. fecha a conexão em `finally`;
|
||||
4. fecha também quando inicialização de PRAGMA/schema falha.
|
||||
|
||||
O PR #69681 aplica o mesmo modelo a três ledgers não alterados pelo PR #69594.
|
||||
|
||||
### Decisão sobre duplicidade
|
||||
|
||||
**Não marcar #69678 como duplicata de #69567.**
|
||||
|
||||
Justificativa:
|
||||
|
||||
- causa técnica idêntica;
|
||||
- arquivos e call paths diferentes;
|
||||
- #69594 altera somente ledger de cron;
|
||||
- mesmo após #69594, os 21 sites de #69678 continuariam vazando conexões;
|
||||
- busca de PRs relacionados não encontrou outro PR cobrindo os três módulos de #69678.
|
||||
|
||||
Classificação correta: issues irmãs pertencentes à mesma classe de defeito.
|
||||
|
||||
## Avaliação do PR #69681
|
||||
|
||||
### Estado observado
|
||||
|
||||
- aberto;
|
||||
- não é draft;
|
||||
- GitHub o considera mergeable;
|
||||
- 1 commit;
|
||||
- 6 arquivos alterados;
|
||||
- 512 adições e 29 remoções;
|
||||
- sem reviews ou comentários no momento da análise.
|
||||
|
||||
### Solução implementada
|
||||
|
||||
Cada módulo recebe um context manager equivalente a:
|
||||
|
||||
```python
|
||||
@contextmanager
|
||||
def _transaction() -> Iterator[sqlite3.Connection]:
|
||||
conn = _connect()
|
||||
try:
|
||||
with conn:
|
||||
yield conn
|
||||
finally:
|
||||
conn.close()
|
||||
```
|
||||
|
||||
Além disso, `_connect()` passa a fechar a conexão caso PRAGMA ou inicialização de schema falhe depois de `sqlite3.connect()` ter retornado com sucesso.
|
||||
|
||||
### Pontos corretos
|
||||
|
||||
- fechamento determinístico em sucesso, early return e exceção;
|
||||
- commit e rollback continuam delegados ao context manager nativo;
|
||||
- locking existente é preservado;
|
||||
- `_transaction()` não adquire `_DB_LOCK`, evitando lock nesting novo;
|
||||
- `_prune()` em `delivery_ledger` continua lock-free;
|
||||
- contrato schema-on-connect é mantido;
|
||||
- todos os 21 call sites são migrados;
|
||||
- nenhuma configuração, schema de ferramenta ou superfície core nova é adicionada.
|
||||
|
||||
Nenhum defeito funcional foi identificado no patch analisado.
|
||||
|
||||
## Minimal fix recomendado
|
||||
|
||||
Solução do PR é pequena no comportamento de produção e resolve a causa raiz. Recomendação: manter `_transaction()` local em cada módulo.
|
||||
|
||||
Não criar agora um helper SQLite global compartilhado. Isso aumentaria escopo, acoplamento e risco para resolver três módulos independentes. Uma abstração compartilhada só deve surgir após demanda concreta e contrato comum comprovado.
|
||||
|
||||
Possíveis reduções sem mudar o desenho:
|
||||
|
||||
- encurtar docstrings repetidas dos três `_transaction()`;
|
||||
- compartilhar fixture de tracking apenas se já existir local apropriado na suíte;
|
||||
- evitar refatorações adjacentes.
|
||||
|
||||
O volume `+512/-29` vem principalmente dos três arquivos de regressão. O fix runtime em si permanece cirúrgico.
|
||||
|
||||
## Avaliação dos testes
|
||||
|
||||
O PR adiciona testes para:
|
||||
|
||||
- fechamento em operações normais;
|
||||
- update sem linha correspondente;
|
||||
- exceção durante operação SQL;
|
||||
- falha durante inicialização do schema;
|
||||
- igualdade entre número de conexões abertas e fechadas.
|
||||
|
||||
Os testes usam conexões SQLite reais envolvidas por um proxy que registra chamadas a `close()`. Isso valida diretamente o contrato quebrado e evita depender do timing do garbage collector.
|
||||
|
||||
### Melhoria opcional
|
||||
|
||||
Adicionar teste Linux de integração contando `/proc/self/fd` após várias operações. Esse teste reproduziria o sintoma externo, mas pode ser específico de plataforma e mais frágil. Não deve bloquear merge se a suíte direta de lifecycle e suítes existentes estiverem verdes.
|
||||
|
||||
Resultados citados pelo autor não foram reexecutados nesta análise, pois checkout local contém várias alterações pré-existentes e branch do PR não foi aplicada. Antes do merge, CI deve confirmar:
|
||||
|
||||
```text
|
||||
tests/gateway/test_delivery_ledger_fd_leak.py
|
||||
tests/tools/test_async_delegation_fd_leak.py
|
||||
tests/agent/test_verification_evidence_fd_leak.py
|
||||
tests/gateway/test_delivery_ledger.py
|
||||
tests/gateway/test_delivery_ledger_producer.py
|
||||
tests/tools/test_async_delegation.py
|
||||
tests/agent/test_verification_evidence.py
|
||||
```
|
||||
|
||||
## Issues relacionadas
|
||||
|
||||
Mesma classe geral de lifecycle SQLite, mas escopos diferentes:
|
||||
|
||||
- [#69567](https://github.com/NousResearch/hermes-agent/issues/69567): cron execution ledger;
|
||||
- [#60859](https://github.com/NousResearch/hermes-agent/issues/60859): leak de `SessionDB` em early return;
|
||||
- [#30027](https://github.com/NousResearch/hermes-agent/issues/30027): listagem de boards kanban;
|
||||
- [#28802](https://github.com/NousResearch/hermes-agent/issues/28802): helpers kanban specify;
|
||||
- [#36111](https://github.com/NousResearch/hermes-agent/issues/36111): lifecycle de `ResponseStore` no API server;
|
||||
- [#37369](https://github.com/NousResearch/hermes-agent/issues/37369): crescimento de descritores de `response_store.db`.
|
||||
|
||||
Essas issues demonstram padrão recorrente: uso de context manager transacional interpretado incorretamente como gerenciamento completo da conexão.
|
||||
|
||||
## Possíveis ocorrências residuais
|
||||
|
||||
Busca estática encontrou padrões semelhantes fora do escopo do PR, incluindo:
|
||||
|
||||
- `gateway/readiness.py`;
|
||||
- templates FastMCP em `optional-skills/mcp/fastmcp/templates/database_server.py`.
|
||||
|
||||
Esses locais não devem ser incluídos automaticamente em #69681. Cada ocorrência precisa de:
|
||||
|
||||
1. confirmação de que conexão não possui outro owner;
|
||||
2. reprodução ou teste de lifecycle;
|
||||
3. análise da frequência e duração do processo;
|
||||
4. fix separado quando comportamento estiver comprovado.
|
||||
|
||||
Expandir #69681 para uma auditoria global contrariaria objetivo de mudança cirúrgica.
|
||||
|
||||
## Solução estrutural futura
|
||||
|
||||
Após merge dos fixes urgentes, abrir tarefa separada de auditoria dirigida:
|
||||
|
||||
1. localizar `with sqlite3.connect(...)`, `with connect(...)` e `with _connect(...)`;
|
||||
2. classificar conexões por lifecycle: per-operation ou long-lived;
|
||||
3. exigir `contextlib.closing`, `try/finally close()` ou helper transacional para conexões per-operation;
|
||||
4. adicionar teste de regressão somente para call paths reais;
|
||||
5. documentar em guia interno que `sqlite3.Connection` context manager não fecha a conexão.
|
||||
|
||||
Evitar mudança mecânica global: alguns componentes podem manter conexão deliberadamente durante lifetime do serviço.
|
||||
|
||||
## Recomendação final
|
||||
|
||||
1. Não fechar #69678 como duplicata.
|
||||
2. Tratar #69594 e #69681 como fixes irmãos.
|
||||
3. Aprovar desenho do PR #69681 após CI verde.
|
||||
4. Não ampliar PR para ocorrências não reproduzidas.
|
||||
5. Criar follow-up separado para auditoria de lifecycle SQLite no repositório.
|
||||
|
||||
**Decisão sugerida:** merge do PR #69681 após validação automática e review, seguido pelo fechamento da issue #69678 como concluída.
|
||||
|
||||
---
|
||||
|
||||
## 📊 Infográfico: Vazamento de FDs e Correção
|
||||
|
||||
|
||||
|
||||
Loading…
Reference in New Issue