From 46b7526c8d040b17430fe3c68b5007e15abf983c Mon Sep 17 00:00:00 2001 From: Francisco Solla Date: Tue, 18 Aug 2026 11:21:46 -0300 Subject: [PATCH] =?UTF-8?q?fix(ops=20OPS56):=20forgejo-pr-automerge=20?= =?UTF-8?q?=E2=80=94=20veredito=20de=20merge=20por=20re-read=20do=20PR=20(?= =?UTF-8?q?falso=20positivo=20'j=C3=A1=20mergeado'=20com=20PR=20aberto)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/CHANGELOG-AGENTS.md | 2 + docs/changelog/2026-08-18-ops56.md | 1 + ...orgejo-pr-automerge-falso-positivo-impl.md | 147 ++++++++++++++++ scripts/forgejo-pr-automerge.mjs | 21 ++- scripts/lib/forgejo-api.mjs | 39 ++++- tests/unit/forgejoApi.unit.spec.ts | 162 ++++++++++++++++++ 6 files changed, 358 insertions(+), 14 deletions(-) create mode 100644 docs/changelog/2026-08-18-ops56.md create mode 100644 docs/plans/ops56-forgejo-pr-automerge-falso-positivo-impl.md diff --git a/docs/CHANGELOG-AGENTS.md b/docs/CHANGELOG-AGENTS.md index a6698494..7fb3128b 100644 --- a/docs/CHANGELOG-AGENTS.md +++ b/docs/CHANGELOG-AGENTS.md @@ -18,6 +18,8 @@ Conteúdo movido do `AGENTS.md` em 2026-07-30 (fatiamento do paradigma de agente **Recently resolved (2026-08-18):** **OPS54** — fim do warning do pnpm sobre o campo `pnpm` ignorado em `package.json`. A allowlist `onlyBuiltDependencies` (`sharp`, `esbuild`, `unrs-resolver`) migrou para o home canônico do pnpm 10: novo `pnpm-workspace.yaml` na raiz (sem `packages` — repo single-package, default `['.']`), e o bloco `"pnpm"` saiu do `package.json` (pnpm 10 não o lê mais; sem a remoção o warning persiste). `Dockerfile` estágio `deps` agora copia `pnpm-workspace.yaml` junto (`COPY package.json pnpm-lock.yaml pnpm-workspace.yaml ./`) — sem isso a allowlist não chegaria ao install da imagem e local×Docker divergiriam nos build scripts. Validado com a versão pinada (10.11.0) e no estágio deps real (`docker build --target deps`): nenhum `[WARN]` do campo `pnpm` em install/CI/Docker, `pnpm config get onlyBuiltDependencies` → os 3 pacotes, `--frozen-lockfile` limpo (lockfile e grafo intocados). Guardrails respeitados: `.npmrc` (`legacy-peer-deps`, `include`) e `packageManager` intocados; nada além do que o warning citava foi migrado (o aviso informativo de `msw`/`onnxruntime-node` é comportamento padrão do pnpm 10 — fora de escopo por anti-goal). Sem migration, sem UI, sem testes de domínio (config de ferramenta; gates completos verdes). Repo: `docs/plans/ops54-pnpm-warning-onlybuiltdependencies-impl.md`. +**Recently resolved (2026-08-18):** **OPS56**: o `forgejo-pr-automerge` nunca mais reporta falso positivo "já mergeado" com PR aberto. Raiz confirmada no source do servidor (Forgejo 9.0.3, tag v9.0.3): o endpoint `POST /pulls/{n}/merge` responde **200 com corpo vazio** no sucesso (falhas são 404/405/409/500), e o `request()` do `forgejo-api.mjs` convertia isso em `null` — o `autoMerge()` devolvia `null` cru e o CLI imprimia "já mergeado" (exit 0) independente do estado real do PR; o incidente (PR #27 da OPS52, aberto das 01:59 às 02:45) deixou o merge falhar silenciosamente com o PR aberto. Correção: o veredito de `autoMerge` agora é **re-read do PR** após a tentativa (`{ attempted, merged, pr }`) — 200-vazio → confirma no `GET /pulls/{n}`; POST falhou e o PR segue aberto → **re-throw** (falha ruidosa, exit 1); raça perdida (outro ator mergeou) → "já mergeado" só com o PR realmente mergeado; PR fechado sem merge → "fechado sem merge — skip" honesto. `waitForChecks` passou a esperar o `mergeable` amadurecer (false → throw imediato de conflito; null → segue no poll), mesmo timeout de 30 min. CLI sem shadowing de `pr` (const redeclarada). Testes: 6 specs novas em `forgejoApi.unit.spec.ts` (veredito por re-read, POST 405 + PR aberto → reject, raça concorrente, no-op de já-mergeado, poll de mergeable null, fail-fast de mergeable false). Gates: 2318 unit + 791 int + tsc/lint/prettier/knip/check:cycles verdes. Débito deferido: alargar o gatilho do `agent-pr-ready-automerge.yml` para branches não-`cursor/*` (decisão de ops separada). + **Recently resolved (2026-08-17):** **OPS51** — o banco de produção saiu do Neon para o Postgres do homeserver (`teqo_1313`, container `teqo-1313`), sem mudança de config: o `~/stack/teqo-1313.env` já apontava para o banco preparado (criado 16/08 com role própria + cadeia completa de 56 migrations + zero conteúdo — verificado), então restaurar = a produção "apareceu" no container. **Divergência deliberada da intenção (questão A/B validada como B):** rejeitado criar `teqo_prod` novo — o argumento "o banco do 1313 tem vida própria" não se sustenta (o site 1313 É a produção deste repo) e o OPS53 já trata `teqo_1313` como produção. Execução 100% no homeserver (PII nunca tocou a workstation): URL do Neon puxada do Vercel (`vercel env pull`, projeto `jorgesolla`, `DATABASE_URL_UNPOOLED` → `NEON_DATABASE_URL` em `~/stack/.env` chmod 600), `pg_dump -Fc` (Neon PG 17.10) → `/srv/hdd/backups/teqo-neon-pre-migracao/teqo-neon-full-20260817-204800.dump` (5,8 MB, sha256 `f12196f2…`), `pg_restore --no-owner --no-privileges` como a role `teqo_1313` (drop schema + grant, padrão do `db-pull`), verificado: contagens por tabela idênticas ao Neon (1919 contatos, 39 posts, 1487 assinaturas, 1484 subscrições, 35 campaign_users, 392 lideranças, 2 pledges, 435 municípios, 6 atividades; supporter segue 0 — blocker LGPD ainda de pé), sequences no topo (`last_value == max(id)`), objetos com dono `teqo_1313`, `payload migrate` no-op (56 = 56, serviço de maintenance `teqo-1313-migrate`), 3 credenciais WebAuthn + 34 sessions + 5 chaves Consent restauradas (RP ID não mudou — biometria segue valendo). Smoke sem deploy: `/`, artigo com conteúdo restaurado, `/campanha/login`, `/admin`, `POST /api/revalidate` (`posts` + `global_privacy-policy`), WebAuthn `login-options`, barreira 307 sem sessão — verdes; smoke de browser (login, biometria, convites) validado manualmente. **Rollback documentado:** Neon nunca foi escrito (só lido); voltar = re-apontar `DATABASE_URL` do `teqo-1313.env` para o Neon + `compose up -d`. **Débito registrado (→ OPS52):** as 40 rows de `media` têm URLs relativas (`/api/media/file/…`) sem arquivos em lugar nenhum — capas dão 500 no homeserver e **404 no próprio Vercel/produção atual** (estado pré-existente; migração manteve paridade). Repo: plano da intenção concluído, `AGENTS.md` e help de `db:pull`/`check-push-chain` sem a referência ao Neon; infra-solla `STATE.md` atualizado à parte. **Recently resolved (2026-08-17):** **OPS53** — o deploy do site 1313 virou etapa do CI: mergir em `main` com a suite verde == site atualizado em `jorgesolla1313.com.br`, sem passo manual. O job `deploy` em `.forgejo/workflows/ci.yml` roda no host do runner (workstation, `runs-on: host`) e streama `scripts/deploy-homeserver.sh` para o homeserver via SSH (`ssh homeserver "bash -s -- "`); o script remoto (bash puro, homeserver sem dependências novas) faz: guard de HEAD (`git ls-remote` de `main` == SHA do job, senão skip "stale"), `flock` de serialização, workspace `~/teqo-deploy` (clone do Forgejo local), `docker login localhost:5000` (REGISTRY_USER/PASSWORD de `~/stack/.env`), build dos stages `runner`+`migrator` no homeserver (`--network stack_default` para a geração estática alcançar o `postgres:5432` do compose; secrets de `~/stack/teqo-1313.env` via BuildKit, sem camadas/logs), push no registry local, swap dos tags de imagem no compose (com backup `docker-compose.yml.pre-`), **migrate pelo serviço de maintenance `teqo-1313-migrate` antes do rollout**, `docker compose up -d`, espera do healthcheck e smoke (`/`, `/campanha/login`, `/admin`, barreira 307, WebAuthn `login-options` com body JSON, `POST /api/revalidate` com o secret real — rehearsal validado contra o site vivo). Falha pós-swap → rollback automático best-effort (restore do backup + `up -d` da imagem anterior) + job vermelho. **Divergência deliberada da intenção (fato novo):** a intenção assumia o runner no homeserver; verificado (`infra-solla/STATE.md` + ao vivo) que o runner é a `workstation-runner` na workstation e o stack de produção vive no homeserver — a abordagem A foi corrigida para "job `runs-on: host` na workstation + todo o deploy no homeserver via SSH", o que manteve **zero secrets novos no Forgejo** e zero mudança de infra no compose. Nenhum secret no repo; envs seguem chmod-600 no homeserver. Runbook (rollback manual, falhas conhecidas, decisão de segurança do host label): `docs/ops/teqo-1313-deploy.md` (novo). Guarda estrutural nova: `tests/unit/deployScript.unit.spec.ts` (bash -n + invariantes do pipeline: HEAD guard, BuildKit secrets, ordem migrate→rollout, smoke). AGENTS.md/AGENT-OPS.md atualizados. Primeiro deploy real = o merge deste próprio item (tracer bullet); infra-solla `STATE.md` anotado à parte. diff --git a/docs/changelog/2026-08-18-ops56.md b/docs/changelog/2026-08-18-ops56.md new file mode 100644 index 00000000..e6eb867a --- /dev/null +++ b/docs/changelog/2026-08-18-ops56.md @@ -0,0 +1 @@ +**Recently resolved (2026-08-18):** **OPS56**: o `forgejo-pr-automerge` nunca mais reporta falso positivo "já mergeado" com PR aberto. Raiz confirmada no source do servidor (Forgejo 9.0.3, tag v9.0.3): o endpoint `POST /pulls/{n}/merge` responde **200 com corpo vazio** no sucesso (falhas são 404/405/409/500), e o `request()` do `forgejo-api.mjs` convertia isso em `null` — o `autoMerge()` devolvia `null` cru e o CLI imprimia "já mergeado" (exit 0) independente do estado real do PR; o incidente (PR #27 da OPS52, aberto das 01:59 às 02:45) deixou o merge falhar silenciosamente com o PR aberto. Correção: o veredito de `autoMerge` agora é **re-read do PR** após a tentativa (`{ attempted, merged, pr }`) — 200-vazio → confirma no `GET /pulls/{n}`; POST falhou e o PR segue aberto → **re-throw** (falha ruidosa, exit 1); raça perdida (outro ator mergeou) → "já mergeado" só com o PR realmente mergeado; PR fechado sem merge → "fechado sem merge — skip" honesto. `waitForChecks` passou a esperar o `mergeable` amadurecer (false → throw imediato de conflito; null → segue no poll), mesmo timeout de 30 min. CLI sem shadowing de `pr` (const redeclarada). Testes: 6 specs novas em `forgejoApi.unit.spec.ts` (veredito por re-read, POST 405 + PR aberto → reject, raça concorrente, no-op de já-mergeado, poll de mergeable null, fail-fast de mergeable false). Gates: 2318 unit + 791 int + tsc/lint/prettier/knip/check:cycles verdes. Débito deferido: alargar o gatilho do `agent-pr-ready-automerge.yml` para branches não-`cursor/*` (decisão de ops separada). diff --git a/docs/plans/ops56-forgejo-pr-automerge-falso-positivo-impl.md b/docs/plans/ops56-forgejo-pr-automerge-falso-positivo-impl.md new file mode 100644 index 00000000..a8862c44 --- /dev/null +++ b/docs/plans/ops56-forgejo-pr-automerge-falso-positivo-impl.md @@ -0,0 +1,147 @@ +# Impl: OPS56 — forgejo-pr-automerge: falso positivo 'já mergeado' com PR aberto + +Status: aprovado +Atualizado em: 2026-08-18 +Issue: #28 +Intenção: sem plano linkado — body é spec (frontmatter + título) +Appetite restante: pequeno (bug de ops, 1 script + 1 lib + testes) + +## Leitura da intenção + +- **Outcome:** o `forgejo-pr-automerge` nunca mais reporta "já mergeado" com o + PR ainda aberto; falha de merge vira falha **ruidosa** (exit ≠ 0), nunca + silenciosa. +- **O que NÃO negociar:** script continua zero-dependência (plain Node, roda + dentro de Forgejo Actions sem `pnpm install`); idempotência (rodar de novo + sobre PR já mergeado = no-op sucesso); merge continua por rebase. +- **O que reavaliar:** a "Direção no codebase" não existe (sem plano de + intenção) — o diagnóstico abaixo foi feito em campo. + +## Diagnóstico (raiz confirmada na versão do servidor) + +Servidor: **Forgejo 9.0.3** (`gitea-1.22.0` base). Fonte do router +`routers/api/v1/repo/pull.go` no tag `v9.0.3`: + +- `POST /pulls/{index}/merge` **sucesso → `ctx.Status(http.StatusOK)` — 200 com + corpo VAZIO** (swagger `"200": responses/empty`). Falhas são sempre + não-2xx (405 já-mergeado/WIP/não-mergeável/proteção, 409 conflito/out-of-date, + 404/500). + +Bug no cliente (`scripts/lib/forgejo-api.mjs`): + +1. `request()` devolve `text ? JSON.parse(text) : null` — o corpo vazio do 200 + vira **`null`**. +2. `mergePullRequest()` retorna esse `null` cru; `autoMerge()` trata `null` + como "PR já mergeado" (`return null`); o CLI imprime **"já mergeado"** e + sai 0. + +Ou seja: o veredito é **dirigido pela forma da resposta**, não pela verdade do +PR. Qualquer 200-vazio (o próprio endpoint de merge, ou um hop intermediário — +o host está atrás do Cloudflare — engolindo a resposta) vira "já mergeado", +com o PR **aberto**. Incidente real: PR #27 (OPS52) aberto 01:59→02:45 de +2026-08-18; Issue #28 registrada às 02:17 com o PR ainda aberto. + +Bug secundário: `autoMerge` retorna `null` também quando o PR foi **fechado +sem merge** entre a entrada e a tentativa — o CLI imprime "já mergeado" para +um PR que não mergeou. + +## Abordagem recomendada + +```mermaid +flowchart LR + A[waitForChecks] -->|open + checks verdes + mergeable| B[POST merge] + B -->|200 vazio| C[re-read GET pulls/N] + B -->|405/409/500 throw| D[re-read GET pulls/N] + C -->|merged| E[verdict: mergeado] + C -->|open| F[verdict: falha ruidosa] + D -->|merged| E + D -->|open| F +``` + +**Opções consideradas:** + +- A. Veredito por **re-read do PR** após a tentativa (`GET /pulls/{n}`) — + `merged`/`state` são a fonte única da verdade; o corpo do POST vira + irrelevante. +- B. Veredito pela forma da resposta (status code / truthiness) — o status quo + que quebrou. +- C. Endpoint próprio de verificação + retry com backoff. + +**Recomendação: A** — o POST 200-vazio do Forgejo é contrato da versão (não +vai mudar); re-read é idempotente, barato (1 GET) e cobre todas as raças +(merge concorrente, PR fechado à mão, hop que engoliu a resposta). Sem retry: +falha vira ruidosa e o próximo evento/run humano retenta — honesto e simples. + +**Rejeitadas:** B porque é exatamente o bug; C porque retry mascara conflito +real e a próxima tentativa no mesmo run de workflow não existe — quem retenta +é o próximo evento/ator, e o ruído do exit 1 é o aceite. + +### Componentes / mudanças + +- **`autoMerge`** (`scripts/lib/forgejo-api.mjs`): passa a retornar + `{ attempted, merged, pr }`: + - `waitForChecks` retornou já-mergeado/fechado → `{ attempted: false, +merged, pr }` (sem POST). + - POST ok → re-read → `{ attempted: true, merged: pr.merged, pr }`. + - POST lançou (405/409/…) → re-read: se `merged` → `{ attempted: true, +merged: true, pr }` (raça perdida — outro ator mergeou); se ainda + `OPEN` → **re-throw** (falha ruidosa com a mensagem da API); se fechado + sem merge → `{ attempted: true, merged: false, pr }`. + - JSDoc atualizado: "o veredito é re-read do PR, não o corpo do POST (200 + vazio no Forgejo 9)". +- **`waitForChecks`** (`scripts/lib/forgejo-api.mjs`): além de statuses + verdes, espera o `mergeable` amadurecer — `mergeable === false` → throw + imediato ("não mergeável — conflito?"); `null` (ainda computando) → segue + no poll. Mesmo timeout existente (30 min) vale para ambos. +- **`forgejo-pr-automerge.mjs`** (CLI): mensagens derivadas do novo shape — + `merged && attempted` → "mergeado (rebase)"; `merged && !attempted` → + "já mergeado" (verdadeiro); `!merged && state CLOSED` → "fechado sem merge + — skip" (exit 0); `!merged && OPEN` não é alcançável no CLI (autoMerge + re-throw) — o catch top-level já imprime "falhou" + exit 1. +- **Migration:** sem migration. **Access/Consent/UI:** n/a (scripts de ops). + +### Depth check + +`mergePullRequest` continua cru (verbo HTTP); a semântica mora em `autoMerge` +(único call site) — sem módulo novo, sem função nova no CLI (mapping é 3 +linhas derivadas do shape). + +## Fases verificáveis + +1. **Lib + testes** — `autoMerge`/`waitForChecks` reescritos + specs + `tests/unit/forgejoApi.unit.spec.ts` (fetch mockado): + - POST 200-vazio → re-read merged → `{ attempted: true, merged: true }`; + - POST 405 + re-read ainda open → reject (mensagem da API); + - POST 405 + re-read merged (raça) → `{ attempted: true, merged: true }`; + - primeiro poll já merged → `{ attempted: false, merged: true }` sem POST; + - `mergeable` null → segue polando; `false` → reject; statuses verdes + + `mergeable: true` → retorna. +2. **CLI** — mensagens pelo novo shape. +3. **Gates** — `pnpm gate:fast`; `pnpm push`; changelog + `docs/changelog/2026-08-18-ops56.md` + `pnpm changelog:build`. + +## Rabbit holes / Não escopo + +- Retry com backoff dentro do script (rejeitado — ver Opções). +- Expandir `agent-pr-ready-automerge.yml` para branches não-`cursor/*`: o + incidente foi num PR de worktree humano, mas o fluxo humano já fecha PR com + o próprio script/skill; alargar o gatilho é decisão de ops (cobertura), + separada do veredito — registrar como débito. +- Integrar o veredito no pool (o pool já deriva do estado real do PR via + `classifyPoolClaim` — nada a fazer lá). +- Mudar o endpoint/versão do Forgejo. + +## Riscos e mitigação + +- Re-read após POST falhar (rede) → o GET propaga erro (ruidoso); próximo run + é idempotente. +- `mergeable` ficar `false` por motivo transitório → throw imediato com + mensagem; run humano retenta após re-test (o PR re-test no UI/API). +- Teste unitário com `mergeable` ausente (undefined) → tratado como "ainda + computando" (só `=== false` é terminal). + +## Aceite de engenharia + +- [ ] Aceite de produto da intenção ainda coberto (nunca "já mergeado" com PR aberto) +- [ ] Invariantes: plain-Node zero-dep preservado; idempotência preservada; rebase preservado +- [ ] Testes de domínio previstos (unit) cobrem os 5 cenários de `autoMerge` + `mergeable` diff --git a/scripts/forgejo-pr-automerge.mjs b/scripts/forgejo-pr-automerge.mjs index b1ac0f1e..0c5e109f 100644 --- a/scripts/forgejo-pr-automerge.mjs +++ b/scripts/forgejo-pr-automerge.mjs @@ -2,7 +2,10 @@ * Plain-Node CLI (no pnpm) — agent PR safety net: wait for checks and merge by * rebase (the Forgejo equivalent of `gh pr merge --auto --rebase`, which the * Forgejo API does not offer as a scheduled operation). Idempotent: already - * merged → no-op success. + * merged → no-op success. The verdict comes from re-reading the PR (Forgejo's + * merge endpoint answers 200 + empty body on success, so the POST response + * alone cannot prove the merge) — "já mergeado" is only printed for a PR the + * server actually reports as merged; a failed merge exits 1 loudly. * * node scripts/forgejo-pr-automerge.mjs --pr */ @@ -45,14 +48,18 @@ try { } else { console.log(`[forgejo-pr-automerge] PR #${prNumber} já Ready`) } - const merged = await api.autoMerge(prNumber, { + const { attempted, merged } = await api.autoMerge(prNumber, { log: (line) => console.log(`[forgejo-pr-automerge] ${line}`), }) - console.log( - merged - ? `[forgejo-pr-automerge] PR #${prNumber} mergeado (rebase)` - : `[forgejo-pr-automerge] PR #${prNumber} já mergeado`, - ) + if (merged) { + console.log( + attempted + ? `[forgejo-pr-automerge] PR #${prNumber} mergeado (rebase)` + : `[forgejo-pr-automerge] PR #${prNumber} já mergeado`, + ) + } else { + console.log(`[forgejo-pr-automerge] PR #${prNumber} fechado sem merge — skip`) + } } catch (error) { console.error(`[forgejo-pr-automerge] falhou: ${error instanceof Error ? error.message : error}`) process.exit(1) diff --git a/scripts/lib/forgejo-api.mjs b/scripts/lib/forgejo-api.mjs index 71545d6c..6d6b33b9 100644 --- a/scripts/lib/forgejo-api.mjs +++ b/scripts/lib/forgejo-api.mjs @@ -269,12 +269,18 @@ export const createApi = ({ base, token, repository, fetchImpl } = {}) => { body: { ...payload, branch_name: branch }, }), - /** All required statuses on the PR head are success (or a failure exists). */ + /** + * All required statuses on the PR head are success (or a failure exists), + * and Forgejo finished computing mergeability. Returns the last PR read. + */ waitForChecks: async (number, { timeoutMs = 30 * 60 * 1000, pollMs = 15000, log } = {}) => { const started = Date.now() for (;;) { const pr = await api.getPullRequest(number) if (!pr || pr.state !== 'OPEN' || pr.merged) return pr + if (pr.mergeable === false) { + throw new Error(`PR #${number} não mergeável (conflito?) — resolver manualmente.`) + } const statuses = await api.listCommitStatuses(pr.head.sha) const failed = statuses.filter( (entry) => entry.state === 'failure' || entry.state === 'error', @@ -284,23 +290,42 @@ export const createApi = ({ base, token, repository, fetchImpl } = {}) => { const names = failed.map((entry) => entry.context).join(', ') throw new Error(`Checks falharam no PR #${number}: ${names}`) } - if (statuses.length > 0 && pending.length === 0) return pr + if (statuses.length > 0 && pending.length === 0 && pr.mergeable === true) return pr if (Date.now() - started > timeoutMs) { throw new Error(`Timeout esperando checks do PR #${number}.`) } if (log) - log(`[pr#${number}] ${statuses.length} status, ${pending.length} pendente — aguardando…`) + log( + `[pr#${number}] ${statuses.length} status, ${pending.length} pendente, mergeable=${ + pr.mergeable ?? 'computando' + } — aguardando…`, + ) await new Promise((resolve) => setTimeout(resolve, pollMs)) } }, - /** Wait for checks then merge by rebase (the `gh pr merge --auto --rebase` equivalent). */ + /** + * Wait for checks then merge by rebase (the `gh pr merge --auto --rebase` + * equivalent). The verdict is a RE-READ of the PR, never the merge POST + * response: Forgejo 9 answers a successful merge with 200 + empty body, so + * response shape alone cannot distinguish "merged" from "still open". + * + * @returns {Promise<{ attempted: boolean, merged: boolean, pr: object | null }>} + */ autoMerge: async (number, options) => { const pr = await api.waitForChecks(number, options) - if (pr && !pr.merged && pr.state === 'OPEN') { - return api.mergePullRequest(number) + if (!pr || pr.merged || pr.state !== 'OPEN') { + return { attempted: false, merged: Boolean(pr?.merged), pr } } - return null + try { + await api.mergePullRequest(number) + } catch (error) { + const after = await api.getPullRequest(number) + if (!after?.merged && after?.state === 'OPEN') throw error + return { attempted: true, merged: Boolean(after?.merged), pr: after } + } + const after = await api.getPullRequest(number) + return { attempted: true, merged: Boolean(after?.merged), pr: after } }, } diff --git a/tests/unit/forgejoApi.unit.spec.ts b/tests/unit/forgejoApi.unit.spec.ts index b334b74b..23dfacac 100644 --- a/tests/unit/forgejoApi.unit.spec.ts +++ b/tests/unit/forgejoApi.unit.spec.ts @@ -176,6 +176,168 @@ describe('forgejo-api', () => { expect(poll).toBe(2) }) + it('waitForChecks keeps polling while mergeable is still being computed', async () => { + let poll = 0 + const api = createApi({ + token: 'tok', + fetchImpl: async (url) => { + if (String(url).includes('/pulls/')) { + poll += 1 + return ok({ + number: 4, + state: 'open', + merged: false, + mergeable: poll < 2 ? null : true, + head: { ref: 'x', sha: 'abc' }, + }) + } + return ok({ statuses: [{ context: 'CI / static', status: 'success' }] }) + }, + }) + + const pr = await api.waitForChecks(4, { pollMs: 1 }) + + expect(poll).toBe(2) + expect(pr.mergeable).toBe(true) + }) + + it('waitForChecks fails fast when the PR is not mergeable', async () => { + const api = createApi({ + token: 'tok', + fetchImpl: async (url) => { + if (String(url).includes('/pulls/')) { + return ok({ + number: 4, + state: 'open', + merged: false, + mergeable: false, + head: { ref: 'x', sha: 'abc' }, + }) + } + return ok({ statuses: [{ context: 'CI / static', status: 'success' }] }) + }, + }) + + await expect(api.waitForChecks(4, { pollMs: 1 })).rejects.toThrow(/não mergeável/) + }) + + it('autoMerge verifies the merge by re-reading the PR (POST answers 200 + empty body)', async () => { + const calls: string[] = [] + let poll = 0 + const api = createApi({ + token: 'tok', + fetchImpl: async (url, init) => { + calls.push(`${init?.method ?? 'GET'} ${url}`) + if (String(url).includes('/merge')) return new Response('', { status: 200 }) + if (String(url).includes('/pulls/')) { + poll += 1 + const merged = poll > 1 + return ok({ + number: 27, + state: merged ? 'closed' : 'open', + merged, + mergeable: true, + head: { ref: 'x', sha: 'abc' }, + }) + } + return ok({ statuses: [{ context: 'CI / static', status: 'success' }] }) + }, + }) + + const result = await api.autoMerge(27, { pollMs: 1 }) + + expect(result).toEqual({ + attempted: true, + merged: true, + pr: expect.objectContaining({ merged: true }), + }) + expect(calls.filter((call) => call.includes('/merge'))).toHaveLength(1) + expect( + calls.filter( + (call) => call === 'GET https://git.solla.dev/api/v1/repos/fsolla/teqo/pulls/27', + ), + ).toHaveLength(2) + }) + + it('autoMerge rethrows when the merge POST fails and the PR stays open', async () => { + const api = createApi({ + token: 'tok', + fetchImpl: async (url, _init) => { + if (String(url).includes('/merge')) + return ok('{"message":"PR not in mergeable state"}', 405) + if (String(url).includes('/pulls/')) { + return ok({ + number: 27, + state: 'open', + merged: false, + mergeable: true, + head: { ref: 'x', sha: 'abc' }, + }) + } + return ok({ statuses: [{ context: 'CI / static', status: 'success' }] }) + }, + }) + + await expect(api.autoMerge(27, { pollMs: 1 })).rejects.toThrow(/405/) + }) + + it('autoMerge reports merged when a concurrent actor merged after our POST failed', async () => { + let poll = 0 + const api = createApi({ + token: 'tok', + fetchImpl: async (url, _init) => { + if (String(url).includes('/merge')) + return ok('{"message":"Pull request was already merged"}', 405) + if (String(url).includes('/pulls/')) { + poll += 1 + const merged = poll > 1 + return ok({ + number: 27, + state: merged ? 'closed' : 'open', + merged, + mergeable: true, + head: { ref: 'x', sha: 'abc' }, + }) + } + return ok({ statuses: [{ context: 'CI / static', status: 'success' }] }) + }, + }) + + const result = await api.autoMerge(27, { pollMs: 1 }) + + expect(result).toEqual({ + attempted: true, + merged: true, + pr: expect.objectContaining({ merged: true }), + }) + }) + + it('autoMerge is a no-op for an already merged PR (no POST)', async () => { + const calls: string[] = [] + const api = createApi({ + token: 'tok', + fetchImpl: async (url, init) => { + calls.push(`${init?.method ?? 'GET'} ${url}`) + return ok({ + number: 4, + state: 'closed', + merged: true, + mergeable: false, + head: { ref: 'x', sha: 'abc' }, + }) + }, + }) + + const result = await api.autoMerge(4, { pollMs: 1 }) + + expect(result).toEqual({ + attempted: false, + merged: true, + pr: expect.objectContaining({ merged: true }), + }) + expect(calls.some((call) => call.includes('/merge'))).toBe(false) + }) + it('getFileContents decodes base64 and updateFile sends sha', async () => { const calls: FetchCall[] = [] const api = createApi({ -- 2.45.2