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:
- 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.
- 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.