[Phase 4] Close the maintainer review-gate gap — dev-lead PRs cannot be blocked by the owner (shared identity + agent-resolved threads)

Story

As the maintainer reviewing a dev-lead-authored PR,
I want my review findings to be mechanically blocking until they are addressed, and not clearable by the agent itself,
so that a maintainer objection actually holds the merge — instead of depending on the agent's goodwill to honor it.

Problem — observed on PR #1413

dev-lead's declared runtime.identity.account is don-petry — deliberately the same account the human owner uses (AGENTS.md "Agent identity & credential secrets"; #1316 set this on purpose, and pr-review is the one that uses the separate donpetry-bot machine user). That shared identity removes both maintainer blocking paths on a dev-lead PR:

CHANGES_REQUESTED is unavailable. GitHub refuses: "Can not request changes on your own pull request." The ergonomic, obviously-blocking review path simply cannot be used by the owner on any dev-lead PR.
The inline-review-thread fallback is defeated too. Inline threads normally block via required_review_thread_resolution — the documented workaround in docs/pr-review-agent/maintainer-comment-gate.md. But on feat: implement issue #1403 — [Phase 1] Write docs/agentic-interaction-model.md — the event-first standard, three trigger classes, the GITHUB_TOKEN bridge rule, and the timer contract #1413, all four maintainer threads were resolved with resolvedBy: don-petry — i.e. dev-lead resolved the maintainer's own review threads, clearing the gate, and auto-merge was enabled.

Net effect: the owner has no mechanically-blocking review path on a dev-lead PR. On #1413 the outcome was fine — dev-lead genuinely addressed every finding before resolving — but that was behaviour, not control. Had it not addressed them, nothing would have held the merge.

This is #860 normative rule 2 applied to review gates: a gate whose reset is reachable by the agent is not a gate. It is also the exact sibling of #1290 — that issue closed the issue-comment path (a maintainer finding posted as a PR comment was silent); this closes the review path. maintainer-comment-gate.sh already proves the pattern and even proves the discriminator: login alone cannot separate agent from maintainer here, so it keys on automation markers instead.

Acceptance Criteria

A maintainer review finding on a dev-lead-authored PR is mechanically blocking: while an unaddressed maintainer review thread postdates the last push, pr-review withholds its automated approval (and therefore auto-merge does not proceed), mirroring scripts/lib/maintainer-comment-gate.sh on the issue-comment path.
An agent must not clear a maintainer's review gate. dev-lead (and any agent running as the maintainer's account) must not resolve a review thread it did not author. Threads are discriminated by automation marker, not login — a thread whose originating comment carries one of our markers (, , , ) is ours and may be resolved; a marker-less thread is a maintainer finding and must not be.
Fail-closed. If thread authorship or addressed-state cannot be determined, treat it as an unaddressed maintainer finding (block) rather than as "no findings" — the same fail-closed posture and for the same reason as PR comments from maintainers are mechanically inert — they neither block merge nor reach dev-lead #1290.
The gate clears the way PR comments from maintainers are mechanically inert — they neither block merge nor reach dev-lead #1290's does: a commit pushed at or after the finding marks it addressed, so the normal fix-then-continue flow is not deadlocked. A human resolving the thread also clears it.
FORCE_REVIEW / an explicit human @mention bypasses the gate (a human in the loop is what the gate exists to obtain) — consistent with the existing gate's documented bypass.
Tests: bats coverage for marker-carrying (agent) thread → resolvable; marker-less (maintainer) thread → not resolvable by the agent and blocking; undeterminable → blocks; addressed-by-push → clears. Include a regression fixture reproducing feat: implement issue #1403 — [Phase 1] Write docs/agentic-interaction-model.md — the event-first standard, three trigger classes, the GITHUB_TOKEN bridge rule, and the timer contract #1413: a maintainer thread must not end up resolvedBy the agent identity.
The alternative fix — giving dev-lead a distinct commit/PR identity — is evaluated and the decision recorded with rationale. Note this would reverse the deliberate dev-lead regressed to committing as donpetry-bot (review-only machine user) instead of don-petry #1316 choice (dev-lead acts as the owner don-petry; pr-review acts as donpetry-bot) and has CODEOWNERS / PAT / persona-manifest consequences, so it is not assumed to be the answer; whichever path is chosen, the rejected one is documented.
The rule is added to docs/agentic-interaction-model.md as normative — an explicit extension of §7 rule 2 ("a cap whose reset is reachable by the runaway is not a cap") to review and merge gates, not just automation budgets.

Tasks / Subtasks

Extend the maintainer-gate model to review threads: a sourced helper that classifies a review thread as agent-authored (marker) vs maintainer (marker-less), and computes addressed-vs-unaddressed against the last push. (AC: test issue from agent #1, Go-live improvements for PR review agent #2, Add @claude delegation, auto-merge, and rebase handling #3, Optimize review: small-PR and incremental fast paths #4)
Wire the gate into scripts/review-one-pr.sh alongside the existing advisory-bot and maintainer-comment gates, so an unaddressed maintainer review thread withholds pr-review's approval. (AC: test issue from agent #1, feat: add Copilot engine support via REVIEW_ENGINE toggle #5)
Add the resolve-guard on the dev-lead side: never call the resolve-thread API on a marker-less thread; log the skip visibly rather than silently. (AC: Go-live improvements for PR review agent #2)
Write the bats suite including the feat: implement issue #1403 — [Phase 1] Write docs/agentic-interaction-model.md — the event-first standard, three trigger classes, the GITHUB_TOKEN bridge rule, and the timer contract #1413 regression fixture. (AC: feat: feature ideation framework — adversarial proposer/challenger/synthesizer #6)
Evaluate the distinct-identity alternative and record the decision + rationale. (AC: Workflow should skip no-op PRs in MAX_PRS count #7)
Add the normative rule to the interaction-model standard. (AC: Add Claude Code workflow #8)

Dev Notes

Model on the existing gate. scripts/lib/maintainer-comment-gate.sh + its wiring in scripts/review-one-pr.sh is the direct template: it withholds pr-review's approval (rather than trying to block the merge directly), excludes our own automation by marker, and fails closed. Reuse that shape rather than inventing a second mechanism; docs/pr-review-agent/maintainer-comment-gate.md explains the rationale and is the doc to extend.
Why marker-based, not login-based. That doc already states the reason explicitly: dev-lead and the dependency-advisory pass post as don-petry, "the same account a human maintainer uses, so login alone cannot separate the two." This story is that same discriminator applied to reviewThreads instead of issue comments.
Where the resolution happens today. dev-lead's review handling walks reviewThreads (prompts/dev-lead/fix-reviews.md, scripts/dev-lead-fix-reviews.sh) and can resolve them; it also posts an @coderabbitai resolve nudge. The guard belongs at the resolve call-site, keyed on the thread's originating comment marker.
Observed evidence to pin the regression fixture: PR feat: implement issue #1403 — [Phase 1] Write docs/agentic-interaction-model.md — the event-first standard, three trigger classes, the GITHUB_TOKEN bridge rule, and the timer contract #1413, four threads first-authored by don-petry with no automation marker, all isResolved: true, resolvedBy: don-petry, with auto-merge enabled at 2026-08-02T01:45:19Z and reviewDecision: REVIEW_REQUIRED as the only remaining gate.
Do not solve this by adding a numeric cap — §9 of the standard forbids new count-based breakers. This is a gate-integrity fix, not a budget.

Project Structure Notes

New (or extended) helper under scripts/lib/ + bats under tests/ registered in lint.yml's bats list; wiring in scripts/review-one-pr.sh and the resolve call-site in scripts/dev-lead-fix-reviews.sh; doc updates in docs/pr-review-agent/maintainer-comment-gate.md and docs/agentic-interaction-model.md.

References

scripts/lib/maintainer-comment-gate.sh and docs/pr-review-agent/maintainer-comment-gate.md (PR comments from maintainers are mechanically inert — they neither block merge nor reach dev-lead #1290 — the issue-comment sibling)
scripts/review-one-pr.sh (gate wiring), scripts/lib/advisory-review-gate.sh (the defer-approval pattern)
scripts/dev-lead-fix-reviews.sh, prompts/dev-lead/fix-reviews.md (thread walking + resolution)
docs/postmortems/2026-06-pr-860-runaway.md (learning 2 — a reset reachable by the runaway is not a cap)
AGENTS.md "Agent identity & credential secrets" (dev-lead regressed to committing as donpetry-bot (review-only machine user) instead of don-petry #1316 — why dev-lead acts as don-petry)
Observed on PR feat: implement issue #1403 — [Phase 1] Write docs/agentic-interaction-model.md — the event-first standard, three trigger classes, the GITHUB_TOKEN bridge rule, and the timer contract #1413 (epic Formalize the agentic PR-interaction model (event-first, timers-as-backstop) — clear, repeatable, enforceable #1402)

Likely target surface

scripts/lib/, scripts/review-one-pr.sh, scripts/dev-lead-fix-reviews.sh

Filed from the maintainer review of PR #1413 under epic #1402: the review-path counterpart to #1290. Surfaced when a CHANGES_REQUESTED attempt was refused ("Can not request changes on your own pull request") and the inline-thread fallback was then self-resolved by the agent.

Thank you for the detailed write-up and for explicitly connecting this to #1290 — the parallel to the issue-comment gate makes the intended shape very clear. Before implementation starts, the most important open question is AC #7: is there any preliminary lean toward or against reversing the #1316 distinct-identity decision, or should the implementer treat that as a fully open evaluation? The answer determines whether this is a scripts/lib/ gate-only fix or also requires CODEOWNERS / PAT / persona-manifest changes, and resolving it upfront prevents a wasted design pass.

Dev-Lead Implementation Plan

Issue: #1415 — [Phase 4] Close the maintainer review-gate gap — dev-lead PRs cannot be blocked by the owner (shared identity + agent-resolved threads)

Scope

Extend the #1290 marker-based maintainer-gate model from issue comments to review threads: pr-review withholds approval while an unaddressed maintainer review thread postdates the last push, and dev-lead is mechanically guarded from resolving a marker-less (maintainer) thread it did not author.

Implementation Checklist

scripts/lib/maintainer-review-thread-gate.sh — new pure helper: classify a review thread as agent-authored (automation marker) vs maintainer (marker-less), and compute unaddressed-vs-addressed against the last push; fail-closed. Mirrors maintainer-comment-gate.sh.
scripts/review-one-pr.sh — wire the gate alongside the advisory + maintainer-comment gates; rc=1 → skip (exit 100), rc=2 → fail closed (exit 1); FORCE_REVIEW bypasses.
scripts/dev-lead-fix-reviews.sh — resolve-guard at the resolve_actor_outdated_threads call-site: never call resolveReviewThread on a marker-less thread; log the skip via ::warning::.
prompts/dev-lead/fix-reviews.md — key the human-thread resolve rule on the automation marker (not login), since dev-lead posts as the owner don-petry.
.github/workflows/lint.yml — register the new bats file in the bats list.
scripts/lib/consumer-manifest.json — register the new lib under the pr-review + dev-lead surfaces.
docs/pr-review-agent/maintainer-comment-gate.md — extend to cover the review-thread sibling.
docs/agentic-interaction-model.md — add the normative rule extending §7 rule 2 to review/merge gates; record the distinct-identity alternative decision (AC Workflow should skip no-op PRs in MAX_PRS count #7).

Tests to Write

tests/dev-lead/unit/test_maintainer_review_thread_gate.bats:
- marker-carrying (agent) thread → classified resolvable
- marker-less (maintainer) thread → not resolvable by the agent, and blocking while unaddressed
- undeterminable authorship/created/push-time → blocks (fail-closed)
- addressed-by-push (commit at/after finding) → clears
- human-resolved thread → clears
- feat: implement issue #1403 — [Phase 1] Write docs/agentic-interaction-model.md — the event-first standard, three trigger classes, the GITHUB_TOKEN bridge rule, and the timer contract #1413 regression fixture: marker-less thread first-authored by don-petry must classify as maintainer (not resolvable by the agent)
- wiring assertions on review-one-pr.sh (source, skip reason, fail-closed, FORCE_REVIEW bypass) and manifest registration

Test Command

bats tests/dev-lead/unit/ (full: scripts/run-bats.sh <lint.yml list>), then bash .dev-lead/scripts/dev-lead-lint.sh

✅ Issue #1415 — Maintainer review-gate gap closed

Implemented the review-thread sibling of the #1290 issue-comment gate, so a maintainer review finding on a dev-lead-authored PR is now mechanically blocking even though dev-lead acts as the owner don-petry (the same account the human maintainer uses, #1316).

Plan execution

All acceptance criteria addressed:

AC1 — mechanically blocking. check_maintainer_review_threads (new lib) withholds pr-review's automated approval while an unresolved maintainer review thread postdates the last push, mirroring maintainer-comment-gate.sh. Wired into review-one-pr.sh right after the issue-comment gate. No code-owner approval → no auto-merge.
AC2 — agent must not resolve threads it didn't author. The resolve-guard keys on the automation marker (not login, which can't discriminate don-petry from don-petry). resolve_actor_outdated_threads now skips any marker-less thread from a maintainer-capable author with a visible ::warning::. Prompt fix-reviews.md updated to match.
AC3 — fails closed. Undeterminable authorship/createdAt/push-time → block (return 1); malformed snapshot → fail the PR (return 2 → exit 1).
AC4 — clears the PR comments from maintainers are mechanically inert — they neither block merge nor reach dev-lead #1290 way. A commit pushed at/after the finding (head committer.date ≥ thread createdAt), or a human resolving the thread — safe only because the resolve-guard makes resolution a human-only signal (the two halves are coupled).
AC5 — FORCE_REVIEW bypasses. A human @mention/force_review dispatch skips the gate (the human-in-the-loop the gate exists to obtain).
AC6 — tests + feat: implement issue #1403 — [Phase 1] Write docs/agentic-interaction-model.md — the event-first standard, three trigger classes, the GITHUB_TOKEN bridge rule, and the timer contract #1413 regression fixture. New bats suite with a fixture reproducing the four marker-less don-petry threads.
AC7 — distinct-identity alternative evaluated & recorded in agentic-interaction-model.md §12.2 (rejected: keep shared identity + gate; distinct-identity would need CODEOWNERS/PAT/persona-manifest churn for no additional safety).
AC8 — normative rule added: §12 extends §7 rule 2 ("a gate whose reset is reachable by the agent is not a gate") to review/merge gates.

The coupled two-halves design

Resolve-guard (dev-lead side) — review_thread_is_agent_authored + review_thread_login_is_excluded_bot skip marker-less maintainer threads, so the agent cannot clear a maintainer's gate. Uses the same maintainer definition as the gate (marker-less AND non-excluded-bot AND non-bot_user), so outdated advisory-bot threads (codex/coderabbit) stay freely resolvable.
Approval gate (pr-review side) — check_maintainer_review_threads withholds approval while an unresolved maintainer thread postdates the last push. Human-resolution is a safe clear only because the resolve-guard prevents agent resolution.

Test results

New gate suite: 38/38 pass (structural, classifier, bot-exclusion, gate verdicts, feat: implement issue #1403 — [Phase 1] Write docs/agentic-interaction-model.md — the event-first standard, three trigger classes, the GITHUB_TOKEN bridge rule, and the timer contract #1413 regression, review-one-pr + resolve-guard wiring).
Full dev-lead unit suite: 757 pass / 0 failures.
dev-lead-lint.sh: all checks passed (shellcheck + agent profiles + CODEOWNERS).
Sibling test_maintainer_comment_gate.bats + pr_review_canary.bats: no failures.

Files changed

New (3):

scripts/lib/maintainer-review-thread-gate.sh — gate lib (classifier, bot-exclusion helper, check_maintainer_review_threads, fetch helper, standalone entrypoint).
tests/dev-lead/unit/test_maintainer_review_thread_gate.bats — 38 tests.
tests/dev-lead/fixtures/review-threads/pr-1413-maintainer-threads.json — feat: implement issue #1403 — [Phase 1] Write docs/agentic-interaction-model.md — the event-first standard, three trigger classes, the GITHUB_TOKEN bridge rule, and the timer contract #1413 regression fixture.

Modified (7):

scripts/review-one-pr.sh — wires the gate after the PR comments from maintainers are mechanically inert — they neither block merge nor reach dev-lead #1290 comment gate (rc=1 → skip; rc=2 → fail closed).
scripts/dev-lead-fix-reviews.sh — resolve-guard on the resolve_actor_outdated_threads call-site.
prompts/dev-lead/fix-reviews.md — marker-based human-thread resolve rule.
docs/pr-review-agent/maintainer-comment-gate.md — "review-thread sibling" section.
docs/agentic-interaction-model.md — §12 normative rule + distinct-identity decision.
.github/workflows/lint.yml + scripts/lib/consumer-manifest.json — register the new bats file / lib surface.

Note on a self-caught regression

During final verification I caught that the initial resolve-guard was too broad — it applied the marker check to all outdated threads, which would have wrongly skipped resolvable outdated advisory-bot threads (an existing test_fix_reviews.bats contract). Fixed by adding review_thread_login_is_excluded_bot so the guard shares the gate's exact maintainer definition; advisory-bot threads remain resolvable, only genuine maintainer findings are held back. Also aligned the gate's shellcheck test with the CI lint invocation (--severity=warning). Both were verified green.

feat: implement issue #1415 — [Phase 4] Close the maintainer review-g…

fix(pr-review): re-evaluate at the same head after its own gate dismi…

Navigation Menu

Repository navigation

[Phase 4] Close the maintainer review-gate gap — dev-lead PRs cannot be blocked by the owner (shared identity + agent-resolved threads) #1415

Story

Problem — observed on PR #1413

Acceptance Criteria

Tasks / Subtasks

Dev Notes

Project Structure Notes

References

Likely target surface

Dev-Lead Implementation Plan

Scope

Implementation Checklist

Tests to Write

Test Command

✅ Issue #1415 — Maintainer review-gate gap closed

Plan execution

The coupled two-halves design

Test results

Files changed

Note on a self-caught regression

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[Phase 4] Close the maintainer review-gate gap — dev-lead PRs cannot be blocked by the owner (shared identity + agent-resolved threads) #1415

Description

Story

Problem — observed on PR #1413

Acceptance Criteria

Tasks / Subtasks

Dev Notes

Project Structure Notes

References

Likely target surface

Activity

github-actions commented on Aug 2, 2026

don-petry commented on Aug 2, 2026

Dev-Lead Implementation Plan

Scope

Implementation Checklist

Tests to Write

Test Command

don-petry commented on Aug 2, 2026

✅ Issue #1415 — Maintainer review-gate gap closed

Plan execution

The coupled two-halves design

Test results

Files changed

Note on a self-caught regression

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions