From 2438c73f07089d65bca5ae05ace2c1945fb8c199 Mon Sep 17 00:00:00 2001 From: Luiz Carlos Date: Thu, 15 Oct 2026 18:13:08 -0300 Subject: [PATCH] feat(harness): prepare the conformance review with verify.sh review and a skill The review steps lived in the root AGENTS.md, loaded in every session, and depended on the agent running git add -N on every new file: a forgotten one stayed out of the review without warning. The procedure moves to the revisao-conformidade skill, and the inputs come from `verify.sh review`, which diffs from the merge-base with every new file through a temporary index, finds the plan commit, always writes the contract drift, and fails before writing when the plan commit or the criteria file is missing. The review result is kept in revisao.md, so another session can publish it, one comment per ticket. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01RgE8sJLFxENgwV47XrwDGP --- .claude/agents/revisor-conformidade.md | 4 +- .claude/skills/implement-spec/SKILL.md | 4 +- .claude/skills/implement/SKILL.md | 2 +- .claude/skills/revisao-conformidade/SKILL.md | 76 +++++++ .../revisao-conformidade/agents/openai.yaml | 5 + AGENTS.md | 41 +--- REVIEW.md | 2 +- scripts/test/verify-review.test.sh | 190 ++++++++++++++++++ scripts/verify.sh | 135 +++++++++++++ 9 files changed, 414 insertions(+), 45 deletions(-) create mode 100644 .claude/skills/revisao-conformidade/SKILL.md create mode 100644 .claude/skills/revisao-conformidade/agents/openai.yaml create mode 100755 scripts/test/verify-review.test.sh diff --git a/.claude/agents/revisor-conformidade.md b/.claude/agents/revisor-conformidade.md index 7b2f67b..45fcf53 100644 --- a/.claude/agents/revisor-conformidade.md +++ b/.claude/agents/revisor-conformidade.md @@ -1,6 +1,6 @@ --- name: revisor-conformidade -description: Confere, em contexto novo, se uma mudança entrega o que foi planejado para o ticket - critérios de aceite, testes contra os cenários, contrato e schema contra o commit de plano e documentos donos. Recebe o caminho de um arquivo de diff, o identificador do ticket (ou, sem ticket, os critérios) e os caminhos da mensagem do commit de plano e da visão do desvio do contrato. Devolve achados, e não corrige nada. Use depois da /code-review e antes do full, como descrito no AGENTS.md da raiz. +description: Confere, em contexto novo, se uma mudança entrega o que foi planejado para o ticket - critérios de aceite, testes contra os cenários, contrato e schema contra o commit de plano e documentos donos. Recebe o caminho de um arquivo de diff, o identificador do ticket (ou, sem ticket, o caminho do arquivo de critérios) e os caminhos da mensagem do commit de plano e da visão do desvio do contrato. Devolve achados, e não corrige nada. Use depois da /code-review e antes do full, pela skill revisao-conformidade. tools: Read, Grep, Glob, mcp__linear-server__get_issue --- @@ -11,7 +11,7 @@ Você confere a conformidade de uma mudança que outro agente escreveu com o que ## O que você recebe - **O caminho de um arquivo de diff,** com a mudança inteira, inclusive o que ainda não está em commit. Leia-o por completo. Depois leia os arquivos alterados inteiros, e não só o trecho do diff, e os arquivos que os chamam. -- **O identificador do ticket.** Leia o ticket e a spec pai dele no Linear. Em mudança sem ticket, os critérios vêm no pedido. +- **O identificador do ticket.** Leia o ticket e a spec pai dele no Linear. Em mudança sem ticket, vem no lugar dele o caminho do arquivo de critérios (`docs/tickets/sem-ticket//criterios.md`): leia-o por completo, e confira a mudança contra ele. - **O caminho da mensagem do commit de plano** (`docs/tickets///plano.txt`), ou o aviso de que ela não existe. São as decisões aprovadas, os arquivos e interfaces e a mudança de dados: uma decisão da mensagem que o diff contraria, sem que o documento dono e o ticket tenham mudado junto, é achado. - **O caminho da visão do desvio do contrato** (`docs/tickets///contract-drift.txt`), ou o aviso de que ela não existe. Ela mostra o que mudou no contrato e no schema desde o commit de plano, que foi o desenho aprovado. diff --git a/.claude/skills/implement-spec/SKILL.md b/.claude/skills/implement-spec/SKILL.md index b0c0f7c..422e089 100644 --- a/.claude/skills/implement-spec/SKILL.md +++ b/.claude/skills/implement-spec/SKILL.md @@ -33,8 +33,8 @@ Communication to and from subagents should be sparse. Communicate primarily thro 6. If this changes the **frontier** of available tickets, kick off more **implementer subagents** to work on the new tickets. This allows for maximum concurrency. -7. Once all tickets are complete, call the Skill tool with `simplify` and then with `code-review` on the integration branch. Then run the conformance review once per ticket, as the root `AGENTS.md`, section "Revisão de conformidade", describes: the same diff of the integration branch, with each ticket's identifier, plan message and contract drift. Fix all issues raised by the code review and the conformance review in a single **implementer subagent**. Then, in the main session, run `scripts/verify.sh full` once, before the commit that closes the work, as the root `AGENTS.md`, section "Definição de pronto", describes. +7. Once all tickets are complete, call the Skill tool with `simplify` and then with `code-review` on the integration branch. Then call the Skill tool with `revisao-conformidade` once per ticket: each review gets the same diff of the integration branch, in that ticket's folder, with its identifier, plan message and contract drift. Triage each ticket's findings in the main session, as step 5 of that skill says, and fix the issues raised by the code review and the findings you accepted in a single **implementer subagent**. Then, in the main session, run `scripts/verify.sh full` once, before the commit that closes the work, as the root `AGENTS.md`, section "Definição de pronto", describes. -8. If a draft PR exists, mark it ready for review. Otherwise, resolve each ticket the way the issue tracker closes work, and report the integration branch. +8. If a draft PR exists, publish each ticket's review comment, as the `revisao-conformidade` skill describes in "Publish", and mark the PR ready for review. Otherwise, resolve each ticket the way the issue tracker closes work, and report the integration branch. 9. Clean up all **implementer subagent** worktrees. diff --git a/.claude/skills/implement/SKILL.md b/.claude/skills/implement/SKILL.md index 7a74cd9..0d879ec 100644 --- a/.claude/skills/implement/SKILL.md +++ b/.claude/skills/implement/SKILL.md @@ -14,6 +14,6 @@ Call the Skill tool with "tdd" where possible, at pre-agreed seams. While working, run only the tests tied to the files you changed, with the commands in the project's `AGENTS.md`; the stop hook already runs the type and lint checks. -Once done, call the Skill tool with "simplify" and then with "code-review" to review the work, then run the conformance review, as the root `AGENTS.md`, section "Revisão de conformidade", describes, and fix the findings you agree with. Then run the full verification once, before the commit that closes the work, as the root `AGENTS.md`, section "Definição de pronto", describes. +Once done, call the Skill tool with "simplify" and then with "code-review" to review the work, then call the Skill tool with "revisao-conformidade" to run the conformance review and triage its findings, as that skill describes. Then run the full verification once, before the commit that closes the work, as the root `AGENTS.md`, section "Definição de pronto", describes. Commit your work to the current branch. diff --git a/.claude/skills/revisao-conformidade/SKILL.md b/.claude/skills/revisao-conformidade/SKILL.md new file mode 100644 index 0000000..d02fd66 --- /dev/null +++ b/.claude/skills/revisao-conformidade/SKILL.md @@ -0,0 +1,76 @@ +--- +name: revisao-conformidade +description: "Run the conformance review of a change that alters code in apps/: prepare its inputs with scripts/verify.sh review, call the revisor-conformidade subagent with paths only, record and triage the findings, then publish them as a PR comment. Use after /code-review and before scripts/verify.sh full, and again when the pull request is opened." +--- + +How to run the conformance review that the root `AGENTS.md` requires, and what to do with its result. The rule (who reviews, when, and that the review does not block) is in that file, section "Revisão de conformidade"; what the `revisor-conformidade` subagent checks is in the root `REVIEW.md` and in `.claude/agents/revisor-conformidade.md`. + +The skill has two parts, at two moments: **Review**, after `/code-review` and before the `full`, so the fixes go into the single full verification; and **Publish**, when the pull request exists, which may be in another session. + +## Review + +1. **Folder.** With a ticket, the review lives in the ticket's folder, `docs/tickets///`, next to its acceptance file. Without a ticket, it lives in `docs/tickets/sem-ticket//`, with a short lowercase name you choose (letters, digits and hyphens). Without a ticket, first write the acceptance criteria of the change in `criterios.md` in that folder, one numbered criterion per line, from what was asked and not from what was done. The script refuses to run without it. + +2. **Inputs.** From the repository root, run: + + ```bash + scripts/verify.sh review # with a ticket + scripts/verify.sh review sem-ticket # without one + ``` + + It writes, in the folder, `review.diff` (everything the branch changed since it left the base, up to the working tree, with every new file Git does not ignore) and, with a ticket, `plano.txt` (the plan commit message) and `contract-drift.txt` (the contract and schema drift since the plan commit, always written). It fails, and says what to do, when the ticket has no plan commit, when `criterios.md` is missing, when the inputs would not be ignored by Git, or, with a ticket, when the `api` container is down. The base is `origin/main`; for a pull request to another base, set `HARNESS_BASE_REF`, as in `HARNESS_BASE_REF=test/main`. Read the list of files the diff adds, committed or not: a file the change created that is not there is ignored by Git, and stays out of the review. + +3. **Call the subagent.** Call `revisor-conformidade` with only the input lines the script printed: the path of the diff, the ticket identifier and the paths of the plan message and the contract drift, or, without a ticket, the path of `criterios.md` and the notice that there is no plan commit. Do not add your summary of the change, your opinion of it or what you think deserves attention: the value of the review is that it does not start from your conclusions. Run it in the branch checkout, because it reads the branch's acceptance file. + +4. **Record.** As soon as the subagent returns, write `revisao.md` in the same folder, in the format below, with its report verbatim inside `
` and one table row per finding. Do not omit a finding nor edit the report. With no findings, write the file anyway, with the table empty and the report. Git ignores the file, as it ignores everything in the ticket folder but the acceptance file. + +5. **Triage.** Decide each finding and fill its row: + - **Agree:** fix it. The situation is `Corrigido`, and the reason says what changed. + - **Disagree:** the situation is `Não corrigido`, and the reason says why it stays as it is. + - **High severity you would leave unfixed:** stop and ask the user before going on. Write the decision and that it was the user's in the reason. + + In `/implement-spec`, one implementer subagent fixes the findings of `/code-review` and of every ticket's review, as that skill says. The commit after the fixes does not go through the reviewer again: the `full` verifies it. + +6. **Go on** to the `full`, as the root `AGENTS.md` says in "Definição de pronto". In `/implement-spec`, that is after every ticket's review and the fixes, and not after each ticket. + +## Publish + +When the pull request exists, post each `revisao.md` of the branch as one comment, one per ticket. The file is local and ignored by Git, so publish from the checkout where the review ran; if it is missing (another checkout, a cleaned worktree), run the review again instead of writing the comment from memory. + +```bash +gh pr comment --body-file docs/tickets///revisao.md +``` + +Add `--repo /` when the pull request is not on the default remote. If the pull request already has the comment of that ticket, edit it instead of posting a second one: find its id by the title, then replace its body. + +```bash +gh api repos///issues//comments --paginate \ + --jq '.[] | select(.body | startswith("## Revisão de conformidade: \n")) | .id' +gh api -X PATCH repos///issues/comments/ -F body=@docs/tickets///revisao.md +``` + +The comment is the record the root `AGENTS.md` asks for, section "Revisão de conformidade". + +## Format of `revisao.md` + +The title names the ticket, or `sem ticket ()` for a change without one. + +```markdown +## Revisão de conformidade: + +| # | Severidade | Achado | Situação | Motivo | +| --- | --- | --- | --- | --- | +| 1 | alta | O que o revisor apontou | Corrigido | O que mudou | +| 2 | média | O que o revisor apontou | Não corrigido | Por que ficou como está | + +
+Relatório do revisor, na íntegra + +(o relatório, sem edição) + +
+``` + +## Without subagents + +In an agent that cannot call a subagent, run the review in a new session, with the instructions of `.claude/agents/revisor-conformidade.md` and the same inputs, and record and publish it the same way. diff --git a/.claude/skills/revisao-conformidade/agents/openai.yaml b/.claude/skills/revisao-conformidade/agents/openai.yaml new file mode 100644 index 0000000..4a9ca78 --- /dev/null +++ b/.claude/skills/revisao-conformidade/agents/openai.yaml @@ -0,0 +1,5 @@ +interface: + display_name: "Revisão de conformidade" + short_description: "Review a change against its plan and publish the findings" +policy: + allow_implicit_invocation: true diff --git a/AGENTS.md b/AGENTS.md index 29c1faa..1e0e8bb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -167,46 +167,9 @@ O aceite é conferido na máquina antes do push, como descrito em "Antes do push **Revisão de conformidade** -Quem escreveu o código não é quem o confere. Toda mudança que altera código em `apps/` passa pelo subagente `revisor-conformidade`, que não viu a implementação e confere se ela entrega o que foi planejado: o aceite, os testes contra os cenários, o contrato e o schema contra o commit de plano e os documentos donos. O que ele confere está no `REVIEW.md` da raiz, que o Copilot e o Claude Code Review também leem no PR, e o que só ele confere, em `.claude/agents/revisor-conformidade.md`. Os defeitos gerais de código ficam com a `/code-review`, e uma revisão feita na mesma sessão que escreveu o código não substitui esta. +Quem escreveu o código não é quem o confere. Toda mudança que altera código em `apps/` passa pelo subagente `revisor-conformidade`, que não viu a implementação e confere se ela entrega o que foi planejado: o aceite, os testes contra os cenários, o contrato e o schema contra o commit de plano e os documentos donos. O que ele confere está no `REVIEW.md` da raiz, que o Copilot e o Claude Code Review também leem no PR. Os defeitos gerais de código ficam com a `/code-review`, e uma revisão feita na mesma sessão que escreveu o código não substitui esta. -No fluxo do ticket, ela vem depois da `/code-review` e antes do `full`, para as correções entrarem na única verificação completa: implementação, `/simplify`, `/code-review`, revisão de conformidade, correções, `full` e commit. - -1. **Gere o diff, a mensagem do plano e a visão do desvio na pasta do ticket,** `docs/tickets///`, a mesma do arquivo de aceite que o commit de plano criou; numa mudança sem ticket, `docs/tickets/sem-ticket//`. O diff é o da pasta de trabalho contra a base, com os arquivos novos, porque a revisão vem antes do commit: - - ```bash - git add -N -- - git diff origin/main > docs/tickets///review.diff - plano="$(git log --follow --diff-filter=A --format=%h -1 -- docs/tickets///aceite.md)" - git show -s --format=%B "$plano" > docs/tickets///plano.txt - scripts/verify.sh contract "$plano" > docs/tickets///contract-drift.txt - ``` - - O `git add -N` só registra que os arquivos novos existem, para o diff trazê-los, sem prepará-los para o commit. Confira com o `git status --short` que nenhum arquivo da mudança ficou de fora: o que não está no diff fica fora da revisão, sem aviso. Sem commit de plano, não há mensagem nem visão do desvio. Os arquivos não aparecem no `git status`, porque o Git os ignora: ele versiona só o arquivo de aceite da pasta do ticket. - -2. **Chame o subagente `revisor-conformidade`** passando só o caminho do diff, o identificador do ticket e os caminhos da mensagem do plano e da visão do desvio, ou o aviso de que elas não existem. Numa mudança sem ticket, passe os critérios de aceite. Não passe o seu resumo da mudança, a sua opinião sobre ela nem o que você acha que merece atenção: o valor da revisão está em ela não partir das suas conclusões. Rode a revisão no checkout da branch, porque ele lê o arquivo de aceite dela. -3. **Corrija os achados com que você concorda** e siga para o `full` (ver "Definição de pronto"). -4. **Publique o resultado como comentário no PR,** quando ele for aberto, com o título `## Revisão de conformidade`, neste formato: - - ```markdown - ## Revisão de conformidade - - | # | Severidade | Achado | Situação | Motivo | - | --- | --- | --- | --- | --- | - | 1 | alta | O que o revisor apontou | Corrigido | O que mudou, e em que commit | - | 2 | média | O que o revisor apontou | Não corrigido | Por que ficou como está | - -
- Relatório do revisor, na íntegra - - (o relatório, sem edição) - -
- ``` - -- **Todo achado entra na tabela,** inclusive o que você considera errado. Nesse caso, a situação é "Não corrigido" e o motivo diz por quê. Não omita achado nem edite o relatório. -- **A revisão não bloqueia.** O que fazer com um achado não corrigido é decisão do usuário no merge. A CI não lê esse comentário. -- **Sem achados também é resultado:** publique o comentário mesmo assim, com o relatório. -- **Em agente sem subagentes,** faça a revisão numa sessão nova, com as instruções de `.claude/agents/revisor-conformidade.md`, e publique o comentário do mesmo jeito. +No fluxo do ticket, ela vem depois da `/code-review` e antes do `full`, para as correções entrarem na única verificação completa: implementação, `/simplify`, `/code-review`, revisão de conformidade, correções, `full` e commit. A revisão não bloqueia: todo achado, corrigido ou não, vai para um comentário no PR, e a decisão sobre o que ficou sem corrigir é do usuário no merge. Como preparar a revisão, chamar o subagente, tratar os achados e publicar o comentário está na skill `revisao-conformidade`. Os comandos de teste, de lint e de formatação de cada projeto estão no `AGENTS.md` dele. diff --git a/REVIEW.md b/REVIEW.md index 275c61e..f11779d 100644 --- a/REVIEW.md +++ b/REVIEW.md @@ -4,7 +4,7 @@ O que toda revisão deste repositório confere além dos defeitos de código: se ## Onde está o padrão -- **O aceite:** o arquivo citado na seção `## Aceite` do corpo do PR, `docs/tickets///aceite.md`. Cada critério é um título `## -CA`, com a marca, o cenário (Dado, Quando, Então) e, nos de teste, o teste previsto. O formato é o da skill de plano, `.claude/skills/plan-ticket/formatos/aceite.md`. Com `nenhum`, não há arquivo de aceite: os critérios, quando há, vêm no pedido de revisão; confira o motivo dado. +- **O aceite:** o arquivo citado na seção `## Aceite` do corpo do PR, `docs/tickets///aceite.md`. Cada critério é um título `## -CA`, com a marca, o cenário (Dado, Quando, Então) e, nos de teste, o teste previsto. O formato é o da skill de plano, `.claude/skills/plan-ticket/formatos/aceite.md`. Com `nenhum`, não há arquivo de aceite: na máquina, os critérios vêm no arquivo que o pedido de revisão aponta; confira o motivo dado. - **O plano:** o commit da branch que criou esse arquivo. A mensagem dele traz as decisões, os arquivos e interfaces e a mudança de dados; na máquina, ela chega ao revisor como arquivo. - **O contrato e o schema:** `apps/api/openapi.json` e `apps/api/prisma/schema.prisma`. O formato de cada rota está no `docs/lld.md`, seção 3, "Formato de uma rota no snapshot", e o de cada tabela, na seção 2, "Formato de uma tabela no schema". diff --git a/scripts/test/verify-review.test.sh b/scripts/test/verify-review.test.sh new file mode 100755 index 0000000..e079cf0 --- /dev/null +++ b/scripts/test/verify-review.test.sh @@ -0,0 +1,190 @@ +#!/usr/bin/env bash +# +# Testes do modo `review` do scripts/verify.sh: os argumentos, o diff da branch com os arquivos novos +# sem mexer no índice, a base pelo ponto em que a branch saiu dela, o commit de plano exigido e a +# mensagem dele, o desvio do contrato gravado sempre e a mudança sem ticket, que exige os critérios. +# O verify.sh roda copiado num repositório temporário, com o desvio do contrato trocado por um falso +# (a comparação real tem os próprios testes) e um `docker` falso, para o contêiner `api` contar como no +# ar. Rodam na máquina, como o verify.sh. Quem chama é o scripts/verify.sh. + +set -uo pipefail +ROOT="$(cd "$(dirname "$0")/../.." && pwd)" +# shellcheck source=sh-helpers.sh +source "$ROOT/scripts/test/sh-helpers.sh" + +tmp="$(mktemp -d)" +trap 'rm -rf "$tmp"' EXIT +repo="$tmp/repo" + +not() { + ! "$@" +} + +contains() { + [[ "$1" == *"$2"* ]] +} + +# O `docker` falso: toda chamada passa, como a do `require_container`. +mkdir -p "$tmp/bin" +printf '#!/usr/bin/env bash\nexit 0\n' >"$tmp/bin/docker" +chmod +x "$tmp/bin/docker" + +# O repositório: os scripts do verify.sh e as regras de docs/tickets do .gitignore, na main. +mkdir -p "$repo/scripts" "$repo/apps/api" "$repo/docs/tickets/PROJ-1/PROJ-3" +cp -R "$ROOT/scripts/verify.sh" "$ROOT/scripts/verify" "$ROOT/scripts/harness-dir.sh" "$ROOT/scripts/full-lock.sh" "$repo/scripts/" +grep '^!\{0,1\}docs/tickets/' "$ROOT/.gitignore" >"$repo/.gitignore" +# O desvio falso: com FAKE_DRIFT, há desvio, e ele faz como o grupo da API (acrescenta a visão ao +# arquivo e avisa); sem ela, diz que o contrato está igual, sem gravar nada. +cat >>"$repo/scripts/verify/api.sh" <<'EOF' + +api_contract_drift() { + if [[ -n "${FAKE_DRIFT_FAIL:-}" ]]; then + echo "o oasdiff não rodou" + return 1 + elif [[ -n "${FAKE_DRIFT:-}" ]]; then + printf '%s\n' "$FAKE_DRIFT" | tee -a "$3" + echo "::warning title=Contrato diferente do planejado::aviso" + else + echo "O contrato e o schema estão iguais ao planejado no commit de plano $(git rev-parse --short "$1")." + fi +} +EOF +git -C "$repo" init -q -b main +git -C "$repo" config user.email teste@exemplo.com +git -C "$repo" config user.name Teste +printf 'export const a = 1;\n' >"$repo/apps/api/existente.ts" +printf 'export const velho = 1;\n' >"$repo/apps/api/apagado.ts" +# Um aceite que já estava na main: nenhum commit da branch o criou. +printf '# Aceite do PROJ-3\n' >"$repo/docs/tickets/PROJ-1/PROJ-3/aceite.md" +git -C "$repo" add -A && git -C "$repo" commit -q -m base + +# A branch: o commit de plano do PROJ-2, uma mudança num arquivo versionado e um arquivo novo. +git -C "$repo" switch -q -c trabalho +mkdir -p "$repo/docs/tickets/PROJ-1/PROJ-2" +printf '# Aceite do PROJ-2\n' >"$repo/docs/tickets/PROJ-1/PROJ-2/aceite.md" +git -C "$repo" add -A && git -C "$repo" commit -q -m 'plano do PROJ-2' -m 'Decisão: a cota vale por usuário.' + +# A main anda depois que a branch saiu dela: a mudança dela não é da branch. +git -C "$repo" switch -q main +printf 'export const alheio = 1;\n' >"$repo/apps/api/alheio.ts" +git -C "$repo" add -A && git -C "$repo" commit -q -m 'mudança de outra branch' +git -C "$repo" switch -q trabalho + +# Um commit comum depois do plano, com um arquivo novo: o commit de plano deixa de ser o HEAD. +printf 'export const feito = true;\n' >"$repo/apps/api/commitado.ts" +git -C "$repo" add -A && git -C "$repo" commit -q -m 'implementa o PROJ-2' + +printf 'export const a = 2;\n' >"$repo/apps/api/existente.ts" +printf 'export const novo = true;\n' >"$repo/apps/api/novo.ts" +printf 'export const espaco = true;\n' >"$repo/apps/api/com espaço.ts" +rm "$repo/apps/api/apagado.ts" + +# review : roda `verify.sh review` no repositório temporário, contra a main dele, e +# deixa a saída em $out, a saída de erro em $err e o código em $code. +review() { + local errfile="$tmp/err" + out="$(cd "$repo" && env -u CLAUDE_PROJECT_DIR PATH="$tmp/bin:$PATH" HARNESS_BASE_REF=main \ + scripts/verify.sh review "$@" 2>"$errfile")" + code=$? + err="$(cat "$errfile")" +} + +# --- argumentos + +review PROJ-1 +check "recusa um argumento só, com código 2" test $code -eq 2 +check "a mensagem mostra as duas formas do comando" contains "$err" "review sem-ticket " + +review pai PROJ-2 +check "recusa um que não tem a forma PREFIXO-N" test $code -eq 2 + +review PROJ-1 ticket +check "recusa um que não tem a forma PREFIXO-N" test $code -eq 2 +check "a mensagem do inválido mostra as duas formas do comando" contains "$err" "review sem-ticket " + +review PROJ-1 PROJ-2 extra +check "recusa três argumentos" test $code -eq 2 +check "a mensagem de três argumentos mostra as duas formas do comando" contains "$err" "review sem-ticket " + +review sem-ticket 'Nome Ruim' +check "recusa um nome sem ticket com maiúscula e espaço" test $code -eq 2 + +review sem-ticket ../fora +check "recusa um nome sem ticket que sai da pasta" test $code -eq 2 + +# --- ticket sem plano + +review PROJ-1 PROJ-9 +check "recusa um ticket sem arquivo de aceite" test $code -eq 2 +check "diz que o arquivo de aceite não existe" contains "$err" "não existe" + +review PROJ-1 PROJ-3 +check "recusa um ticket cujo aceite nenhum commit da branch criou" test $code -eq 2 +check "diz que falta o commit de plano" contains "$err" "não tem commit de plano" + +# --- ticket planejado + +dir="$repo/docs/tickets/PROJ-1/PROJ-2" +plan="$(git -C "$repo" log --format=%H -1 --diff-filter=A -- docs/tickets/PROJ-1/PROJ-2/aceite.md)" +index_before="$(git -C "$repo" ls-files --stage | sha256sum)" +review PROJ-1 PROJ-2 +check "com o commit de plano, passa" test $code -eq 0 +diff_text="$(cat "$dir/review.diff" 2>/dev/null)" +check "o diff traz o arquivo novo" contains "$diff_text" "b/apps/api/novo.ts" +check "o diff traz a mudança no arquivo versionado" contains "$diff_text" "+export const a = 2;" +check "o diff traz o aceite criado pelo commit de plano" contains "$diff_text" "b/docs/tickets/PROJ-1/PROJ-2/aceite.md" +check "o diff não traz a mudança que a main ganhou depois" not contains "$diff_text" "alheio" +check "o diff traz o arquivo novo já em commit" contains "$diff_text" "b/apps/api/commitado.ts" +check "o diff traz o arquivo apagado" contains "$diff_text" "deleted file mode" +check "o diff traz o arquivo novo com espaço no nome" contains "$diff_text" "b/apps/api/com espaço.ts" +check "o índice de quem trabalha fica igual" test "$(git -C "$repo" ls-files --stage | sha256sum)" = "$index_before" +check "o arquivo novo continua fora do índice" not git -C "$repo" ls-files --error-unmatch --quiet apps/api/novo.ts 2>/dev/null +check "a mensagem do plano é a do commit de plano, e não a do HEAD" test "$(cat "$dir/plano.txt" 2>/dev/null)" = "$(git -C "$repo" log -1 --format=%B "$plan")" +check "sem desvio, o arquivo do desvio diz que o contrato está igual" contains "$(cat "$dir/contract-drift.txt" 2>/dev/null)" "iguais ao planejado" +check "o desvio é comparado com o commit de plano" contains "$(cat "$dir/contract-drift.txt" 2>/dev/null)" "$(git -C "$repo" rev-parse --short "$plan")" +check "a saída lista o arquivo novo fora do Git" contains "$out" "- apps/api/novo.ts" +check "a saída lista o arquivo novo já em commit" contains "$out" "- apps/api/commitado.ts" +check "a saída lista o arquivo novo com espaço no nome" contains "$out" "- apps/api/com espaço.ts" +check "a saída traz o caminho da mensagem do plano" contains "$out" "docs/tickets/PROJ-1/PROJ-2/plano.txt" +check "os insumos ficam fora do git status" not contains "$(git -C "$repo" status --porcelain)" "review.diff" + +FAKE_DRIFT="visão do desvio" review PROJ-1 PROJ-2 +drift_text="$(cat "$dir/contract-drift.txt" 2>/dev/null)" +check "com desvio, passa" test $code -eq 0 +check "com desvio, o arquivo traz a visão" contains "$drift_text" "visão do desvio" +check "com desvio, o arquivo não traz o aviso da CI" not contains "$drift_text" "::warning" + +FAKE_DRIFT_FAIL=1 review PROJ-1 PROJ-2 +check "com o desvio que falha, a revisão continua" test $code -eq 0 +check "com o desvio que falha, o arquivo diz que ele não foi conferido" contains "$(cat "$dir/contract-drift.txt" 2>/dev/null)" "não pôde ser conferido" + +review PROJ-1 PROJ-2 +check "uma revisão sem desvio não deixa a visão da anterior" not contains "$(cat "$dir/contract-drift.txt" 2>/dev/null)" "visão do desvio" + +# --- insumos que iriam para um commit + +cp "$repo/.gitignore" "$tmp/gitignore" +: >"$repo/.gitignore" +rm -f "$dir/review.diff" +review PROJ-1 PROJ-2 +check "sem a regra do .gitignore, recusa a revisão" test $code -eq 2 +check "sem a regra do .gitignore, diz que o insumo iria para um commit" contains "$err" "iria para um commit" +check "sem a regra do .gitignore, não grava o diff" test ! -e "$dir/review.diff" +cp "$tmp/gitignore" "$repo/.gitignore" + +# --- sem ticket + +review sem-ticket skill-de-revisao +check "sem ticket, recusa a revisão sem os critérios" test $code -eq 2 +check "sem ticket, diz que falta o criterios.md" contains "$err" "criterios.md" + +mkdir -p "$repo/docs/tickets/sem-ticket/skill-de-revisao" +printf '1. Um critério.\n' >"$repo/docs/tickets/sem-ticket/skill-de-revisao/criterios.md" +review sem-ticket skill-de-revisao +check "sem ticket, com os critérios, passa" test $code -eq 0 +check "sem ticket, grava o diff" test -s "$repo/docs/tickets/sem-ticket/skill-de-revisao/review.diff" +check "sem ticket, não grava mensagem de plano" test ! -e "$repo/docs/tickets/sem-ticket/skill-de-revisao/plano.txt" +check "sem ticket, a saída traz o caminho dos critérios" contains "$out" "sem-ticket/skill-de-revisao/criterios.md" +check "sem ticket, a saída avisa que não há plano nem desvio" contains "$out" "sem commit de plano" + +finish "do modo review do verify.sh" diff --git a/scripts/verify.sh b/scripts/verify.sh index cbeae44..1e14be4 100755 --- a/scripts/verify.sh +++ b/scripts/verify.sh @@ -25,6 +25,12 @@ # atalho para o plan-page.sh da skill de plano, com # os valores deste projeto; é a issue pai do # ticket, a pasta dele em docs/tickets +# scripts/verify.sh review +# scripts/verify.sh review sem-ticket +# os insumos do revisor de conformidade na pasta do +# ticket: o diff da branch, com os arquivos novos, e, +# com ticket, a mensagem do commit de plano e o desvio +# do contrato. É da skill revisao-conformidade # # Roda tudo nos contêineres do compose.dev.yaml, que já têm de estar no ar. Na suíte no navegador, # a web e a API também têm de estar respondendo; o `--start-apps` as sobe, e é para a CI. @@ -212,6 +218,7 @@ harness_tests() { scripts/test/freeze-acceptance.test.sh && scripts/test/plan-page.test.sh && scripts/test/verify-plan.test.sh && + scripts/test/verify-review.test.sh && scripts/test/harness-guard.test.sh && scripts/test/push-verify.test.sh && scripts/test/full-lock.test.sh && @@ -447,6 +454,116 @@ run_plan() { "$PLAN_SKILL_SCRIPTS/plan-page.sh" "${args[@]}" } +# O diff que o revisor lê: tudo o que a branch mudou desde que saiu da base, até a pasta de trabalho, +# com os arquivos novos que o Git não ignora. Compara com o ponto em que a branch saiu da base, e não +# com a ponta dela, para uma base que andou não trazer as mudanças alheias como se fossem da branch. +# Usa um índice temporário, como o `worktree_state`, para contar os arquivos novos sem mexer no índice +# de quem trabalha. Grava também a lista dos arquivos que o diff acrescenta, em commit ou não. +# review_diff +review_diff() { + local base dir code=0 + base="$(git merge-base "$BASE_REF" HEAD)" || return 1 + dir="$(mktemp -d)" || return 1 + ( + export GIT_INDEX_FILE="$dir/index" + git read-tree HEAD && git add -A && + plain_diff --cached "$base" >"$1" && + git -c core.quotePath=false diff --cached --name-only --diff-filter=A "$base" >"$2" + ) || code=1 + rm -rf "$dir" + return $code +} + +# O desvio do contrato desde o commit de plano, pelo grupo de cada app que declara +# _contract_drift, o mesmo do aviso do modo completo. O grupo só grava o arquivo quando há +# desvio; o revisor recebe o arquivo sempre, então, sem desvio, ele leva o que o grupo disse. O grupo +# que falha deixa no arquivo que o desvio dele não foi conferido, para o revisor não ler um vazio. +# review_drift +review_drift() { + local plan="$1" drift="$2" app out said=() failed=() + rm -f "$drift" + for app in "${APPS[@]}"; do + declare -F "${app}_contract_drift" >/dev/null || continue + if out="$("${app}_contract_drift" "$plan" "" "$drift" 2>&1)"; then + [[ -n "$out" ]] && said+=("$out") + else + failed+=("O desvio do contrato da app $app não pôde ser conferido. O que o grupo dela disse:"$'\n\n'"$out") + fi + done + if [[ ${#failed[@]} -gt 0 ]]; then + printf '%s\n\n' "${failed[@]}" >>"$drift" + elif [[ ! -s "$drift" && ${#said[@]} -gt 0 ]]; then + printf '%s\n' "${said[@]}" >"$drift" + elif [[ ! -s "$drift" ]]; then + echo "Nenhuma app declara contrato planejado: não há desvio a conferir." >"$drift" + fi +} + +# O modo `review`: os insumos do revisor de conformidade, na pasta do ticket, e os caminhos que a +# skill revisao-conformidade passa a ele. Com ticket, exige o commit de plano: um ticket sem ele não +# foi planejado, e não chega à revisão. Sem ticket, exige os critérios escritos antes, em +# criterios.md, para a revisão ter contra o que conferir. +# run_review , ou run_review sem-ticket +run_review() { + local dir="$TICKETS_DIR/$1/$2" acceptance plan="" file added + require_base + if [[ "$1" == sem-ticket ]]; then + [[ -f "$dir/criterios.md" ]] || + abort "não há critérios para a revisão: $dir/criterios.md não existe." \ + "escreva nele os critérios de aceite da mudança, antes de revisar, e rode \`scripts/verify.sh review sem-ticket $2\` de novo." + else + acceptance="$dir/aceite.md" + [[ -f "$acceptance" ]] || + abort "o arquivo de aceite $acceptance não existe." \ + "confira a issue pai e o ticket; se o ticket não foi planejado, rode a /plan-ticket antes." + plan="$(plan_commit "$acceptance")" || + abort "não deu para ler o histórico da branch ($BASE_REF..HEAD) com o \`git log\`." \ + "confira se a base existe (\`git fetch origin\`, ou outra base em HARNESS_BASE_REF) e se o clone tem o histórico inteiro." + [[ -n "$plan" ]] || + abort "nenhum commit desta branch criou o $acceptance: o ticket não tem commit de plano." \ + "planeje o ticket com a /plan-ticket e faça o commit de plano antes da revisão." + require_container api + fi + + # Os insumos não podem ir para um commit: o .gitignore ignora tudo na pasta do ticket, salvo o aceite. + # Confere antes de gravar, pelo padrão, para um abort não deixar arquivos prontos para um `git add`. + for file in review.diff plano.txt contract-drift.txt criterios.md; do + git check-ignore -q --no-index "$dir/$file" || + abort "o $dir/$file não seria ignorado pelo Git, e iria para um commit." \ + "confira as regras de $TICKETS_DIR no .gitignore: ele ignora tudo na pasta do ticket, salvo o aceite.md." + done + + added="$(mktemp)" || abort "não deu para criar um arquivo temporário." "confira o espaço e as permissões da pasta temporária." + review_diff "$dir/review.diff" "$added" || { + rm -f "$added" + abort "não deu para montar o diff da branch contra $BASE_REF." \ + "confira se o repositório do Git está íntegro, com \`git status\`, e se a base existe." + } + if [[ -n "$plan" ]]; then + git show -s --format=%B "$plan" >"$dir/plano.txt" || { + rm -f "$added" + abort "não deu para ler a mensagem do commit de plano $plan." "confira o repositório do Git com \`git status\` e rode de novo." + } + review_drift "$plan" "$dir/contract-drift.txt" + fi + + printf 'Insumos do revisor de conformidade em %s:\n' "$dir" + printf -- '- diff: %s\n' "$dir/review.diff" + if [[ -n "$plan" ]]; then + printf -- '- ticket: %s\n' "$2" + printf -- '- mensagem do commit de plano %s: %s\n' "$(git rev-parse --short "$plan")" "$dir/plano.txt" + printf -- '- desvio do contrato: %s\n' "$dir/contract-drift.txt" + else + printf -- '- critérios: %s\n' "$dir/criterios.md" + printf -- '- sem commit de plano: não há mensagem do plano nem desvio do contrato\n' + fi + if [[ -s "$added" ]]; then + printf '\nArquivos que o diff acrescenta:\n' + sed 's/^/- /' "$added" + fi + rm -f "$added" +} + mode="${1:-}" [[ $# -gt 0 ]] && shift @@ -534,6 +651,24 @@ case "$mode" in run_plan "$@" exit $? ;; + review) + # Os insumos não são verificação: não aprovam nada. + review_example='scripts/verify.sh review PROJ-1 PROJ-123` ou `scripts/verify.sh review sem-ticket ' + [[ $# -eq 2 ]] || + abort "o \`review\` recebe dois argumentos: \`review \`, ou \`review sem-ticket \`." \ + "passe a issue pai e o ticket, ou \`sem-ticket\` e um nome curto, como em \`$review_example\`." + if [[ "$1" == sem-ticket ]]; then + [[ "$2" =~ ^[a-z0-9][a-z0-9-]*$ ]] || + abort "o nome \`$2\` não vale para uma mudança sem ticket." \ + "use só letras minúsculas, algarismos e hífens, começando por letra ou algarismo, como em \`scripts/verify.sh review sem-ticket skill-de-revisao\`." + else + [[ "$1" =~ ^[A-Z][A-Z0-9]*-[0-9]+$ && "$2" =~ ^[A-Z][A-Z0-9]*-[0-9]+$ ]] || + abort "os argumentos de \`review\` não são e ." \ + "passe a issue pai e o ticket, os dois no formato PREFIXO-N, como em \`$review_example\`." + fi + run_review "$@" + exit $? + ;; *) usage ;; esac