hyperhive/docs/process/pr-review-gate.md
iris 07b62612b0 docs: restructure into topic subdirectories, collapse duplicated index
Per mara's go-ahead on hyperhive#3902 ("getting started is good, but
terminal rendering does not go in there i think"):

Moved 21 top-level docs/*.md files into 7 new topic subdirectories
(existing web-ui/, turn-loop/, swarm/, tools/, crates/ untouched):
  getting-started/  setup.md
  agent-lifecycle/  agent-hierarchy.md, approvals.md, persistence.md
  trust-boundary/   boundary.md, security.md
  integrations/     forge.md, matrix.md, github.md, knowledge.md
  networking/       gateway.md, network.md, snapshot-store.md
  scheduler/        jobq.md, coordinator.md, ci.md, observability.md
  process/          conventions.md, gotchas.md, pr-review-gate.md
  web-ui/           terminal-rendering.md (moved into the EXISTING dir,
                    per mara's correction to the original getting-started
                    guess -- it's UI implementation detail, not onboarding)

The physical layout now matches docs/README.md's own topical headers,
which already amounted to this taxonomy -- see the scoping comment on
the issue for the two findings that motivated this (a genuine
duplication between CLAUDE.md's old "Reading paths" list and
docs/README.md's grouped one, since drifted out of sync with each
other; and the flat layout not matching the grouping we already had).

Fixed every cross-reference this moved across the whole repo (~120
files: docs/ internal links at every depth, Rust doc comments, nix
module option docs, crate READMEs) -- verified two ways: a grep sweep
confirming zero remaining references to any old path, and a script
that resolves every markdown link in docs/**/*.md + CLAUDE.md +
README.md against the filesystem and reports anything that doesn't
exist (zero broken links).

Collapsed CLAUDE.md's "Reading paths" section (the duplicate) down to
a pointer at docs/README.md, now the single index. Rewrote
docs/README.md itself to use the new subdirectory paths and added the
one doc it was missing that CLAUDE.md's old copy had (pr-review-gate.md).

Classified all 22 docs/*.md files first via a haiku subagent (mara's
suggestion) on two axes -- proposed grouping and operator-vs-
implementation focus -- before finalizing the taxonomy; spot-checked
the report and found internal inconsistencies (its classification
table disagreed with its own summary section for a few files), so this
taxonomy is my original proposal + the one correction mara gave
directly, not a blind application of the subagent's table. The
operator-focus data it gathered is still useful for a follow-up
content pass (docs skewing 'mixed' rather than pure operator-facing),
not addressed in this PR -- structure only.

nix fmt clean, both pre-push lints clean.
2026-09-02 01:55:37 +02:00

3.1 KiB

The PR review gate

What a review verdict means, why a reviewer shouldn't wait on CI to submit one, and what "armed to auto-merge" actually signals about the human review that already happened.

The gate has (up to) three parts, and they're per-repo settings

Whether a PR can merge, and what counts toward "can", is configured per repo in its branch-protection settings — not a fact true of every hive or every repo. The pieces a repo can require:

  • CI is green — the repo's required status checks pass on the PR's current head commit, if the repo requires any.
  • Requested reviews are APPROVED — and whether a review is invalidated by a later commit ("stale") is itself a setting (Forgejo's "dismiss stale approvals" branch-protection option), not universal behavior.
  • Someone with write access has armed the PR to merge — a manual merge once the required conditions hold, or Forgejo's auto-merge (merges automatically the moment the other required conditions are met).

Where these are required, they're independent of each other. A reviewer only ever owns the review-approval piece — CI resolves (or doesn't) on its own regardless of what a review says, and merge-arming is someone else's call.

Reviewers: submit the verdict, don't gate it on CI

Submit hive-forge pr-reviews <pr> --approve or --request-changes as soon as you've finished checking the diff — don't hold it back waiting for CI to go green first. Mention CI's current state in the review body if it's relevant (e.g. "approving; nix flake check is still running"), but don't gate the formal verdict on it: CI isn't a signal a reviewer waits on, it's a separate condition that resolves independently.

What arming auto-merge actually means

Auto-merge isn't "no human ever looked at this." Whoever arms it has already judged the PR sound at a coarse level — the signal it sends is roughly "apart from maybe minor tweaks a reviewer can still catch, I think this is fine." That's the human-in-the-loop step, and it already happened. No large changes are expected to surface after that point — a reviewer's job past that point is to flag it if one does, not to assume none ever will.

The practical consequence for a reviewer: on a repo where auto-merge may already be armed before your review lands, a plain APPROVED can be the last step before the merge actually happens, with no further review pass after yours. That's a reason to actually finish checking before approving — not a reason to hesitate over every small thing. Genuine, substantive doubt (a claim you haven't verified, a real correctness question) is worth a request-changes or a clarifying comment before approving; a stray style nit isn't the same category.

Re-review after a repo requires it

Where a repo dismisses stale approvals on a new commit, a review you already gave stops counting the moment a follow-up commit lands — even one whose message reads as trivial ("just a wording fix", "just trimming comments"). Re-diff and re-verify before submitting a fresh verdict; don't take a small-sounding commit message as an accurate description of the diff.