upstream feedback aberto: #1106 (overflow real, CodeRabbit) e #1107 (overlap com #1082, mantenedor humano) #8

Activity

1. FabioLeitao commented on Oct 3, 2026

   

   FabioLeitao

   on Oct 3, 2026

   OwnerAuthor

   More actions

   Correção — existe REVIEW real, não só comentário de issue (eu verifiquei errado antes)

   Erro meu: só tinha checado gh pr view 1106 --json comments (Issue Comments API), que NUNCA
   mostra Reviews. Reviews são outro objeto (gh api repos/Twigpine/zero/pulls/1106/reviews).
   Confirmado agora, texto exato, sem paráfrase:

   Review real: Twigpine#1106 (review)
   Autor: Vasanthdev2004 (colaborador, não o dono do repo)
   Estado: CHANGES_REQUESTED
   Data: 2026-10-02T15:24:26Z
   Commit revisado: 4710959 (ANTES de qualquer fix nosso — nem o 2e820b38 nem o e3a3f237
   foram vistos por esse review ainda)

   Corpo do review, verbatim:

   Thanks for picking this up. The env parsing matches ResolveStreamIdleTimeout line for line,
   the default stays at 120s, and the wiring works end to end: with
   ZERO_RESPONSE_HEADER_TIMEOUT=300s in the environment, the shared transport's
   ResponseHeaderTimeout comes out at 5m. Two test gaps before it goes in:

   • Nothing pins the transport to the resolver. TestResolveResponseHeaderTimeout calls the
     resolver directly, so putting DefaultResponseHeaderTimeout back on the transport still
     passes the whole package.
   • TestHTTPClientReturnsStallHardenedSharedClient now depends on the developer's shell.
     sharedHTTPClient is built at package init, so with ZERO_RESPONSE_HEADER_TIMEOUT=300s set
     it fails with "ResponseHeaderTimeout = 5m0s, want 120s". On main it passes with the same
     variable set. The people most likely to have it set are the ones this PR is for.

   One change covers both: build the client in a function the package var calls, and test that
   function under t.Setenv, once unset (120s) and once with an override. Having it return the
   idle closer's stop function lets the test clean up after itself. The existing test can then
   stop asserting on the init-time value.

   CodeRabbit's overflow note is real, but the idle resolver on main has the same bare-seconds
   multiply. If you take it, one parse helper that both resolvers call would keep them identical.

   CI hadn't run: both runs were waiting behind the fork gate, and I approved them after reading
   the diff. Requesting changes for the two tests.

   Checklist de entrega (pra verificar contra isso, não contra memória)

   • Helper compartilhado do overflow (bareSecondsDuration) — ele mesmo marcou como opcional,
     já feito em e3a3f237.
   • Gap 1: teste que force o TRANSPORTE real (não só o resolver isolado) a respeitar o timeout.
   • Gap 2: TestHTTPClientReturnsStallHardenedSharedClient deixar de depender do shell do dev —
     resolver lido na chamada, não no init do pacote.

   Status em andamento: Cursor já está implementando os dois gaps agora (sessão tmux
   cursor-t14:1.1) — stallHardenedClient(timeout) parametrizada + sharedStallClient() com cache
   por valor resolvido + teste novo TestHTTPClientTransportUsesResolvedHeaderTimeout com servidor
   HTTP real provando o timeout. Confirmar ao fechar que os dois itens acima batem com o texto do
   review, não com o que eu tinha deduzido antes de verificar.