Pular para conteúdo

AUR-519: skill vira gate, com bloqueio e inconclusivo configuráveis

Contrato

A análise do gate é feita pelo que uma skill Markdown diz, não por regra escrita em Go. Toda seção ## de toda skill (as da política central e as do próprio repositório) é lida a cada execução e vira uma regra citável: <arquivo-base-da-skill>#<slug-da-seção>. A política (nunca o repositório sozinho, a menos que ele opte) declara, em gate.fail_on, quais severidades reprovam o check, e em gate.inconclusive, o que uma revisão inconclusiva faz (block ou warn). Ver docs/configuration.md's "Gate" section para a sintaxe completa.

Estado desta entrega

O mecanismo está implementado e ligado de ponta a ponta em --base (runReview) e --pr (runPRReview), com testes de comportamento reais pelo comando (não só unitários) para AC-001 a AC-008 — incluindo o caminho multi-perfil (--perfis): runProfilePasses (cmd/aurumcode/profiles.go) instala o mesmo catálogo dinâmico e o mesmo catálogo de rule_id fundido em CADA Reviewer que constrói, um por perfil, para que selecionar --perfis nunca narrowe o gate de volta ao catálogo embutido sozinho.

Correções de uma revisão independente (B1-B6)

Uma revisão independente do commit 472f56b encontrou 5 defeitos reais, todos corrigidos nesta entrega, cada um com um teste de regressão que falha sem o fix (confirmado por mutação manual em B1 e B3):

  • B1 evaluateGate retornava assim que inconclusiveReason era não-vazio, para QUALQUER modo (inclusive warn), pulando o loop de limiar de severidade por completo -- uma quebra de severidade real, presente ao lado de uma cobertura parcial forçada (ex.: um arquivo ignorado/oversized no mesmo PR), saía com exit 0. Corrigido: só block retorna antes do loop; warn e a ausência da chave inconclusive marcam Inconclusive e continuam para o loop, e uma quebra real sempre marca Fail e o novo campo Breach. Breach separa "falhou porque o bloqueio de inconclusivo disparou, sem nunca checar achados" de "falhou porque um achado real cruzou o limiar", para o switch do status publicado e para o código de saída (Breach → exitFindings sempre, Fail sem Breach → exitQualityNotReviewed).
  • B4 o limiar comparava só issue.Severity -- a severidade que o próprio modelo devolveu, sob o controle do diff que ele está revisando (uma linha de injeção de prompt poderia pedir para rebaixar a severidade e escapar do gate). effectiveSeverityRank agora usa o MAIOR rank entre issue.Severity e a severidade que a própria regra dinâmica declarou (review.Rule.Severity, a linha opcional severity: da seção da skill) -- a declaração do autor da política/skill age como piso que o modelo não controla.
  • B5 com gate.fail_on configurado e gate.inconclusive ausente, uma revisão inconclusiva publicava "gate de política aprovado". Corrigido pelo mesmo fix de B1: ausência de inconclusive se comporta como warn -- nunca "aprovado", nunca bloqueia por si só.
  • B2 runProfilePasses nunca copiava Metadata de cada passe de perfil para o resultado combinado, então prompt.IsDegradedParse(merged) era sempre falso sob --perfis, por mais que um perfil tenha degradado. Corrigido: se QUALQUER perfil degradou, o combinado carrega ParseModeKey=ParseModeDegraded (escrito pelo motor, nunca copiado de um mapa que o modelo controla); os contadores de cobertura do AUR-476 são preservados do primeiro perfil (são os mesmos para todos, mesmo diff/orçamento).
  • B3 persistFreshResults cacheava um parse degradado (ou uma cobertura parcial) como "zero achados" -- uma segunda execução contra a MESMA fixture (a chave de cache já incorpora o conteúdo do arquivo de fixture) lia do cache um resultado "limpo" sem nenhum sinal de degradação, e um gate configurado passava com exit 0 na repetição. Corrigido: o cache nunca persiste quando prompt.IsDegradedParse(result) ou a cobertura é parcial.
  • Também corrigido (não um defeito B1-B5, apontado na mesma revisão): o motivo "provider_failure" do --pr estava rotulado errado para uma falha de PARSE (qualityDegraded: o provedor respondeu, o JSON é que era inválido) -- renomeado para "model_parse_failure".
  • B7 (achado nesta rodada, escrevendo o teste do item 3 "veredito nunca Approve" pedido pela revisão): o "pull-down" de result.Verdict para "comment" (B1-B5) nunca tinha efeito nenhum no --pr. reviewVerdictForLanguage (o texto "Verdict:" do corpo publicado) e formalReviewEvent (a ação formal real do GitHub, APPROVE/COMMENT/ REQUEST_CHANGES) nunca liam result.Verdict -- cada uma rederivava seu próprio veredito só de Issues/Suggestions/quality_degraded, sem nenhum conhecimento do gate. Um PR bloqueado ou inconclusivo pelo gate podia publicar "Verdict: Approve" no corpo E a ação real "APPROVE" no GitHub -- exatamente "aprovado com o defeito presente". Corrigido com duas linhas (uma em cada função, mesma prioridade que o checagem de quality_degraded já usa): if result.Verdict == "comment" { return copy.comment / "COMMENT" }. Essa correção saiu do escopo "só testes" que a rodada pediu porque o teste do item 3, escrito como pedido, provou que a premissa (o pull-down já funcionava) estava errada -- reportado aqui em vez de escondido atrás de um teste reescrito para passar.

Declarado, não corrigido

Uma falha de transporte/provedor genuína em --pr (llm.ErrAllProvidersFailed, llm.ErrBudgetExceeded) ainda retorna 1 ANTES do gate ser avaliado em runPRReview -- o gate nunca a vê, nenhum status é publicado, e gate.inconclusive não tem efeito sobre ela. Dado o prazo, esta entrega não estende essas duas saídas antecipadas para passar pelo gate; fazer isso corretamente exige decidir se --limite/um provedor totalmente fora do ar devem continuar sendo refusals antes de qualquer publicação (hoje) ou se tornam mais um motivo de gateInconclusiveReason com status/exit code próprios -- uma escolha de design que merece sua própria revisão, não uma correção apressada dentro de um prazo de 50 minutos. Fica nomeado no comentário de runPRReview (pr.go) exatamente onde esse desvio acontece.

M13b (pedido pela revisão, item 2): não matado. TestAUR519PartialCoverageNeverCachedUnderBlock passa com e sem o guard !cachePartial em main.go ~981. Investigado: um arquivo coberto por ignore nunca chega a diff.Files/toSend (removido por config.FilterIgnoredPaths antes da partição do cache), então o cache nunca grava uma entrada PARA ELE; a entrada que É gravada (para o arquivo que FOI revisado por completo, ex. app.go) está correta e é segura de reusar independente da cobertura de outro arquivo. O único jeito de code_files_partial/code_files_omitted (a metade do sinal de cobertura que REALMENTE describe um arquivo parcialmente enviado e cacheado) ficar não-zero é via truncamento por orçamento de token dentro do prompt builder -- e internal/review/reviewer.go nunca copia essas duas chaves de promptParts.Meta para result.Metadata (lacuna pré-existente, não deste card). Sem essa chave populada, cachePartial só fica verdadeiro pelo lado Ignored/Filtered, que não corresponde a um risco de cache real. O teste fica como regressão válida (prova que cobertura parcial continua bloqueando em replay), mas M13b especificamente não é morto por ele; matá-lo de verdade exigiria primeiro fechar a lacuna do reviewer.go, fora do escopo desta rodada.

Atualização desta rodada (correções B-V e B-C):

  • B-C corrigida: internal/review/reviewer.go agora copia code_files_total/complete/partial/omitted de promptParts.Meta para result.Metadata, sobrescrevendo qualquer valor que o próprio modelo tenha tentado colocar lá (essas quatro chaves também são varridas em internal/prompt/parser.go, no mesmo lugar que já varre parse_mode). Teste: TestAUR519ReviewerCopiesCoverageMetadata (internal/review), com um MaxTokens minúsculo forçando a omissão real de um arquivo — confirmado por mutação manual que falha sem a cópia.
  • M13b continua vivo, agora por um motivo diferente e documentado: mesmo com reviewer.go corrigido, o CLI não tem nenhuma flag ou variável de ambiente que exponha MaxTokens — toda chamada em cmd/aurumcode (main.go, pr.go, profiles.go) usa review.DefaultConfig() sem overrides, e MaxTokens: 0 significa "sem orçamento" (nunca omite nada, internal/prompt/builder.go:285). Isso significa que, hoje, code_files_omitted/code_files_partial são SEMPRE "0" em qualquer execução real do CLI — a correção de B-C é real e necessária para qualquer chamador que informe um MaxTokens (uso direto de internal/review.Reviewer, ou uma futura flag), mas o próprio CLI não tem como hoje produzir o cenário "orçamento parcial" que tornaria cachePartial (main.go ~981) dependente de alguma coisa além de Ignored/Filtered. Fechar M13b de verdade exigiria acrescentar essa flag/variável — fora do escopo de uma rodada de correção de bugs.
  • B-V corrigida: o "pull-down" de B1-B5 (result.Verdict = "comment") só disparava quando o PRÓPRIO modelo dizia "approve" — um modelo respondendo "" ou "changes_requested" (ambos aceitos por validateReviewResult, internal/prompt/parser.go) com o gate falho/inconclusivo ainda publicava APPROVE. Meu conserto anterior (B7) tinha o problema oposto: checava result.Verdict == "comment" direto em reviewVerdictForLanguage/formalReviewEvent, o que regredia o caso sem gate nenhum (um modelo que legitimamente responde "comment" sem achados passou a publicar COMMENT em vez do APPROVE que essas funções sempre devolveram antes deste card). Removidos os dois checks de result.Verdict; agora, logo depois da avaliação do gate, result.Metadata[prompt.PolicyGateWithheldKey] ("true") é setado sempre que gateResult.Fail || gateResult.Inconclusive, independente do que o modelo tenha dito -- essa chave é varrida de qualquer JSON do modelo no parser (mesmo padrão de parse_mode), então o modelo não pode forjá-la nem apagá-la. reviewVerdictForLanguage/formalReviewEvent/canonicalVerdict (esta última so delega a formalReviewEvent) checam essa chave na mesma prioridade que já checam quality_degraded; render.Summary (--base, fora dos paths deste card) continua lendo result.Verdict diretamente, então main.go/pr.go também continuam forçando result.Verdict para "comment" quando era ""/"approve" -- só para o render.Summary que não pode ser tocado, nunca mais como a checagem de autoridade. Testes: TestAUR519VerdictWithheldAcrossModelVerdicts (--pr, corpo + ação formal, três verdicts do modelo), TestAUR519BaseVerdictWithheldAcrossModelVerdicts (--base, mesma matriz), TestAUR519NoGatePublishesApproveDespiteModelCommentVerdict (sem gate, verdict do modelo "comment", ainda publica APPROVE -- prova exatamente a regressão do B7 anterior). Todas as mutações (M6, mais a reintrodução literal do bug do B7) confirmadas manualmente.
  • Levantamento dos outros cards: go test ./... inteiro continua verde. Das acceptance scripts de outros cards (AUR-4xx/5xx) que tocam cmd/aurumcode, as que aceitaram rodar deram: AUR-476 (o próprio AUR-476) pass, AUR-508 pass, AUR-517 pass, AUR-518 pass, AUR-402 pass. AUR-423/427/433/447/458/449/450 falharam por infraestrutura pré-existente e não relacionada (artefatos de outros cards ausentes neste worktree reconstruído, ou GOPROXY=off bloqueando uma dependência externa de um card antigo). AUR-467 e AUR-477 falham com o MESMO erro de orçamento de token COM e SEM as mudanças desta rodada (confirmado via git stash/git stash pop) -- pré-existente, não é regressão.

Mecanismo (internal/review/skillrules.go)

ParseSkillSections(skillFile, content, origin) []Rule lê cada ## e devolve uma Rule com id <base>#<slug>, título, descrição (limitada a 400 bytes), severidade (warning por padrão, ou a declarada na primeira linha do corpo como severity: error|warning|info) e Origin ("policy" ou "repo"). resolveRule resolve primeiro contra o catálogo embutido, depois contra esse conjunto dinâmico — o catálogo embutido nunca pode ser sombreado. enforceRuleCitations (internal/review/reviewer.go) aceita esse catálogo extra via Reviewer.SetDynamicRules; Reviewer.SetRuleCatalog repassa o catálogo fundido (embutido + dinâmico) ao PromptBuilder, para que o modelo seja ensinado exatamente os ids que o gate vai aceitar. Testes: TestAUR519ParseSkillSections (AC-007: uma seção nova vira citável sem mudança de código), TestAUR519EnforceRuleCitationsDynamic (AC-002: uma citação a uma seção que não existe é descartada e contada como não vinculada).

Configuração (internal/config/gate.go, central.go)

GateConfig.Threshold() normaliza fail_on reaproveitando o vocabulário de severidade de --fail-on (high|error, medium|warning, low|info), mais o alias critical que o próprio exemplo do card usa, mapeado no mesmo posto de high/error. InconclusiveMode() aceita block/warn (e bloquear/alertar). Config.Gate é validado em Parse. ApplyCentralPolicy estende a mesma precedência que já aplica a Rules/Ignore: sob política, só o Gate da política vale; um Gate do repositório é descartado com um aviso nomeado. Sem política, o Gate do repositório é seu próprio opt-in explícito (AC-005), inalterado. Testes: TestAUR519GateConfigThreshold, TestAUR519GateConfigInconclusive, TestAUR519ApplyCentralPolicyGate.

Parse degradado à prova de forja (internal/prompt/parser.go)

ParseModeKey/ParseModeDegraded/IsDegradedParse(result) expõem que um ParseReviewResponse veio do fallback de texto livre e não de uma resposta JSON válida. O decode de sucesso apaga qualquer valor que o próprio modelo tenha colocado em metadata.parse_mode antes de qualquer metadado do motor ser escrito — sem isso, uma resposta JSON válida e comum poderia forjar "metadata":{"parse_mode":"degraded"}. Teste: TestAUR519DegradedParseDetection. internal/review/reviewer.go foi unificado para usar esse único predicado em vez de manter sua própria cópia literal do aviso (ver nota de merge abaixo).

Núcleo de decisão (cmd/aurumcode/policygate.go)

evaluateGate(gate, acceptedOrigin, dynamic, issues, inconclusiveReason) decide, sem tocar stdout/stderr nem o cliente do GitHub: inconclusivo (quando inconclusiveReason != "") decide primeiro — bloqueia ou só alerta conforme gate.inconclusive, igual à prioridade que AUR-458 já dá a "não revisado" sobre "revisou e achou algo"; senão, qualquer achado na severidade do limiar ou acima, cuja regra dinâmica seja da origem aceita (policy sob política, repo no opt-in do repositório), reprova e nomeia a skill e a seção (AC-001). policyGateContext = "aurumcode/policy-gate" é o nome estável de status (AC-006), distinto do checkContext ("aurumcode/review") que AUR-439 já publica.

Ligação ao CLI (cmd/aurumcode/main.go, pr.go, policygate.go)

Antes da chamada ao modelo: dynamicRulesFromLocalSkills (política e repositório locais, --base; política local no --pr) e dynamicRulesFromRemoteSkills (repositório via API de conteúdo no ref base, --pr, o mesmo ref que loadPullRequestContext já usa) constroem o conjunto dinâmico; mergeDynamicRules funde política e repositório com a política sempre vencendo um conflito de id (CR-TRUST-001: um repositório não pode sombrear o id de uma regra da política); mergedRuleCatalogIDs funde com o catálogo embutido para Reviewer.SetRuleCatalog. Depois do resultado pronto: gateInconclusiveReason é derivado, em ordem de prioridade, de qualityFailed/qualitySkipped (--base) ou qualityDegraded (--pr), depois prompt.IsDegradedParse(result) (AC-008), depois coverageBreakdown.partial() do AUR-476 (AC-004); evaluateGate roda com o Gate já resolvido por ApplyCentralPolicy e a origem aceita (policy sob política central, repo no opt-in); as linhas da decisão entram em result.Limitations antes do relatório --base ser impresso e antes do corpo do PR ser montado; o veredito é tirado de "approve" quando o gate falha ou é inconclusivo; o código de saída reaproveita exitFindings/ exitQualityNotReviewed (nenhum código novo). No --pr, publishPolicyGateStatus publica aurumcode/policy-gate via SetStatus, independente do aurumcode/review que --check já publica, e só quando o gate foi declarado (sem gate, nenhuma chamada extra à API é feita).

Sem gate: em lugar nenhum da configuração, toda a cadeia é um no-op: todo o go test ./... do repositório (nenhum teste alterado) continua verde sem nenhuma mudança de bytes, e TestAUR519NoGateConfiguredStaysUntouched fixa isso como regressão.

--perfis (cmd/aurumcode/profiles.go): runProfilePasses constrói um Reviewer por perfil selecionado (um model pass cada); cada um deles recebe SetDynamicRules(dynamicRules)/SetRuleCatalog(ruleCatalogIDs) com o MESMO conjunto que o caminho de um único reviewer já instala, computado uma vez em runReview antes da ramificação profilesApplied. Sem isso, uma citação de seção de skill sob --perfis era descartada como rule_id desconhecido antes do gate nunca a ver — "aprovado com o defeito presente" só por --perfis ter sido selecionado, o pior defeito possível do produto. Prova em TestAUR519GateSeverityBreachFailsCheckWithProfiles: o mesmo cenário de AC-001, agora com --perfis seguranca,solid, reprova o check exatamente como sem perfis. Confirmado por mutação manual (remover a instalação faz o teste ir a RED com exit 0 em vez de exitFindings).

Testes de comportamento fim a fim (cmd/aurumcode/aur519_e2e_test.go)

Rodam o comando real via AURUMCODE_LLM_FIXTURE (mesmo padrão de aur476_test.go/aur518_test.go):

  • TestAUR519GateSeverityBreachFailsCheck (AC-001): achado citando uma seção de skill do repositório (opt-in, sem política) na severidade do limiar reprova o check (exitFindings) e nomeia skill+seção no resultado.
  • TestAUR519GateInconclusiveProviderFailureBlocks (AC-003, âncora de MUT-001): com gate.inconclusive: block, um provedor configurado que falha reprova o check e deixa a linha "policy gate: review inconclusive (provider_failure)" no resultado; o veredito nunca aparece como aprovado.
  • TestAUR519GatePartialCoverageInconclusiveWarns (AC-004): cobertura parcial do AUR-476 com gate.inconclusive: warn passa (exit 0) mas com um alerta visível e o veredito tirado de "approve".
  • TestAUR519PolicyGateStatusContextIsStable (AC-006): o nome do status ("aurumcode/policy-gate") é fixado contra mudança acidental, e publishPolicyGateStatus não publica nada quando o gate não foi declarado.
  • TestAUR519NoGateConfiguredStaysUntouched: regressão explícita pedida pelo coordenador — sem gate:, nenhum rastro novo aparece e um veredito "approve" limpo continua publicando sem alteração.
  • TestAUR519GateSeverityBreachFailsCheckWithProfiles: o gate é idêntico com e sem --perfis — a mesma citação de AC-001 reprova o check mesmo com dois perfis selecionados.
  • TestAUR519SkillsConfiguredNoGateStaysSafe: skills configuradas sem gate: nenhum continuam exit 0 — o catálogo crescer nunca vira um exit 2 de orçamento para um repositório que não pediu gate algum.
  • TestAUR519DegradedParseNeverCached (B3, prova também AC-008 fim a fim sob --base): a mesma fixture degradada roda duas vezes contra o mesmo cache; as duas devem reprovar o gate (gate.inconclusive: block) — nunca uma "aprovação" na segunda por causa do cache.
  • TestAUR519PRGateWarnStillFailsOnBreach (B1 fim a fim em --pr): PR com cobertura parcial forçada (ignore: ["tests/**"]) e um achado citando uma seção de skill do repositório na severidade do limiar, sob gate.inconclusive: warn — reprova (exitFindings) e o status aurumcode/policy-gate publicado é failure, nomeando a quebra.

Testes unitários novos desta rodada de correção: TestAUR519EvaluateGateWarnStillFailsOnBreach (B1), TestAUR519EvaluateGateRuleSeverityFloorsModel (B4), TestAUR519EvaluateGateFailOnWithoutInconclusiveNeverApproves (B5, inclui o status publicado), TestAUR519MergeDynamicRulesPolicyWins e TestAUR519ResolveRuleBuiltinWinsOverDynamic (precedência), TestAUR519ProfilePassesCarryDegradedMetadata (B2).

MUT-001

"Tratar falha do provedor como ausência de achados reprova AC-003." A mutação (tests/acceptance/AUR-519.sh AC-003-MUT-001) apaga exatamente a linha gateInconclusiveReason = "provider_failure" em runReview (cmd/aurumcode/main.go) — o switch cai para "", evaluateGate nunca entra no ramo inconclusivo para esse cenário, e TestAUR519GateInconclusiveProviderFailureBlocks vai a RED porque a linha "policy gate: review inconclusive (provider_failure)" deixa de aparecer. Confirmado manualmente.

Non-goals (do card, preservados)

Decidir o default de gate.inconclusive/gate.fail_on continua sendo da política (nenhum default é inventado aqui); nenhuma skill de convenção do repositório ganhou autoridade de bloqueio além do opt-in explícito já descrito.

Decisões de design tomadas por ambiguidade do card

  • fail_on é uma lista; o limiar é o menor rank entre as entradas. O card mostra fail_on: [critical, high] sem dizer como uma lista se traduz num limiar único. Tratá-la como "o menor rank listado" mantém a mesma leitura de --fail-on ("nesta severidade ou acima") aplicada a várias severidades em vez de uma.
  • critical é um alias de high/error. O exemplo do card usa critical, que --fail-on não aceita. Em vez de inventar uma quarta severidade só para o gate, critical foi mapeado no mesmo posto de high/error.
  • A declaração severity: de uma seção de skill usa só o vocabulário do motor (error/warning/info), não os aliases de CI do fail_on. Mantém uma seção de skill (Markdown livre, escrita por humano) numa gramática mínima e documentada, sem um segundo vocabulário de severidade divergente.
  • Dois nomes de status distintos: checkContext ("aurumcode/review", AUR-439, achado grave) e policyGateContext ("aurumcode/policy-gate", este card, compliance por skill) — para que um ruleset da organização possa exigir um sem o outro.
  • Rule.Origin foi adicionado ao tipo Rule já existente em vez de um tipo novo, para que resolveRule/enforceRuleCitations precisem de uma única forma de regra, sem conversão entre pacotes.
  • Na colisão de id entre política e repositório, a política vence. Um repositório não pode sombrear o id de uma regra da política com sua própria cópia de origem repo e, assim, tirá-la do conjunto aceito pelo gate (mergeDynamicRules).
  • O motivo de inconclusivo tem prioridade fixa: falha/skip do provedor primeiro, depois parse degradado, depois cobertura parcial — a mesma lógica de "não revisado supera achou algo" que AUR-458 já estabeleceu, agora estendida ao gate.

Nota de merge (AUR-517)

internal/review/reviewer.go ganhou, de main, sua própria exceção de retenção de resumo para o parse degradado (uma cópia literal do aviso do parser). Unificada para usar prompt.DegradedParseSummary/ prompt.IsDegradedParse — a mesma fonte única este card já exportava — em vez de manter uma segunda cópia do texto; os testes de fixação do AUR-517 foram adaptados para continuar flagrando qualquer desvio.

Validação executada

gofmt -l <arquivos tocados>                                   # limpo
go vet ./internal/... ./cmd/aurumcode/...                      # limpo
go test -count=1 ./...                                         # todos OK (repositório inteiro)
bash tests/acceptance/AUR-519.sh all                            # AUR-519/all/pass
bash tests/acceptance/AUR-519.sh AC-001 .. AC-008               # pass
bash tests/acceptance/AUR-519.sh AC-003-MUT-001                 # pass (mutação produziu RED)
bash tests/acceptance/AUR-519.sh bogus                          # unknown-selector, exit 64

Todos executados em container via .board/bin/go-shared exec; nenhum Go no host. git merge main (trazendo AUR-517) rodou no host, como autorizado.