From 39fb3b47f001c108f27426bcc78872b48cd207fb Mon Sep 17 00:00:00 2001 From: Francisco Solla Date: Wed, 19 Aug 2026 13:00:15 -0300 Subject: [PATCH] =?UTF-8?q?fix(OPS62-followup):=20=C3=A2ncora=20de=20rel?= =?UTF-8?q?=C3=B3gio=20no=20fixture=20do=20municipalitiesWithoutUpdateTool?= =?UTF-8?q?=20=E2=80=94=20elimina=20o=20flake=20de=20ordena=C3=A7=C3=A3o?= =?UTF-8?q?=20por=20raiz?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Causa raiz (reproduzida, refuta a hipótese da Issue): daysAgo() chamava Date.now() por chamada; as duas daysAgo(40) de Beta/Alpha podiam cair em ms diferentes sob carga, deixando Alpha estritamente mais novo — sortStale ordenava por timestamp e o tie-break por nome nunca disparava (0.08% das construções sem pausa; 100% com pausa de 1 ms; fixture ancorado: 0 em 2000). Fix test-only: âncora fixtureNowMs no load do módulo (padrão prioritiesTool) + hoist do timestamp empatado (igualdade estrutural) + comentário. Nada muda em produção — o sort real é determinístico sobre timestamps fixos. Verificação: spec 30/30, unit 2405, int 791, tsc/lint/format/cycles verdes. --- docs/CHANGELOG-AGENTS.md | 2 + docs/changelog/2026-08-19-ops62-followup.md | 1 + ...llowup-ci-flake-fixture-timestamps-impl.md | 138 ++++++++++++++++++ ...nicipalitiesWithoutUpdateTool.unit.spec.ts | 13 +- 4 files changed, 151 insertions(+), 3 deletions(-) create mode 100644 docs/changelog/2026-08-19-ops62-followup.md create mode 100644 docs/plans/ops62-followup-ci-flake-fixture-timestamps-impl.md diff --git a/docs/CHANGELOG-AGENTS.md b/docs/CHANGELOG-AGENTS.md index 08580099..5ce1bb7b 100644 --- a/docs/CHANGELOG-AGENTS.md +++ b/docs/CHANGELOG-AGENTS.md @@ -6,6 +6,8 @@ Conteúdo movido do `AGENTS.md` em 2026-07-30 (fatiamento do paradigma de agente **OPS69** — o admin do Payload abria **em branco em produção** (P0, 2026-08-19): o `importMap` commitado (`src/app/(payload)/admin/importMap.js`) ficou sem a entrada `@payloadcms/storage-s3/client#S3ClientUploadHandler` — o commit OPS52 (`dabc00f4`) removeu o handler do Vercel Blob e **não** adicionou o do S3, porque o `pnpm generate:importmap` rodou **sem** as envs `S3_*`: com o plugin `s3Storage` condicional, sem as envs o importMap sai consistente (dev/test, storage local) e com elas (produção, Garage) o admin precisa do handler e o montava → `getFromImportMap: PayloadComponent not found in importMap` → login nunca renderizava. Fix mínimo (opção A do plano, aprovado no gate): regenerar o importMap com as 4 `S3_*` dummy inline (a mecânica do CLI: `iterateConfig` sem DB; `loadEnvConfig` do `@next/env` não sobrescreve envs já no processo) → diff de **exatamente +2 linhas** (import + entrada do `S3ClientUploadHandler`), nenhum outro drift. Verificação: smoke headless com S3 ativo no dev local — `/admin/login` renderiza (6 inputs, formulário visível, **zero erros de console**) vs. o branco reproduzido no diagnóstico; gates completos verdes (tsc/lint/format/knip/cycles, unit+int 791, build). Sem migration/access/Consent/UI. Docs: linha no AGENTS.md (checklist local: `generate:importmap` precisa das `S3_*` setadas — valores dummy ok — enquanto o plugin for condicional; sem elas o importMap fica órfão, classe OPS69) + item sucessor registrado (guard de CI que regenera com envs dummy e falha o gate se o arquivo commitado divergir — `depends` #77). Verificação pós-deploy: `/admin` abre o login e o log do container não re-emite o `getFromImportMap` para o handler (runbook `docs/ops/teqo-1313-deploy.md`). +**OPS62-FOLLOWUP** — flake do `municipalitiesWithoutUpdateTool.unit.spec.ts` (2 ocorrências 2026-08-18: gate local 19:22 e CI run 779 23:53) eliminado pela raiz, com a hipótese original da Issue refutada por reprodução. Causa raiz: o fixture do teste (não o ambiente nem locale/ICU): `daysAgo()` chamava `Date.now()` **por chamada**; as duas construções `daysAgo(40)` de `Beta`/`Alpha` podiam cair em milissegundos diferentes sob carga (GC/preempção do fork do vitest na máquina compartilhada), deixando o timestamp de `Alpha` estritamente mais novo — em `sortStale` (`getMunicipalitiesWithoutUpdate.ts`) o sort por timestamp vencia o tie-break por nome e o teste recebia `[Beta, Alpha]` no lugar de `[Alpha, Beta]` (só o par trocado, resto correto — "sort não atuou" estava errado; o sort atuou sobre timestamps acidentalmente desiguais). Repro mecânica (sort de produção verbatim + fixture): fixture atual 0.08% de falhas em 200k runs sem pausa e **100%** com pausa de 1 ms injetada; fixture ancorado 0 em 2000 runs. Fix (test-only, nada em produção — o sort real é determinístico sobre timestamps fixos do DB): âncora `fixtureNowMs = Date.now()` no load do módulo (padrão do `prioritiesTool.unit.spec.ts`) + hoist do timestamp empatado (`const stale40 = daysAgo(40)` compartilhado por `Beta`/`Alpha` — igualdade estrutural, imune até a regressão da âncora) + comentário explicando a classe de flake. Scan da mesma classe: `prioritiesTool` (âncora no load) e `suggestionCatalog` (now fixo) já seguros; nenhum outro spec combina `Date.now()` por chamada com assunção de igualdade. Verificação: 30/30 runs do spec, unit full (249 arquivos/2405 testes), int (89/791), tsc, lint, format, check:cycles verdes (knip só com o P3 ledgered de sempre). /simplify: 3 reviewers aprovaram; débito absorvido no próprio teste (comentário do porquê). Impl plan: `docs/plans/ops62-followup-ci-flake-fixture-timestamps-impl.md`. + **OPS67** — o cliente `scripts/lib/forgejo-api.mjs` ganhou **retry com backoff** nos fetchs: um `fetch failed` transitório (rede/DNS/reset — o sintoma que matava o run do safety net `agent-pr-ready-automerge.yml` no meio da espera de checks) agora retenta em **qualquer método**, e a leitura de body interrompida no meio (mesma classe de falha de rede) retenta **em GET**; 5xx do Cloudflare (502/503/504) retenta **só em GET** (protege o poll de 45 min do `waitForChecks` sem arriscar duplicar efeitos colaterais de escrita — body do 5xx cancelado antes do retry para liberar o socket); 4xx e body-drop em escrita continuam falhando na hora, fail-closed intacto. Política: 3 retries, base 300 ms ×2, jitter ±20%, `console.warn` pt-BR em cada retry (`tentativa N/4`), validação `retries >= 0`/`backoffMs >= 0`. Opções flat no `createApi` (`retries`, `backoffMs`, `jitter`, `sleepImpl`) — defaults preservam o comportamento de todos os 19 call sites (safety net, claim/status, issue-transition, pool, worktree) sem mudança de chamador; zero-dep mantido. Testes: 12 specs novas em `forgejoApi.unit.spec.ts` (rede GET/POST → sucesso, body-drop GET retenta + POST fail-closed, exaustão de rede/5xx, `retries: 0`, backoff exponencial [100, 200, 400], jitter ±20% com `Math.random` pinado, 4xx sem retry, 5xx-GET retry/exaustão, 5xx-POST sem retry) + pin no spec 403 existente (1 fetch por invocação). Verificação: unit 2403 verdes; lint/format/typecheck/knip/cycles verdes. Impl plan: `docs/plans/ops67-forgejo-api-retry-backoff-impl.md` (aprovado no gate humano; refinamentos do /simplify absorvidos na execução). **OPS67** — o cliente `scripts/lib/forgejo-api.mjs` ganhou **retry com backoff** nos fetchs: um `fetch failed` transitório (rede/DNS/reset — o sintoma que matava o run do safety net `agent-pr-ready-automerge.yml` no meio da espera de checks) agora retenta em **qualquer método**; 5xx do Cloudflare (502/503/504) retenta **só em GET** (protege o poll de 45 min do `waitForChecks` sem arriscar duplicar efeitos colaterais de escrita); 4xx continua falhando na hora, fail-closed intacto. Política: 3 retries, base 300 ms ×2, jitter ±20%, `console.warn` pt-BR em cada retry (`tentativa N/4`). Opções flat no `createApi` (`retries`, `backoffMs`, `jitter`, `sleepImpl`) — defaults preservam o comportamento de todos os 19 call sites (safety net, claim/status, issue-transition, pool, worktree) sem mudança de chamador; zero-dep mantido. Testes: 6 specs novas em `forgejoApi.unit.spec.ts` (rede transitória→sucesso, rede persistente→throw após N, 4xx sem retry, 5xx-GET retry, 5xx-POST sem retry, backoff exponencial [100, 200, 400] + pin do 403 existente com calls=1). Verificação: unit 2397 verdes; lint/format/typecheck/knip/cycles verdes. Impl plan: `docs/plans/ops67-forgejo-api-retry-backoff-impl.md` (aprovado no gate humano). diff --git a/docs/changelog/2026-08-19-ops62-followup.md b/docs/changelog/2026-08-19-ops62-followup.md new file mode 100644 index 00000000..d1daa5f9 --- /dev/null +++ b/docs/changelog/2026-08-19-ops62-followup.md @@ -0,0 +1 @@ +**OPS62-FOLLOWUP** — flake do `municipalitiesWithoutUpdateTool.unit.spec.ts` (2 ocorrências 2026-08-18: gate local 19:22 e CI run 779 23:53) eliminado pela raiz, com a hipótese original da Issue refutada por reprodução. Causa raiz: o fixture do teste (não o ambiente nem locale/ICU): `daysAgo()` chamava `Date.now()` **por chamada**; as duas construções `daysAgo(40)` de `Beta`/`Alpha` podiam cair em milissegundos diferentes sob carga (GC/preempção do fork do vitest na máquina compartilhada), deixando o timestamp de `Alpha` estritamente mais novo — em `sortStale` (`getMunicipalitiesWithoutUpdate.ts`) o sort por timestamp vencia o tie-break por nome e o teste recebia `[Beta, Alpha]` no lugar de `[Alpha, Beta]` (só o par trocado, resto correto — "sort não atuou" estava errado; o sort atuou sobre timestamps acidentalmente desiguais). Repro mecânica (sort de produção verbatim + fixture): fixture atual 0.08% de falhas em 200k runs sem pausa e **100%** com pausa de 1 ms injetada; fixture ancorado 0 em 2000 runs. Fix (test-only, nada em produção — o sort real é determinístico sobre timestamps fixos do DB): âncora `fixtureNowMs = Date.now()` no load do módulo (padrão do `prioritiesTool.unit.spec.ts`) + hoist do timestamp empatado (`const stale40 = daysAgo(40)` compartilhado por `Beta`/`Alpha` — igualdade estrutural, imune até a regressão da âncora) + comentário explicando a classe de flake. Scan da mesma classe: `prioritiesTool` (âncora no load) e `suggestionCatalog` (now fixo) já seguros; nenhum outro spec combina `Date.now()` por chamada com assunção de igualdade. Verificação: 30/30 runs do spec, unit full (249 arquivos/2405 testes), int (89/791), tsc, lint, format, check:cycles verdes (knip só com o P3 ledgered de sempre). /simplify: 3 reviewers aprovaram; débito absorvido no próprio teste (comentário do porquê). Impl plan: `docs/plans/ops62-followup-ci-flake-fixture-timestamps-impl.md`. diff --git a/docs/plans/ops62-followup-ci-flake-fixture-timestamps-impl.md b/docs/plans/ops62-followup-ci-flake-fixture-timestamps-impl.md new file mode 100644 index 00000000..bff0e1d3 --- /dev/null +++ b/docs/plans/ops62-followup-ci-flake-fixture-timestamps-impl.md @@ -0,0 +1,138 @@ +# Impl: OPS62-followup — flake do municipalitiesWithoutUpdateTool: fixture com relógio ancorado + +Status: aprovado +Atualizado em: 2026-08-19 +Issue: #81 +Intenção: body da Issue é a spec (sem plano de intenção linkado) +Appetite restante: P3 — correção de teste, 1 linha de produção de fixture + verificação + +## Leitura da intenção + +- **Outcome:** eliminar o flake do teste `orders never-updated first (by name), then oldest +to newest (by name as tie-break)` (`municipalitiesWithoutUpdateTool.unit.spec.ts`), que + recebeu `[Beta, Alpha]` no lugar de `[Alpha, Beta]` em 2026-08-18 (gate local 19:22 e CI + run 779 23:53), tornando o spec deterministicamente estável sob carga. +- **O que NÃO negociar:** nenhum — o corpo da Issue não impõe lockdowns de produto; a + semântica de ordenação B189 (nunca atualizados por nome no topo, depois do mais velho ao + mais novo com tie-break por nome) é contrato do teste e do tool e **não muda**. +- **O que reavaliar:** a hipótese da Issue ("ordem recebida é a do fixture — sort não atuou; + suspeita de locale/ICU do ambiente"). **Refutada por reprodução mecânica** — a causa raiz + é do próprio fixture. + +## Causa raiz (reproduzida) + +`tests/unit/municipalitiesWithoutUpdateTool.unit.spec.ts:44`: + +```ts +const daysAgo = (days: number): string => new Date(Date.now() - days * DAY_MS).toISOString() +``` + +O teste de ordenação constrói `Beta` e `Alpha` com **duas chamadas separadas** `daysAgo(40)` +(linhas 142 e 144). Se ≥1 ms decorrer entre as duas chamadas (GC pause, preempção do fork do +vitest sob suíte full em máquina compartilhada), o timestamp ISO de `Alpha` fica +**estritamente mais novo** que o de `Beta` → em `sortStale` +(`getMunicipalitiesWithoutUpdate.ts:198-205`), `leftAt !== rightAt` vence e o tie-break por +nome nunca dispara → `['Mike','Zulu','Beta','Alpha','Gama']` em vez de +`['Mike','Zulu','Alpha','Beta','Gama']`. É exatamente o diff reportado (só o par trocado, +resto correto). + +Medições (`node /tmp/opencode/ops62-followup-repro.mjs`, sort de produção copiado verbatim): + +- Fixture atual, sem pausa, 200 000 runs: **0.082% de falhas** (fronteira de ms cruzada). +- Fixture atual com pausa de 1 ms injetada entre as duas chamadas, 2000 runs: **100% de + falhas**, amostra `["Mike","Zulu","Beta","Alpha","Gama"]`. +- Fixture ancorado com a mesma pausa, 2000 runs: **0 falhas**. + +20 runs isolados passarem é consistente: em máquina ociosa a dupla chamada quase sempre +cai no mesmo milissegundo; sob carga, preempções ≥1 ms são rotina. Locale/ICU não é o +fator decisivo — `localeCompare('pt-BR')` é determinístico; quem decide o par errado são +timestamps acidentalmente desiguais. + +## Abordagem recomendada + +**Opções consideradas:** A | B | C + +- **A — Ancorar o relógio do fixture uma vez no load do módulo:** `const fixtureNowMs = +Date.now()` no topo do spec e `daysAgo = (days) => new Date(fixtureNowMs - days * +DAY_MS).toISOString()`. As duas `daysAgo(40)` produzem timestamps **exatamente iguais** → + o tie-break por nome é o que o teste realmente exercita, deterministicamente, qualquer + que seja a carga. +- **B — Data literal fixa** (padrão `suggestionCatalog.unit.spec.ts:364`, ex. + `new Date('2026-08-18T12:00:00.000Z')`). +- **C — Reescrever o teste para não depender de timestamps iguais** (ex.: assert por ids, + ou remover o par empatado). + +**Recomendação: A** — porque é a mudança mínima (1 linha + âncora), elimina **toda** +dependência de relógio do spec, e segue o precedente local exato +(`tests/unit/prioritiesTool.unit.spec.ts:12` tem `const nowMs = Date.now()` no escopo do +módulo). A âncora no load garante que os timestamps empatados sejam **exatamente** iguais +(ms a ms), o que nenhuma outra opção faz com tanta simplicidade. + +**Rejeitadas:** + +- **B** porque introduz uma data mágica no fixture sem necessidade; o âncora de load faz o + mesmo com menos atrito e alinha com o precedente do `prioritiesTool`. (O `suggestionCatalog` + precisa da data fixa porque injeta `now` na função sob teste — aqui a função sob teste usa + `new Date()` interno, então a âncora de load é suficiente.) +- **C** porque o teste existe para cobrir o tie-break por nome do contrato B189; reescrevê-lo + sem o par empatado **remove cobertura** em vez de estabilizá-la. +- **Mexer no código de produção** (`getMunicipalitiesWithoutUpdate.ts`) porque não há bug de + produção: os timestamps reais vêm do DB (valores fixos) e o sort é determinístico. O bug é + exclusivamente do fixture do teste. + +### Componentes / mudanças + +- **`tests/unit/municipalitiesWithoutUpdateTool.unit.spec.ts`**: âncora `const fixtureNowMs = +Date.now()` no escopo do módulo + `daysAgo` derivando dela; comentário curto explicando o + porquê (a classe de flake é sutil — sem o comentário, um editor pode re-inliner o + `Date.now()` e reintroduzir o flake). No teste de ordenação, o timestamp empatado é + **hoisted** (`const stale40 = daysAgo(40)` usado em `Beta` e `Alpha`): a igualdade vira + estrutural, não probabilística — mesmo uma regressão da âncora não reativa o flake nesse + par. +- **Migration:** sem migration. +- **Access / Consent:** não se aplica (arquivo de teste). +- **UI:** não se aplica. + +### Scan da mesma classe (feito, não escopo) + +- `prioritiesTool.unit.spec.ts:12` — já ancorado no load do módulo (seguro). +- `suggestionCatalog.unit.spec.ts:364` — `now` fixo (seguro). +- `campaignComponents.unit.spec.ts:545` — chamada única, sem assunção de igualdade (seguro). +- Nenhum outro spec combina `Date.now()` por chamada + assunção de igualdade entre duas + chamadas. + +## Fases verificáveis + +1. **Fixture** — âncora de relógio no spec (1 linha + comentário). Quota: mínima. +2. **Reprodução & estabilidade** — rodar o spec 30×: `for i in $(seq 1 30); pnpm test:unit +-- --reporter=dot municipalitiesWithoutUpdateTool` (todas verdes). Rodar o script + `/tmp/opencode/ops62-followup-repro.mjs` (não commita) para o registro do mecanismo. +3. **Gates** — `pnpm test:unit` full, `pnpm test:int` (int), `pnpm exec tsc --noEmit`, + `pnpm lint`, `pnpm format:check`, `pnpm exec knip`, `pnpm check:cycles`. +4. **Changelog da entrega (OPS44)** — `docs/changelog/2026-08-19-ops62-followup.md` + + `pnpm changelog:build` + `pnpm changelog:check` (guards do CI exigem). + +## Rabbit holes / Não escopo (engenharia) + +- Investigação de locale/ICU do container — refutada pela reprodução; não seguir. +- Mudança no sort de produção — não há bug lá. +- Instrumentar o CI para capturar a 3ª ocorrência — o mecanismo está provado; a correção + remove a classe inteira, e a 3ª ocorrência deixa de ser possível por este caminho. +- Ancorar/limpar os outros specs da mesma classe — já verificados seguros. + +## Riscos e mitigação + +- **A âncora muda a semântica de idade relativa** (timestamps ficam relativos ao load do + módulo, não à execução): os asserts de limiar usam dias distintos (40/31/30/5 e 20/16/15/14) + e a execução ocorre sempre **depois** do load → idades só crescem; nenhum assert depende do + valor exato de `diasSemAtualizacao` (o teste do limiar asserta ids, não idades). Mitigação: + rodar a suíte full para confirmar. +- **Alguém reverter a âncora no futuro**: mitigado pelo comentário no spec. + +## Aceite de engenharia + +- [x] Aceite de produto da intenção ainda coberto (teste verde determinístico, contrato B189 intacto) +- [x] Invariantes AGENTS/engineering-standards (test-only; sem migration/access/URL) +- [x] Testes de domínio previstos (spec existente — a própria correção é dele) +- [x] Self-score decision-quality ≥4 (opções com rejeitadas; rabbit holes nomeados; depth + check: zero código novo, reusa padrão local `prioritiesTool`; appetite P3 respeitado) diff --git a/tests/unit/municipalitiesWithoutUpdateTool.unit.spec.ts b/tests/unit/municipalitiesWithoutUpdateTool.unit.spec.ts index 2d28d983..5e12c0df 100644 --- a/tests/unit/municipalitiesWithoutUpdateTool.unit.spec.ts +++ b/tests/unit/municipalitiesWithoutUpdateTool.unit.spec.ts @@ -41,7 +41,13 @@ const municipalityDoc = (overrides: Record) => ({ ...overrides, }) -const daysAgo = (days: number): string => new Date(Date.now() - days * DAY_MS).toISOString() +// Fixture clock anchored once at module load (same pattern as the prioritiesTool +// spec): per-call Date.now() drifting ≥1 ms between two daysAgo(40) calls made +// the tie-break pair unequal and flipped the name ordering (OPS62-followup CI +// flake 2026-08-18). The ordering spec also hoists its tied timestamp so the +// equality is structural, not just probabilistic. +const fixtureNowMs = Date.now() +const daysAgo = (days: number): string => new Date(fixtureNowMs - days * DAY_MS).toISOString() type ExecutableTool = { execute: (args: unknown, options?: unknown) => Promise @@ -135,13 +141,14 @@ describe('getMunicipalitiesWithoutUpdate coverage criterion (B189)', () => { }) it('orders never-updated first (by name), then oldest to newest (by name as tie-break)', async () => { + const stale40 = daysAgo(40) const find = vi .fn() .mockResolvedValue( findResult([ - municipalityDoc({ id: 1, lastUpdateAt: daysAgo(40), name: 'Beta' }), + municipalityDoc({ id: 1, lastUpdateAt: stale40, name: 'Beta' }), municipalityDoc({ id: 2, lastUpdateAt: null, name: 'Zulu' }), - municipalityDoc({ id: 3, lastUpdateAt: daysAgo(40), name: 'Alpha' }), + municipalityDoc({ id: 3, lastUpdateAt: stale40, name: 'Alpha' }), municipalityDoc({ id: 4, lastUpdateAt: null, name: 'Mike' }), municipalityDoc({ id: 5, lastUpdateAt: daysAgo(32), name: 'Gama' }), ]), -- 2.45.2