Pular para conteúdo

AUR-438 — Comentar no arquivo e na linha exata (fiacao no cmd)

Documento histórico do board. Pode descrever componentes removidos; para uso atual, consulte o guia do produto.

O que este card entrega

O caminho de pull request de aurumcode review:

aurumcode review --pr <numero> --repo <dono>/<projeto> --publicar --na-linha

Le as mudancas de um pull request atraves do cliente GitHub restaurado (AUR-437, internal/git/githubclient), revisa o diff atraves do mesmo motor e da mesma selecao de provider que o caminho --base ja usa, e publica cada problema encontrado como comentario no pull request: na linha exata do arquivo quando o diff adicionou aquela linha, ou como comentario geral do pull request quando o problema esta fora das linhas alteradas -- nunca descartado em silencio. A publicacao recusa, antes de qualquer POST, quando o token nao tem permissao de escrita no repositorio.

Este card e fiacao, nao reescrita: o cliente GitHub (leitura de diff, listagem de arquivos, comentario inline, comentario geral, verificacao de permissao) e o motor de review ja existem e ja estao provados. O que falta, e o que este card adiciona, e a conversao do formato local de diff do cliente para pkg/types que o motor consome, a classificacao de cada achado em "linha exata" ou "geral", e o loop de publicacao -- tudo em cmd/aurumcode/pr.go.

Flags

  • --pr <numero>: numero do pull request. Ativa o caminho de PR; ausente, review continua exatamente como antes (--base, byte a byte).
  • --repo <dono>/<projeto>: obrigatorio com --pr. Qualquer outra forma (sem barra, dono ou projeto vazio, mais de uma barra) e erro de uso.
  • --publicar: obrigatorio com --pr (booleano). E o consentimento explicito do usuario para publicar; independente dele, a publicacao ainda recusa quando o token carecer de permissao de escrita (ver abaixo).
  • --na-linha: obrigatorio com --pr (booleano). Nomeia o comportamento que este card entrega -- comentar na linha exata quando possivel, geral quando nao.

--repo, --publicar e --na-linha sao obrigatorios com --pr pela mesma razao que --base ja e obrigatorio no caminho existente: este card constroi e testa exatamente o comando que ele declara (review --pr N --repo o/r --publicar --na-linha); nenhuma variante nao testada (por exemplo, --pr sem --publicar fazendo uma execucao seca) e um comportamento este card promete.

Deteccao de --pr usa fs.Visit (nomes canonicos de flag apos o parse), e o parser flag do Go ja trata -pr e --pr de forma identica -- um traco ou dois, tanto faz. Isso vale para toda flag deste comando, incluindo --base; nenhum guarda manual de string "--" foi escrito aqui (ver "Achado herdado" abaixo).

Classificacao: linha exata vs. comentario geral

Para cada achado do motor, o arquivo e a linha sao comparados ao diff que o cliente GitHub leu: se a linha e uma linha que o diff ADICIONOU (marcador + no hunk que a cobre) naquele arquivo, o achado vira um comentario inline (PostReviewComment, ancorado em path+line). Qualquer outro caso -- uma linha de contexto, uma linha alem de todo hunk, ou um arquivo que o diff nem tocou -- vira um comentario geral do pull request (PostIssueComment, com o arquivo e a linha no corpo do texto), nunca e descartado.

Os achados sao publicados em ordem (arquivo, linha) -- deterministica, independente da ordem em que o modelo os listou -- entao repetir a mesma entrada produz a mesma sequencia de publicacao, nao so a mesma saida de tela.

A conversao de tipos (achado herdado do revisor do AUR-437)

internal/git/githubclient define Diff/DiffFile/DiffHunk locais ao seu proprio pacote (seu aceite selado materializa so os paths/ read_paths daquele card, entao ele nao pode importar pkg/types). Os campos espelham pkg/types.Diff{,File,Hunk} um a um, de proposito, para que a conversao neste card seja trivial e sem perda -- e essa conversao (convertDiff em cmd/aurumcode/pr.go) e o que este card entrega: nao ha mais um espelhamento sem uso real, o motor de review consome o resultado convertido diretamente.

Nota deliberada: este documento nao repete a alegacao sobre extractFilePath e o separador " b/" que o spec do AUR-437 fez ao descrever a correcao do defeito 4 daquele card. extractFilePath pertence a internal/git/githubclient (read_path aqui, nao reescrito por este card), e este card nao caracteriza nem depende de nenhum comportamento alem do que esta unico teste ja prova: o path do lado novo do fixture pr-42.diff (cmdb/settings.go, cujo diretorio termina em "b") chega correto na conversao.

Permissao de escrita: verificada antes de qualquer trabalho, nao so antes de postar

runPRReview chama client.HasWritePermission uma vez, antes da busca do diff e antes de qualquer chamada ao motor/modelo, e recusa com uma mensagem clara, citando githubclient.ErrNoWritePermission, sem enviar nenhum POST, quando o token nao pode escrever. A ordem importa: um token somente-leitura nao deve pagar por uma chamada ao LLM cujo resultado ele nunca podera publicar. Isso e adicional, nao um substituto, ao guardiao que ja existe dentro de PostReviewComment/PostIssueComment (requireWritePermission, fail-closed) -- a checagem explicita aqui existe so para dar uma mensagem unica e clara em vez de uma falha por achado; a garantia de fail-closed em si pertence ao cliente e este card nao a reimplementa nem a enfraquece.

O SHA do commit: recusa fail-closed, nunca um POST com commit_id vazio

PostReviewComment exige um commit_id. O formato de diff unificado que GetPullRequestDiff le (Accept: application/vnd.github.v3.diff) nao carrega o SHA do commit de topo do pull request, e este card nao estende internal/git/githubclient (e read_path aqui) para adicionar um endpoint que o buscasse. commit_id vem da variavel de ambiente GITHUB_SHA -- a mesma que o GitHub Actions ja expoe a um step (ver docs/specs/AUR-440.md).

Isto nao e uma alegacao de que o comentario inline fica ancorado no commit de topo do pull request em uso real; e uma limitacao documentada, nao uma capacidade reivindicada. E, ao contrario de uma versao anterior deste documento, nao e mais tratado como inofensivo publicar com commit_id vazio: a API real do GitHub responde 422 a um comentario inline sem commit_id, e uma primeira versao deste card deixava esse POST sair mesmo assim (bloqueador medido pelo revisor: o aceite so nao pegava porque seus proprios fakes aceitavam qualquer commit_id, um mock que sempre concordava).

A regra agora: depois de classificar os achados, se pelo menos um exige comentario inline (ver "Classificacao" acima) e GITHUB_SHA esta ausente, o comando recusa antes de qualquer POST -- inline ou geral -- com uma mensagem clara em stderr, o mesmo formato fail-closed da checagem de permissao. Um review cujos achados sao todos gerais nao precisa de SHA algum (PostIssueComment nao carrega commit_id), entao esse caso nao e bloqueado pela ausencia da variavel. tests/e2e/AUR-438.sh reproduz o 422 real: seu fake GitHub responde 422 a um commit_id vazio no endpoint de comentario inline, e o cenario "sem GITHUB_SHA" prova que zero POSTs chegam ao servidor -- a recusa e um guardiao deste comando, nao uma consequencia observada so depois de uma resposta de erro do servidor.

Uma falha de publicacao isolada nao engole as demais

O loop de publicacao agrega falhas em vez de abortar no primeiro erro: se um POST (inline ou geral) falha, o achado seguinte ainda e tentado. Ao final, cada achado que falhou aparece listado em stderr e o exit code e 1; todo achado que PODE ser publicado ainda e. Uma versao anterior deste card retornava no primeiro erro (return 1 dentro do loop), o que, dado que os achados sao publicados em ordem (arquivo, linha), fazia uma falha no primeiro comentario (tipicamente o inline) descartar silenciosamente todo achado geral que viria depois -- o mesmo bloqueador do revisor, pela mesma causa raiz de "nenhum POST empresta commit_id vazio a esta versao antiga do codigo". tests/integration/AUR-438.go e tests/e2e/AUR-438.sh provam isso injetando uma falha (400, nunca retentada pelo cliente) na primeira publicacao e checando que a segunda ainda chega ao servidor.

Gap anotado, nao implementado: PostIssueComment sem chave de idempotencia

PostReviewComment recebe uma chave de idempotencia (X-GitHub-Idempotency-Key); PostIssueComment, na assinatura de internal/git/githubclient (read_path aqui, fora de paths), nao recebe nenhuma. Isso significa que reexecutar aurumcode review --pr ... --publicar --na-linha sobre o mesmo pull request -- por exemplo, um retry de CI apos uma falha de rede no meio da execucao -- pode duplicar o comentario geral de um achado ja publicado; o comentario inline, por ter chave de idempotencia, nao duplica. Esta e uma limitacao real do cliente restaurado que este card nao corrige: uma correcao exigiria mudar a assinatura de PostIssueComment, fora do que este card pode escrever. Fica registrada aqui para quem assumir esse trabalho.

Exit codes

  • 0: os achados foram publicados (ou "No issues found." quando o motor nao encontrou nenhum), sem erro.
  • 1: falha em tempo de execucao -- busca do diff, chamada ao motor, recusa por falta de permissao de escrita, ou falha ao publicar um comentario especifico.
  • 2: erro de uso -- --repo/--publicar/--na-linha ausentes com --pr, --pr nao positivo, ou --repo fora do formato dono/projeto.

Nenhum destes e novo: e o mesmo vocabulario de exit code que --base ja usa (docs/specs/AUR-430.md).

Exemplo executavel sem segredo

bash tests/e2e/AUR-438.sh E2EAUR438

Compila o binario real, sobe um fake GitHub em loopback (net/http/httptest sobre net.Listen) a partir de tests/fixtures/scm/github/ (o diff do PR 42, as permissoes de leitura-escrita e so-leitura, a resposta de comentario criado), escreve uma resposta de modelo deterministica com dois achados -- um na linha 3 de cmdb/settings.go (linha que o diff adicionou) e um na linha 99 de docs/notas.md (bem alem do unico hunk +1 daquele arquivo) -- e roda aurumcode review --pr 42 --repo dono/projeto --publicar --na-linha contra ele. O fake responde 422 a um comentario inline com commit_id vazio, exatamente como a API real -- nao e mais um mock que concorda com qualquer requisicao. Prova: o primeiro achado chega como comentario inline (path=cmdb/settings.go, line=3); o segundo chega como comentario geral citando docs/notas.md:99 em vez de desaparecer; repetir a execucao produz a mesma saida e a mesma sequencia de publicacao; com um token so-leitura nenhum POST alcanca o servidor -- o comando recusa antes; sem GITHUB_SHA (variavel genuinamente ausente, nao so vazia) o comando tambem recusa antes de qualquer POST, mesmo havendo um achado inline valido; e quando o fake falha deliberadamente a primeira publicacao (400, nunca retentado pelo cliente), a segunda ainda chega ao servidor -- uma falha isolada nao engole as demais. Tokens sao strings sinteticas montadas em tempo de execucao; nenhum shape de credencial existe em arquivo rastreado.

Selectors e exit codes do aceite

  • Aceite: tests/acceptance/AUR-438.sh -- selectors AC-001 (tudo), TestAUR438, IntegrationAUR438, E2EAUR438, AC-001-MUT-001.
  • Exit codes (convencao do board, tests/acceptance/EXIT_CODE_CONVENTION.md): 0 comportamento provado; 1 RED comportamental; 64 selector desconhecido; 79 infraestrutura / inconclusivo (nunca evidencia de RED).

Diagnostico de falha

  • AUR-438/AC-001/behavior-missing: o caminho de PR nao entrega o fluxo prometido (ver stderr do harness em tests/e2e/AUR-438.sh).
  • missing_general_finding / missing_general_marker / missing_general_comment: o achado fora das linhas alteradas nao foi publicado -- exatamente o que MUT-001 provoca ao mutar cmd/aurumcode/pr.go para descartar esse ramo em silencio.
  • readonly_not_refused / readonly_post_leaked: a recusa de publicacao sem permissao de escrita foi violada.
  • nosha_not_refused / nosha_post_leaked: a recusa fail-closed por SHA de commit ausente foi violada -- um POST (inline, com commit_id vazio) chegou ao servidor, ou o comando nao recusou.
  • onefail_swallowed_other_finding / onefail_missing_failure_summary / onefail_second_post_not_attempted: uma falha isolada de publicacao impediu outro achado de ser publicado -- a regressao que o bloqueador do revisor descreveu.
  • non_deterministic / non_deterministic_publish_count: duas execucoes identicas divergiram na saida ou na sequencia de publicacao.
  • infrastructure/missing_go, infrastructure/mktemp, infrastructure/fakegithub_did_not_start (exit 79): ambiente, nunca RED.

Achado herdado do revisor do AUR-440

O guarda de flags das lanes do AUR-440 so reconhecia tokens com prefixo "--", entao a grafia de traco unico ("-publish", que o pacote flag do Go trata identicamente) escapava da deteccao. Aquele guarda vive em tests/acceptance/AUR-440.sh/docs/specs/AUR-440.md, fora de paths e read_paths deste card -- nao foi tocado aqui. O que este card faz e nao repetir o erro na sua propria fiacao: nenhuma flag deste comando e detectada por um prefixo "--" escrito a mao; toda deteccao (--pr incluido) passa por fs.Visit sobre um flag.FlagSet comum, que ja trata -x e --x de forma identica para toda flag, nova ou preexistente (prova: TestReviewFlagsAcceptSingleOrDoubleDash em cmd/aurumcode/pr_test.go). A correcao do guarda das lanes do AUR-440 em si, e da frase correspondente em docs/specs/AUR-440.md:38-39, permanece um gap para um card com esses arquivos em paths.

Compatibility, migration, rollback

  • Compatibility: --base, --fail-on, --modelo e --seguranca mantem o contrato publicado byte a byte quando --pr esta ausente -- o dispatch para o caminho de PR acontece antes de qualquer verificacao do caminho --base, entao nenhum codigo existente e alterado, so contornado quando --pr e dado.
  • Migration: not-applicable.
  • Rollback: reverter somente os paths deste card; nenhum estado externo e publicado por ele fora do que o proprio usuario pediu ao rodar o comando.