Pular para conteúdo

AUR-458 — exit 0 means reviewed and found nothing

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

aurumcode review used to report success for runs in which the quality review never happened. That is the worst defect class this product has: a confidently wrong "all clear". A CI job reads the exit code, not stderr, so an honest explanation on stderr does not repair a green exit.

This card fixes two things at once:

  1. The security pass survives a quality-review failure. It is a deterministic regex matcher over the diff's added lines and calls no model, so a broken provider has no business taking it down. AUR-449 already established this for a provider that was never configured; this card extends it to one that was configured and failed.
  2. The exit code says the quality half did not happen.

The classification

Situation Reviewed? Exit
Model answered, nothing found yes 0
Model answered, findings at/above --fail-on yes 3
Model answered, rule gate discarded every finding yes 0
Diff is empty yes 0
No provider configured, no --seguranca no 1
No provider configured, --seguranca alone yes 0
No provider configured, --seguranca --exigir-qualidade no 1
Provider configured, call failed (network/4xx/5xx/timeout) no 1
Response does not parse no 1
--limite refused before the call no 1
--exigir-qualidade with --pr usage error 2

Why "empty diff" is exit 0

Not reviewing anything because there was nothing to review is a complete review of an empty set. The command has always sent an empty diff to the model like any other (see runReview's cache-bypass condition, which deliberately keeps len(diff.Files) == 0 on the call path), and the outcome is a genuine, honest "nothing wrong here".

Why "the rule gate discarded everything" is exit 0

The model was called and did answer intelligibly. The gate then made a policy decision to drop findings that cite no known rule_id, and AUR-448 already made that decision loud on stderr (discard_warning). That is "reviewed, and the findings were rejected as unsubstantiated" — a different thing from "could not review". Reviewers looking for a defect here should note it is a deliberate, recorded classification.

Why --seguranca with no provider stays exit 0

aurumcode review --base X --seguranca with no credential at all is the product's only free, offline, deterministic path — the one the public demo and the self-review action run. The user asked for the deterministic pass and got exactly that, in full. Making it non-zero unasked would break those consumers in production.

A CI job that meant to get a quality review says so with --exigir-qualidade, and then the same situation exits 1. One flag, opt-in, no silent change to a published path.

Why exit 1 and not a new code

The existing taxonomy already reserves 1 for "behavioral failure", and every could-not-review path in this command (no provider, unavailable --modelo, unparseable response, --limite refusal) already returned 1. Reusing it means a pipeline that already treats 1 as "do not merge" is protected with no change. What changed is that 1 now also covers the runs where the security pass did deliver findings — runs that previously reported 0, or lost the security pass entirely.

"Did not review" outranks the --fail-on gate

if qualityFailed is checked before the threshold gate. Exit 3 is a claim that the review ran and its findings reached the threshold; half a review cannot make that claim. The security findings have already printed, so the user loses no information from the code being 1 rather than 3.

How the published paths were kept intact

Every diversion is conditional on --seguranca:

if !*seguranca {
    // exactly today's behavior, byte-identical
    return rc
}
qualityFailed = true

Without --seguranca there is nothing to fall through to, so the published refusals (empty stdout, exit 1) that AUR-430, AUR-436, AUR-443 and AUR-433 pin byte-for-byte are returned unchanged. reportQualityFailure is the pre-existing diagnosis cascade extracted verbatim, so the "return immediately" and "keep the security pass alive" callers can never print different text for the same failure.

Verified byte-identical against a binary built from the parent commit: --seguranca stdout with a fixture provider (63c649af1c90e38b473e1bd45b4152b1f96ecad17d5d9c05c17bb94df7b8240f, the hash tests/acceptance/AUR-449.sh pins), the no-provider --seguranca stdout and stderr, and plain review --base HEAD~1.

--exigir-qualidade is refused with exit 2 on the --pr path rather than silently dropped: accepting a flag that says "fail if you did not review" and then not honoring it is the exact defect class this card exists to kill.

Proof

  • cmd/aurumcode/notreviewed_test.go — the diagnosis cascade and the exit-code constants, in-package.
  • tests/unit/AUR-458.go — the exit code alone, per situation.
  • tests/integration/AUR-458.go — the security section survives on stdout and the clean-review sentence never appears.
  • tests/e2e/AUR-458.sh — the CI-shaped invocation and gate precedence.
  • tests/acceptance/AUR-458.sh — AC-001 and MUT-001.

MUT-001 restores return 0 for qualityFailed. All three selectors fall independently.