Pular para conteúdo

AUR-537 — Provider failure on --pr now reaches the policy gate

Status

Implemented in cmd/aurumcode/pr.go (runPRReview) and cmd/aurumcode/aur537.go. Behavior proof: cmd/aurumcode/aur537_test.go::TestAUR537*; acceptance: tests/acceptance/AUR-537.sh.

Problem

AUR-519 wired the policy gate (evaluateGate, cmd/aurumcode/policygate.go) and gate.inconclusive into both review paths. AUR-521 added the compliance audit record and SARIF document on top of that same gate decision. But on --pr, a provider/transport failure that happens while the model is actually being called — every configured provider failed in the orchestrator (llm.ErrAllProvidersFailed) or --limite refused the call before any provider was ever reached (llm.ErrBudgetExceeded) — returned exit 1 before the gate was ever reached (runPRReview, the err != nil branch right after reviewer.GenerateReviewWithContext). Concretely, for a run with a gate declared:

  • no aurumcode/policy-gate status was published at all;
  • gate.inconclusive: warn ("continue, but flag it visibly") was never honored — the run failed exactly like block would, with no visible gate decision;
  • --auditoria/--sarif, if requested, wrote nothing for this run.

--base (runReview, cmd/aurumcode/main.go) already routes this same class of failure through the gate as the inconclusive reason "provider_failure" (see TestAUR519GateInconclusiveProviderFailureBlocks, cmd/aurumcode/aur519_e2e_test.go). This card closes the matching gap on --pr, reusing that exact reason token.

Behavior

runPRReview computes gateDeclared := reviewConfig.Gate.Declared() once, right after the model call, before deciding what a provider/transport failure does:

  • llm.ErrBudgetExceeded (a pre-call --limite refusal): the existing diagnosis (reportBudgetExceeded) is always printed. Without a gate declared, its exit code is returned unchanged (AC-003). With a gate declared, the function does not return; it falls through with providerFailed = true instead.
  • llm.ErrAllProvidersFailed: the existing diagnosis (naming the model with reportModelUnavailable when --modelo was given, or the bare transport error otherwise) is always printed. Without a gate declared, the same exit code (1) is returned unchanged (AC-003), regardless of --modelo. With a gate declared, the function falls through with providerFailed = true.
  • Every other error (an unparseable response, *prompt.ParseError) is unaffected: it keeps AUR-505's own qualityDegraded/"model_parse_failure" handling, unchanged by this card.
  • Selecting a provider in the first place (selectProvider/ selectProviderForModel, earlier in the function) is a separate, earlier failure this card does not touch: a model that was never even chosen has nothing for the gate to grade either way, and that path's behavior is unchanged.

When providerFailed is set, runPRReview builds an empty *types.ReviewResult carrying Metadata["quality_degraded"] = "true" (the same key AUR-505's parse-failure path already sets) plus a new declared limitation, providerFailureNotice (cmd/aurumcode/aur537.go), naming that the review did not run. Reusing "quality_degraded" means reviewVerdictForLanguage/formalReviewEvent/reviewSummaryTextForLanguage (pr.go) already never report approval and already print "the model review did not complete" in the published body, with no new rendering branch. The function then falls through exactly like a degraded parse already does: the deterministic passes (static analysis, and with --seguranca, the security pass) still run on the diff that was already fetched, the gate's own gateInconclusiveReason switch is checked — providerFailed is the first, highest-priority case, resolving to gateReasonProviderFailure (" provider_failure", the exact token --base already uses) — and evaluateGate, publishPolicyGateStatus and writeComplianceArtifacts run unchanged, exactly as they already do for AUR-505/AUR-476's own inconclusive reasons:

  • gate.inconclusive: block → aurumcode/policy-gate publishes failure naming provider_failure; exit is exitQualityNotReviewed (AC-001).
  • gate.inconclusive: warn (or undeclared) → aurumcode/policy-gate publishes success carrying the visible inconclusive alert — never the word "aprovado" — and exit is 0 unless a real deterministic finding also crosses gate.fail_on (AC-002).
  • In both cases, --auditoria/--sarif (when requested) write the audit record and SARIF document for this run, both marking it inconclusive (SARIF's invocations[0].executionSuccessful = false).
  • With no gate: key declared anywhere, behavior is byte-identical to before this card: exit 1, the same diagnosis, no status published, and no audit/SARIF file written even when both flags were given (AC-003).

B1: --check's own aurumcode/review status must never read "success"

Independent review of the first round found a regression: publishCheckStatus (AUR-439's legacy, grave-finding-only aurumcode/review status) only read providerFailed through the pre-existing opts.exigirQualidade && qualityDegraded guard. Before AUR-537, a provider outage meant this status was never published at all (the function returned earlier), which failed closed for any branch-protection rule requiring it. After the first round, a run with --check and no --exigir-qualidade would publish aurumcode/review as a false "success" ("nenhum achado grave…") for a review that never ran — strictly worse than before.

publishCheckStatus now takes providerFailed as its own, independent parameter: when true, it always publishes failure with a description naming provider_failure, in both gate.inconclusive: block and warn, and regardless of --exigir-qualidade — warn only ever softens aurumcode/policy-gate's own status, never this one. Its own contribution to the process exit code stays 0 for this case on purpose: the real block-vs-warn exit decision is already made once, correctly, by gateResult.Fail/Breach at the bottom of runPRReview (which only sets Fail for block); letting publishCheckStatus also claim exitQualityNotReviewed here would double-count that decision and wrongly force exit 1 under warn, defeating AC-002. The published state and the exit code are deliberately two different questions.

Non-goals

  • --base (runReview) is untouched: it already had this behavior (qualityFailed → "provider_failure") before this card.
  • No change to --limite's own cost semantics, to the orchestrator's retry behavior, or to the gate's severity-threshold logic (evaluateGate, policygate.go) — this card only supplies a new trigger for machinery AUR-519/520/521 already shipped and tested.
  • --check's own aurumcode/review status now does react to providerFailed (see B1 above) — this is a deliberate change from the first round, not an unrelated non-goal. What is still untouched is its grave-finding logic for every other case (a real error-severity finding, or AUR-505's own qualityRequiredButIncomplete/--exigir-qualidade guard for a model parse failure), and aurumcode/policy-gate remains the org-facing lever for "this status being green means the review actually ran" via gate.inconclusive: block.

Acceptance

container_profile: go-unit-offline-v1; accept: ./.board/bin/oci-run --profile go-unit-offline-v1 --card AUR-537.

tests/acceptance/AUR-537.sh selectors (each AC covers both the transport failure, llm.ErrAllProvidersFailed, and the --limite pre-call refusal, llm.ErrBudgetExceeded):

  • all — runs every TestAUR537* test, then applies both AC-001-MUT-001 (transport) and AC-001-MUT-002 (--limite) mutations, each against a freshly re-seeded tree so neither confounds the other's result, and confirms each turns its own AC-001 tests red.
  • AC-001 — gate.inconclusive: block: aurumcode/policy-gate AND aurumcode/review (B1) both publish failure naming provider_failure, exit is exitQualityNotReviewed, the audit record's gate.decision/gate.reason name the failure, SARIF marks executionSuccessful=false, and the published review body says the review did not run and never reads as approved.
  • AC-002 — gate.inconclusive: warn: aurumcode/policy-gate publishes success with the inconclusive word present and the "aprovado" word absent, aurumcode/review still reads failure naming provider_failure (B1), exit is 0 — even under --exigir-qualidade's own, separate, stricter check (N2) — and the audit record's gate.decision reads inconclusive.
  • AC-003 — no gate: key anywhere: exit 1, the underlying diagnosis on stderr, no status published at all, and neither --auditoria nor --sarif writes a file.
  • AC-001-MUT-001 — restores the unconditional early return on llm.ErrAllProvidersFailed (the exact defect this card fixes); AC-001's transport tests must go RED.
  • AC-001-MUT-002 — the same mutation on the llm.ErrBudgetExceeded branch; AC-001's --limite tests must go RED.

A build failure during either mutation run is infrastructure, never treated as a passing mutation.

Unknown selector exits 64; infrastructure failures (missing go, missing source/behavior files, a mutation anchor that no longer matches) exit 79.

Security

providerFailureNotice's text is a trusted, non-model-authored constant (one of two fixed strings, chosen only by reviewLanguage): it carries no model output and needs no redaction. The diagnosis printed for the underlying transport error (err) is the orchestrator's own wrapped error text, exactly as already printed today without a gate declared — this card does not change what is printed, only whether the function keeps going afterward.