Pular para conteúdo

AUR-461: the review prompt hands the model the rule catalog

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

The defect, as measured

Run against the real gateway on 2026-08-14 over a diff with three planted defects (secret, SQL injection, command injection):

aurumcode review: 5 finding(s) discarded: 5 citing an unknown rule_id
(quality/documentation, quality/naming, quality/readability,
 security/shell-injection)
svc.py:6: [error] SQL injection vulnerability ... (rule security/sql-injection)

Six findings came back from the model; one reached the user. The command injection was found and lost: the model called it security/shell-injection, the catalog calls it security/command-injection. quality/naming misses by a prefix (quality/poor-naming exists); quality/documentation and quality/readability exist in no form.

internal/prompt/templates/review.md mentioned rule_id exactly once, in its output example, and never said which ids exist. Worse, that example taught quality/naming — the prompt was teaching the model to emit an id the AUR-434 gate discards.

The fix

The rendered prompt now carries the complete, closed list of catalog ids with the instruction to copy one verbatim, and the example was corrected to quality/poor-naming.

What the fix deliberately does not do:

  • The AUR-434 gate is untouched. An unknown rule_id is still discarded and still announced (AUR-448). tests/integration/AUR-461.go and tests/e2e/AUR-461.sh assert this with security/shell-injection itself.
  • Nothing is matched by string similarity. Rewriting shell-injection into command-injection would turn a visible model error into a wrong citation presented to the user as true — a worse defect than the one being fixed.
  • No rule was added to the catalog to accommodate an invented id.

Coupling: why the list is mirrored, not imported

internal/review/reviewer.go imports internal/prompt, so internal/prompt importing internal/review is an import cycle; and injection from the caller would mean editing internal/review, which this card does not own (read_paths). So prompt.DefaultRuleCatalog mirrors the ids as compile-time data, and tests/unit/AUR-461.go asserts set equality in both directions against review.NewRulesLoader().GetAll(). A rule added, renamed or removed in the YAML without the matching edit fails that test — which is exactly what AC-001 asks for, and what a dynamic load could never give, since it would agree with itself by construction. PromptBuilder.SetRuleCatalog is the seam for a later card that owns internal/review to pass the live loader ids down.

AC-002: the budget fails high

ValidateRuleCatalog rejects an empty, unsorted or over-budget catalog (MaxRuleCatalogTokens, currently ~4x the shipped catalog's cost), and BuildPrompt returns an error when the instructions plus the reply reservation leave no room for the diff. RenderRuleCatalog has no truncation path at all. A shortened list would make the model cite a rule that exists but was cut, and the gate would discard it: this card's defect back by another road.

Selectors, each its own program

  • tests/unit/AUR-461.go::TestAUR461 — rendered list vs. loader (set equality), every rule literal in the template resolves, over-budget catalog rejected, no <no value> in the prompt.
  • tests/integration/AUR-461.go::IntegrationAUR461 — through the real GenerateReview: every offered id survives the gate carrying its citation; security/shell-injection is still discarded and named.
  • tests/e2e/AUR-461.sh — real binary, --fail-on: a catalog id closes the gate (exit 3) and prints; the invented id leaves it open (exit 0) with the discard on stderr and no silent remap.
  • tests/acceptance/AUR-461.sh::AC-001 — shell greps the ids straight out of internal/review/rules/*.yml and requires each in a prompt printed by a generated program calling BuildPrompt; neither side can agree with itself by construction. AC-001-MUT-001 removes {{.RuleCatalog}} and requires the nominal proof to go red.