# Pipeline Review And Agents

> Use when changing review sessions, finding decisions, agent timeouts, local Test behavior, or intent conformance.

- Skill: `mvanhorn/pipeline-review-and-agents` (Agent Skill)
- Install (CLI): `npx skillmds@latest add mvanhorn/pipeline-review-and-agents`
- Raw SKILL.md: https://api.skillmd.com/api/skills/mvanhorn/pipeline-review-and-agents/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: DevOps & Infra
- Author: mvanhorn (https://skillmd.com/u/mvanhorn)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/mvanhorn/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`.