Code Review, Suggested Changes, CODEOWNERS, Review Requests, and Review Discipline: Diagnostics, Failure Modes, Security, and Performance

1. The diagnostic sequence: preserve before you repair

When a PR “should have requested the owner” or “is approved but cannot merge,” do not start by deleting reviews or rewriting the branch. Preserve the PR URL/number, base/head names and OIDs, changed paths, CODEOWNERS content from the base branch, requested reviewers, submitted reviews and their commit IDs, conversation state, check rollup, and active rules. Then identify which layer disagrees with your expectation.

1. Preserve evidence: PR JSON, head/base OIDs, review list, check state, CODEOWNERS errors.
2. Identify scope: repository, base branch, head branch, paths, reviewer identities, applicable rules.
3. Inspect permissions/policy: owner write access, team visibility, required-review settings, dismissal authority.
4. Choose least destructive correction: fix one pattern, access grant, review request, or rule interpretation.
5. Verify independently: repeat parser/API/PR inspection and compare OIDs/review state.

2. Intentionally broken CODEOWNERS pattern: GitHub skips the line

Create this only in a disposable branch/repository. The intent is to exclude generated documentation with a negation pattern—but CODEOWNERS does not support !.

* @octo-owner
!docs/generated/** @octo-owner

After committing the file to the PR base/default branch, inspect the parser endpoint:

gh api   -H "Accept: application/vnd.github+json"   -H "X-GitHub-Api-Version: 2026-03-10"   "repos/$OWNER/$REPO/codeowners/errors"   --jq '.errors[] | {line,column,kind,source,suggestion,message,path}'

Interpretation: the invalid line is skipped. The fix is not to move branches, re-open the PR, or force push. Replace the unsupported rule with explicit positive ownership patterns that express the actual desired owner set, then repeat the parser query until .errors is empty.

3. Wrong CODEOWNERS location or wrong base branch

A syntactically perfect config/CODEOWNERS does nothing because config/ is not a supported location. A valid file on the head branch also does not control a PR whose base branch lacks that ownership definition. Diagnostic order: inspect .github/CODEOWNERS, root CODEOWNERS, then docs/CODEOWNERS on the base ref; only then debug patterns and users.

BASE=$(gh pr view "$PR_NUMBER" --json baseRefName --jq .baseRefName)
gh api "repos/$OWNER/$REPO/contents/.github/CODEOWNERS?ref=$BASE" --jq '{path,sha}' 2>/dev/null || true
gh api "repos/$OWNER/$REPO/contents/CODEOWNERS?ref=$BASE" --jq '{path,sha}' 2>/dev/null || true
gh api "repos/$OWNER/$REPO/contents/docs/CODEOWNERS?ref=$BASE" --jq '{path,sha}' 2>/dev/null || true

4. “Approved” but merge still blocked: enumerate gates instead of blaming GitHub

A PR can display an approval while another gate remains unsatisfied: required code-owner approval, minimum approval count, unresolved conversations, required status checks, deployment requirements, merge queue, or a ruleset restriction. reviewDecision summarizes review state; it is not a complete explanation of every merge rule.

5. New commits: distinguish policy state from substantive freshness

Suppose Alice approved head OID A, then the author pushes OID B. With stale-dismissal enabled, GitHub can dismiss the approval when the documented conditions apply. Without it, the approval may remain visible. The team still must ask whether Alice reviewed the material introduced after A. Compare the REST review commit_id with current headRefOid, then interpret the repository freshness policy.

CURRENT_HEAD=$(gh pr view "$PR_NUMBER" --json headRefOid --jq .headRefOid)

gh api   -H "Accept: application/vnd.github+json"   -H "X-GitHub-Api-Version: 2026-03-10"   "repos/$OWNER/$REPO/pulls/$PR_NUMBER/reviews"   --jq '.[] | {reviewer:.user.login,state,commit_id,submitted_at}'

printf 'current_head=%s
' "$CURRENT_HEAD"

6. Rubber-stamp approval is a control failure even when policy passes

A repository can satisfy “one approval required” with a reviewer who clicks Approve without reading. That is formal compliance without substantive review. Evidence of a healthy review includes scope-aware comments, questions about risk or operability, explicit validation of critical paths, and re-review after material updates. The remedy is not automatically “require more approvals”; two rubber stamps are still weak. Improve ownership, review guidance, and change-set size.

7. Sensitive data in review comments or screenshots is a security incident

Review conversations are durable hosted records visible according to repository access. Never paste PATs, SSH private keys, recovery codes, cloud credentials, customer secrets, production log payloads containing sensitive data, or screenshots that expose them. If a credential is exposed, revoke/rotate first; editing or deleting a comment does not make an already disclosed credential trustworthy again.

8. Review dismissal and policy changes are privileged operations

Dismissing a blocking review changes governance evidence. Depending on branch/ruleset configuration, write-level actors may be able to dismiss a blocking review, and administrators can restrict dismissal to selected actors. Editing branch protection/rulesets requires admin or an eligible custom role. Treat these as policy changes, record why they were used, and never “fix” a merge block by granting yourself broader bypass privileges in a production repository.

9. Reliability, rate limits, and performance only where they matter

Review APIs can be polled by bots, but high-frequency review-request automation can trigger secondary rate limiting. Prefer event-driven workflows/webhooks or bounded polling when automation is genuinely required. Human review latency is usually dominated by queueing, scope, and ownership—not REST response time. Reduce unnecessary reviewer fan-out before adding more automation.

10. Compact diagnostic lab

In a disposable repository: place a valid CODEOWNERS file in .github/; open a PR touching an owned path; record expected owner; temporarily introduce one unsupported pattern on the base/default branch; capture the API parser error; repair the pattern; open a fresh PR if needed to observe routing; push one new commit; compare current head OID with review commit IDs; document whether your configured freshness rule would keep, dismiss, or require a new approval.

11. Lesson summary

Review failures become tractable when you identify the exact layer: base-branch CODEOWNERS, pattern/owner validity, review routing, submitted review state, freshness, conversations, checks, or merge policy. Preserve evidence first, repair only the responsible layer, and treat secret exposure/rubber-stamping as genuine security failures.

Knowledge check

Authoritative references

• About code owners
• REST endpoint: list CODEOWNERS errors
• About pull request reviews
• Dismissing a pull request review
• About protected branches
• Available rules for rulesets
• gh pr view
• gh pr checks
• REST API endpoints for pull request reviews
• REST API versions