Pular para conteúdo

AUR-557 -- Gate as one pipeline of contributors shared by --base and --pr

A refactor with no change of behavior. runReview (--base) and runPRReview (--pr) each wired every gate source by hand, in two places, in the right order, inside functions of 1203 and 1033 lines. This card gives the gate one pipeline, and gives each path explicit phases.

What changed

internal/gate

Type Role
Result One run's gate outcome: what gateDecision was (Active, Fail, Inconclusive, Breach, Lines, BlockingFindings, AppliedExceptions), plus the analysis-data audit, Reason (the machine-readable inconclusive motive, comma-joined) and Trail (what each contributor added; never published). cmd/aurumcode keeps type gateDecision = gate.Result.
Run The inputs every contributor shares: context, effective config, diff, the review result, Extra deterministic findings that count toward the gate but stay out of Review.Issues (the --base security pass), language, redaction filter and writers, verified repo identity, clock.
Contributor Name(), Origin(), Apply(ctx, *Run, *Result) error.
Pipeline NewPipeline, Names, Contributors, Run. Applies contributors in declared order and records each one's footprint in Result.Trail.

The failure rule lives in Pipeline.Run, once. A contributor that returns an ordinary error does not abort and is never read as "no findings": the result becomes Active and Inconclusive, Reason gains contributor_error:<name>, a line names the failure, and under gate.inconclusive: block the result Fails (exit 1, never approved). Later contributors still run. An error wrapped with gate.Fatal aborts the pipeline and is returned unchanged: configuration errors keep their historical exit 2 (aurumcode review: gate: ...).

Contributors and their order (cmd/aurumcode/review_gate*.go)

assembleGatePipeline is the only place the pipeline is declared:

  1. exceptions -- declares that no exception can match when the repository identity could not be verified (AUR-520). The matching itself is applied by contributors 3 and 5, where a finding is judged.
  2. verdict-reuse -- reuses or stores a concluded verdict (AUR-524).
  3. policy-skills -- evaluateGate: the policy's skill sections (AUR-519, AUR-556 gate.sources).
  4. sast -- applySASTGate (AUR-548).
  5. embedded-analysis -- applyAnalysisGate (AUR-556).
  6. analysis-data -- applyAnalysisDataGate (AUR-533).
  7. dependency-track -- applyDTrackGate (AUR-550); it may replace the redaction filter and wrap the writers, which the path reads back from the Run.

This is exactly the order the two functions used. A new gate source is one Contributor in its own file and one line in assembleGatePipeline.

Phases

Both paths run: resolve inputs, run analyses, run the gate pipeline, publish and exit. A phase returns (exit, done) so every early return N of the original keeps its code.

--base --pr
entry runReview (review_base.go) runPRReview (review_pr.go)
resolve inputs review_base_inputs.go review_pr_inputs.go
analyses review_base_analysis.go, review_base_quality.go review_pr_model.go, review_pr_analysis.go, generateReview in pr.go
gate review_base_gate.go review_pr_gate.go
publish and exit review_base_publish.go review_pr_publish.go

They differ only in where the diff comes from and how the review is published (stdout report / exit code versus comments, formal review and commit statuses).

Before / after

Function Before After
runReview 1203 lines 27 lines
runPRReview 1033 lines 20 lines
largest function in cmd/aurumcode 1203 128 (runSign, untouched)

TestAUR557NoFunctionExceedsLineLimit (go/ast) fails if any production function of cmd/aurumcode exceeds 150 lines.

What stayed in cmd/aurumcode, and why

The decision logic of each source (evaluateGate, applySASTGate, applyAnalysisGate, matchException, applyAnalysisDataGate, applyDTrackGate, reuseOrStoreGateVerdict, applyStructuralCoverage) still lives in its original file (policygate.go, aur548.go, gate_sources.go, aur520.go, aur533.go, aur550.go, aur524.go, structural_coverage.go). The contributors are thin adapters that call it. Moving those functions into internal/gate would change the signature of every one of them, and the existing tests (package main) call them directly; the card forbids editing those tests. Moving them is a follow-up (together with the file renames of AUR-558). Structural coverage is not a contributor: it runs before the inconclusive reason is ranked and its notice must keep its place in Limitations ahead of the changelog and policy notices, so moving it into the pipeline would reorder the published output.

Source-anchored acceptance mutations: AUR-519.sh, AUR-537.sh, AUR-538.sh and AUR-543.sh apply sed mutations anchored on exact statements of main.go/pr.go, and may not be edited. Those statements stay where the scripts look for them, with unchanged semantics: gateInconclusiveReason = "provider_failure", cacheErr = promptDigestErr and the three-tab result.Metadata[prompt.PolicyGateWithheldKey] = "true" (shared applyGateOutcome) in main.go; the transport-failure and budget branches (rc = 1, rc := reportBudgetExceeded(stderr, limiteUSD, err) followed by if !gateDeclared {) in generateReview in pr.go.

Adaptations of existing tests

Two existing tests assert, by reading the source, that a path calls the shared passes. The assertion is unchanged; only the files it reads changed, because the bodies moved.

  • aur499_test.go TestAUR499PublishedBody: reads review_base_inputs.go and review_pr_inputs.go instead of main.go and pr.go for buildChangelogSection(.
  • passes_test.go TestAUR490SharedPasses: the same five calls per path, now asserted across the phase files that hold them.

No other existing test or acceptance script was edited.

Acceptance (tests/acceptance/AUR-557.sh)

Selector Proves
AC-001 The all selector of the existing acceptances in the subset below, run unedited on the refactored code.
AC-001-full The all selector of all 15 existing acceptances the contract lists with a script (518 519 520 521 524 537 538 543 548 549 550 551 552 555 556). AUR-553 has no acceptance program in the repository and is not run.
AC-002 TestAUR557PathsShareOnePipeline: a real --base run and a real --pr run execute the same contributor names, in the declared order, equal to assembleGatePipeline.
AC-003 internal/gate pipeline tests: declared order; a failing contributor is inconclusive and never approved in block, warn and unset modes; fatal errors abort.
AC-004 No function of cmd/aurumcode exceeds 150 lines.
AC-002-MUT-001 In a copy, the --pr path alone drops the last contributor: AC-002 must fail and AUR-550's acceptance must fail.
all AC-002, AC-003, AC-004, MUT-001 and AC-001 over the subset 519, 537, 538, 543, 524, 548, 550, 556 (the ones carrying source-anchored mutations and the gate wiring).

Timing, measured in go-shared with a shared GOCACHE (a go shim first on PATH pins it, since every nested script sets its own): the 15 all selectors take about 490 s together (518 cold 138 s, 538 107 s, the rest 8-32 s each). That is past the 450 s margin of the sealed profile's 600 s limit, so all carries the subset and AC-001-full carries every script.

The sealed profile's /tmp is a 512 MB tmpfs, which a cache shared by scripts that build mutated variants outgrows. The script therefore warms the cache once with the unmodified module, marks it, and deletes every entry created after the mark when a nested script or the mutation copy finishes. Measured with oci-run --profile go-unit-offline-v1 --card AUR-557 (all): exit_code 0 in 270 s.

Anchors realigned (card v2)

AUR-522.sh and AUR-533.sh anchored mutations on lines the refactor moved. They are realigned with the same mutation and the same expected red:

  • AUR-522.sh AC-004-MUT-001: now anchored (unique) on the mergeReviewCoverage(result.Metadata, uninspectedPRNotices(p.diff, p.verifiedDir), ...) call in review_pr_analysis.go.
  • AUR-533.sh AC-003-MUT-002: now anchored (unique) on applyAnalysisDataGate(ctx, run.Cfg.AnalysisData, mode) in review_gate_contributors.go. The call is shared by both paths, so the mutation removes it for both; the --pr tests still go red.

AC-001-full now also runs 522, 533 and 559; all adds 522 and 533 to its subset.