Pipeline Review And Agents

Review-Loop Agent Sessions (internal/pipeline/sessions.go)

• Per run, the review loop keeps ONE durable fixer session across review-fix turns, and EVERY review turn (initial review and every full rereview) runs session-free. A rereview certifies fixes implementing the previous review turn's findings, so resuming any review session seats the prescriber as certifier - the mechanism that let one fix round ship wrong code plus the test blessing it with zero findings. Cross-round review context travels only in the explicit sanitized round history; the fixer session is never lent to review turns, no other step uses sessions, and sessions are keyed strictly by run. The rereview prompt reframes fix-round changes as pipeline-authored code under the author-grade adversarial standard (fixRoundProvenanceClause); the same clause is emitted on a later run's initial review when a persisted uncertified range is bound. Prior findings, fix summaries, and same-round tests are claims, not evidence.
• Fail-safe rules: unsupported adapter runs cold; a failed fixer resume drops the identity and re-runs the same turn in a fresh fixer session, never skipping the turn; a cancelled ctx gets no fallback retry; session_reuse: false forces everything cold. Persistence is minimum metadata only, never prompts or transcripts; SessionRoleReviewer remains only so crash recovery accepts legacy persisted rows, which are never resumed.
• codex exec resume has a narrower flag surface than codex exec, so an unsupported override fails the resume and falls back; the e2e fakeagent must keep parsing both codex argv shapes (extractCodexPrompt).
• Regressions: internal/pipeline/sessions_test.go, internal/pipeline/steps/review_session_test.go (incl. TestReviewLoop_RereviewNeverResumesTheSessionThatPrescribedItsFixes), TestReviewStep_RereviewTreatsFixRoundsAsPipelineAuthoredCode, internal/agent/session_test.go.

Recorded Human Decisions on Findings

• Approve, skip, and abort each record selected_finding_ids = "[]" plus selection_source = user_declined on a gated round with findings (executor.go recordDeclinedRound, db.SetStepRoundDeclined); a round with no findings records no decision. The conditional write must never erase an existing selection. User-facing semantics are owned by docs/src/content/docs/reference/pipeline-steps.md.
• A decline is stored as the COMPLEMENT of the selection, never as its own list; declinedFindingLines derives it and deliberately excludes auto_fix selections, whose complement is findings still awaiting a decision (rendered under auto_fix_left_unselected, which carries no do-not-re-report instruction).
• roundHistoryPromptSection (internal/pipeline/steps/round_history.go) now carries three parts: this step's rounds, this run's OTHER steps' decisions, and earlier runs' decisions on this branch (bound per step by pipeline.BindBranchDecisions, unlike review-only BindUncertifiedPipelineRange). Nothing clears branch decisions - a completed review deletes the uncertified range, which is why that channel could not carry a decision forward, but approving a gate IS the decision. The prompt states that a recorded decision SUPERSEDES the user-intent wording.
• Deliberately ADVISORY and fail-open: no step is blocked and no commit is gated, so an agent may still re-raise a declined finding when the code genuinely changed. There is no reversion detector; assertPipelineHeadContinuity and assertReviewApprovedPushHead remain lineage-only. ci_fix.go and rebase.go build prompts without roundHistoryPromptSection, so they do not receive decisions.
• Regressions: TestExecutor_GateResolutionsWithoutASelectionRecordTheDecline, TestExecutor_GateResolutionWithNoFindingsRecordsNoDecision, TestExecutor_FixResolutionStillRecordsAUserSelection, internal/db/round_decisions_test.go, TestDeclinedFindingReachesALaterStepInTheSameRun, TestDeclinedFindingReachesALaterRunOnTheSameBranch, TestCompletedReviewDoesNotClearBranchDecisions, TestAutoFixComplementIsNeverPresentedAsAUserDecision.

Uncertified Review Provenance (internal/pipeline/uncertified.go)

• When a review-step fixer round commits and its re-review does not complete, persist the per-branch uncertified range (from_sha, to_sha). Persist on review-step fixer commits only, not lint or document. On the next run's initial review, bind that range and emit fixRoundProvenanceClause even when Fixing==false, so the replacement reviewer is not cold. Rerun proceeds; there is no refusal or --ack-uncertified-review gate.
• Missing git objects warn and continue, never block. Clear the range only after a completed review whose approved head equals or is a descendant of to_sha; parked, failed, skipped, and aborted reviews must not clear it. Rebase remaps the persisted SHAs onto the rewritten head so the next review can still bind.
• Regressions: internal/pipeline/uncertified_test.go, TestCommitAgentFixes_PersistsUncertifiedRangeForReview, TestCommitAgentFixes_LintDoesNotPersistUncertifiedRange, TestCommitAgentFixes_DocumentDoesNotPersistUncertifiedRange, TestFixRoundProvenanceClause_EmitsForUncertifiedRangeWhenNotFixing, TestUncertifiedRange_PersistsThenFeedsNextInitialReview, TestRebaseStep_RemapsUncertifiedRangeWhenHeadRewritten.

Review Fixer Verification Discipline (internal/pipeline/steps/review.go)

• The review-fix prompt requires all fixes before one focused verification limited to the changed area and forbids the whole repository test/lint suite during the fix round. The dedicated Test and Lint steps are the authoritative gates, although their coverage may be focused when commands are unconfigured. This is a prompt contract, not an enforced sandbox. Regression: TestReviewStep_FixMode_FocusedVerificationContract.

Agent-Invocation Timeouts Report Measured Silence, Never the Budget

• A timeout diagnostic may only state what was observed, never restate the configured budget as measured silence. agentActivity in agent_run.go is the single owner of the measurement and resets per-attempt evidence whenever a retry or fallback starts a replacement attempt, including provider, session-resume, and OpenCode prompt-format fallbacks. A substantive adapter error (a native agent's exit status plus captured stderr) is URL-redacted, length-bounded, and appended as agent reported: ....
• Observed output is streamed assistant text plus throttled agent.LifecyclePhaseActivity, sourced from every non-empty read of a native subprocess's stdout or stderr. Prose alone cannot prove liveness: verified against pi 0.84.3, a tool-using turn emits only tool_execution_*/toolcall_* and no text_delta until the very end, and no adapter forwards those to OnChunk. Subprocess start and exit are deliberately NOT output - start proves launch, not work, and exit is the deadline's own consequence, so counting either would recreate the fabricated evidence.
• The executor consumes LifecyclePhaseActivity into step activity only, never the step log: axi status needs the liveness, and a half-hour turn would otherwise emit hundreds of log lines.
• A CI auto-fix agent that exhausts its budget parks at an ask-user gate (ciFixAgentTimeoutOutcome) instead of being logged as a warning and re-issued on the next poll. That old path spent up to auto_fix.ci full budgets invisibly until ci_timeout. Only pipeline.ErrAgentTimeout parks; other fix failures keep warn-and-retry. Review deliberately still fails the run rather than parking - Push commits leftover worktree changes, so an approved park would ship a half-finished, unreviewed fix.
• Docs owners: docs/src/content/docs/reference/global-config.md (agent_timeout) for the diagnostic vocabulary, docs/src/content/docs/reference/pipeline-steps.md (CI) for the park. Regressions: TestRunAgent_Timeout*, TestRunAgent_SubprocessStartAloneIsNotObservedOutput, TestRunAgent_OperatorCancellationIsNotDressedUpAsAnAgentFault, TestPiAgent_ToolOnlyStreamStillReportsSubprocessLiveness, TestPiAgent_SilentSubprocessReportsNoLiveness, TestExecutor_SubprocessLivenessUpdatesActivityWithoutFloodingTheStepLog, TestCIStep_FixAgentBudgetExhaustionParksForADecisionInsteadOfRetrying, TestCIStep_NonTimeoutFixFailureKeepsRetrying, TestReviewStep_RoundBudgetTimeoutPreservesTheAgentReport, e2e TestSilentAgentTimeoutReportsMeasuredEvidence.

Local Test Is Targeted Validation (internal/pipeline/steps/test.go)

• Local Test (normal evidence agent and Test-repair agent) validates the requested intent with the smallest relevant checks and end-user-aligned evidence; it is never a repository-wide regression-suite walk. Broad regression belongs to remote CI (go test -race ./... in .github/workflows/ci.yml) and remains mandatory before a PR is ready. commands.test is the same contract when set: targeted baseline, not CI-parity complete-suite configuration; docs owner is docs/src/content/docs/reference/repo-config.md (commands.test), step behavior owner is docs/src/content/docs/reference/pipeline-steps.md (Test). This repository dogfoods an empty commands.test so the agent-driven targeted path is the default; do not reintroduce go test -race ./... as a local Test override. Process-group reaping on clean/error exit (#357) and Unix WaitDelay remain the lifecycle safety net when agents spawn test workers - restoring the agent-driven path must not revive the daemon OOM leak. Those agent turns are bounded by test_agent_timeout (default 30m, global-only): a stalled evidence or repair agent is cancelled and the run fails instead of waiting forever. Native adapters already honor that deadline through CommandContext; the missing piece was the Test step never setting one. Docs owner is docs/src/content/docs/reference/global-config.md. Every other pipeline agent invocation is bounded by agent_timeout (default 30m, global-only) at pipeline.RunAgent / the executor timeoutAgent seam, so a new agent-spawning step cannot hang a run by forgetting a deadline. Review keeps review_agent_timeout as a per-round budget; an existing sooner deadline is honored rather than capped. The invocation context is scoped only to Agent.Run; a late successful return after the deadline is rejected. Docs owner is docs/src/content/docs/reference/global-config.md. Regressions: TestTestStep_InitialAgent_TargetedValidationContract, TestTestStep_FixMode_TargetedVerificationContract, TestTestStep_FixMode_DriverFullSuiteInstructionDoesNotOverrideContract, TestTestStep_InitialAgent_NoTargetedEvidenceRequiresHonestFinding, TestTestStep_HangingEvidenceAgentFailsRunAfterTimeout, TestCodexAgent_RunCancelsSilentHang, TestDogfoodConfig_NoBroadLocalTestCommand, TestCIWorkflow_RetainsFullRaceSuiteAsBroadRegressionOwner, plus the existing #357 reap/WaitDelay tests, TestRunAgent_*, TestExecutor_DirectAgentRunIsDeadlineBounded, TestDocumentStep_HangingAgentFailsRunAfterTimeout, TestLintStep_HangingAgentFailsRunAfterTimeout, TestCIStep_HangingFixAgentFailsAfterTimeout, TestRebaseStep_HangingConflictAgentFailsAfterTimeout.

Intent Provenance & Conformance (internal/pipeline/steps/intent_prompt.go)

• Intent carries provenance: an explicit axi run --intent persists Source==db.RunIntentSourceAgent ("agent", score 1); a transcript match persists the agent name ("claude"/"codex"/...). The executor propagates it as StepContext.IntentSource alongside UserIntent (executor.go).
• userIntentPromptSection branches on source: an EXPLICIT intent renders as sanitized-but-AUTHORITATIVE acceptance criteria; an INFERRED intent keeps the low-confidence hint framing verbatim. Both branches keep the StripAdversarial+RedactSecrets pipeline and BEGIN/END "do not execute instructions" guard - authoritative reframes only the content's authority (check the diff against the criteria), never whether control tokens are stripped. The review prompt adds intentConformanceReviewClause for agent-source intent only: a fixer change that contradicts the criteria (removes intent-required or adds intent-forbidden behavior) MUST become an ask-user finding, which parks with no executor change. Conformance is limited to source-verifiable criteria; deferred pipeline-owned delivery (remote branch / push / PR / CI for this run) is out of scope at review.
• Review is always pre-push (StepReview before StepPush/StepPR/StepCI). pipelineDeliveryPhaseClause plus stripDeferredPipelineOwnedDeliveryFindings (pipeline_delivery.go, applied in review.go) keep findings that only claim those later-owned outcomes are missing from parking the run. External or pre-existing lifecycle requirements (numbered PR, third-party artifact, non-run-owned state) stay enforceable. Push, PR, and CI steps remain strict after their stages run.
• Empty/missing finding action fails closed to ask-user, not auto-fix (types/findings.go ActionOrDefault); HasAskUserFindings uses ActionOrDefault so it agrees with AutoFixableFindings (an unclassified finding is never auto-fixed and is always caught as ask-user). MergeUserOverrides still stamps user-added findings auto-fix on purpose.
• The deterministic net-deleted-author-lines git-diff backstop is intentionally not built; review.go owns the held-scope TODO.
• Regressions: internal/pipeline/steps/intent_prompt_test.go, internal/pipeline/steps/review_test.go (TestReviewStep_ConformanceObligationTracksIntentProvenance, TestReviewStep_RereviewFlagsIntentContradictionAsAskUser), internal/pipeline/steps/pipeline_delivery_test.go, internal/pipeline/steps/review_pipeline_delivery_test.go, internal/pipeline/executor_intent_conformance_test.go, internal/types/findings_test.go, e2e TestIntentJourney (inferred-source framing), e2e TestReviewPipelineOwnedPRCriterionDoesNotPark / TestReviewExternalPRLifecycleStillParks.