Skip to main content

Review guards

What gets reviewed

Every change merged to main is reviewed. Solo work-in-progress on agent/<role> branches is unreviewed by definition; the gate is at the PR back into main.

Pre-merge checks (automated)

Every PR must:

  • go test ./... passes.
  • go test -tags=integration ./... passes.
  • golangci-lint run is clean.
  • go vet ./... is clean.
  • gofmt -s -l . returns empty.
  • If internal/store/migrations/ changed: migration applies cleanly to an empty DB and to a fixture DB from the previous version.

These run in CI. PRs cannot merge red.

Review routing

Routing rules live in .fairway/config.toml under [[review_routes]]. For fairway's own repo:

[[review_routes]]
match = "internal/store/**"
reviewer = "arch"

[[review_routes]]
match = "internal/state/**"
reviewer = "arch"

[[review_routes]]
match = "internal/dashboard/assets/**"
reviewer = "ui"

[[review_routes]]
match = "internal/dashboard/templates/**"
reviewer = "ui"

[[review_routes]]
match = ".github/workflows/**"
reviewer = "ops"

[[review_routes]]
match = ".goreleaser.yaml"
reviewer = "ops"

[[review_routes]]
match = "docs/governance/**"
reviewer = "governance"

[[review_routes]]
match = "docs/design/**"
reviewer = "arch"

[[review_routes]]
match = "**"
reviewer = "arch" # catch-all for the v0.1 cycle

Apply with fairway route review <task-id>. By default the command derives changed paths from main_branch...HEAD and applies the first matching route. Operators can pin a route explicitly with --reviewer <role> or test routing against known paths with repeated --path <path> flags.

No self-review

The role that owns a task, or the active claimant identity when present, cannot review it. Enforced in code when routing and when inserting task_reviews.

If the catch-all reviewer is the same as the claimant, route manually:

fairway record review <task-id> --reviewer <other-role> --verdict approve --reason "manual route away from claimant"

If the independent reviewer identity is not the same string as the required review domain, record both values:

fairway record review <task-id> --reviewer ops-reviewer --domain ops --verdict approve --reason "independent ops review"

The no-self-review rule applies to the reviewer identity, while merge readiness uses the review domain when one is recorded.

Verdicts

VerdictEffect
approveTask may transition to a terminal state (subject to [gates]).
changestask_state.review_status becomes changes_requested; the task may transition to blocked only when the claimant cannot continue without external input.
rejectTask transitions to blocked; requires discussion before re-claiming.

Verdicts are recorded immutably in task_reviews. Revising a verdict means a new row.

Reopens

A terminal task can be reopened with fairway set-status --reopen <task-id> <new-state>. This writes a history row with reason = "reopen: <text>". The dashboard highlights reopened tasks in the activity feed.

Historical review debt

When config or product maturity makes older done tasks fail current review-domain checks, treat that as review debt rather than automatic approval. A sweep may inventory the missing domains and group tasks for follow-up, but it must not backfill approvals from commit history, chat summaries, or validation logs alone.

Clear historical review debt only when a required reviewer records a real review, an existing review artifact is attached and referenced by the reviewer, or governance records an explicit waiver with scope, reason, and residual risk.

Cross-role changes

A PR touching multiple [[review_routes]] matches requires approval from each matched role. The first-match-wins rule for routing applies to which queue picks it up; final merge requires all matched roles to approve.

Time bounds

  • A review awaiting verdict for > 24h shows in the dashboard "stale reviews" badge.
  • After 72h the original claimant may escalate by handing off to governance, who decides whether to reroute or push for resolution.

Reviewer responsibilities

A reviewer:

  • Reads the diff and the task definition.
  • Verifies task existence and runtime state with fairway task-detail <task-id> before treating a configured YAML queue text-search miss as a blocker. A DB-visible task can be valid even when the queue file has not been exported or refreshed; block only on real config/import/export drift or missing review artifacts.
  • Runs the acceptance checks locally when reasonable.
  • Verifies the pre-merge checks actually ran (not just claimed).
  • Requires exact verification command evidence for CI scripts, release gates, workflow YAML/JSON, generated artifacts, deploy/promotion wiring, and docs publishing changes. The evidence should name the command that was run and attach the relevant artifact or log when available.
  • Notes any coding standards violations.
  • Approves, requests changes, or rejects — does not silently leave a PR open.