dev-lead fixes-and-merges PRs it did NOT author — races the human driver (dropped a commit)

Summary

dev-lead's review-changes path acts on any PR that receives a trusted review — including PRs a human/session is actively authoring. On #1303 it fixed the bot review comments and drove a human-opened PR to merge before the human finished with it, silently dropping intended work.

Evidence (#1303, from GitHub's timeline — absolute UTC)

Time	Event
16:56	Human pushed `08794259` (3 personas), opened #1303
17:04–17:16	dev-lead pushed `f107d8ec`→`31734115`→`dc028f6c` (as donpetry-bot, "review-changes")
17:20	donpetry-bot approved `dc028f6c`; #1303 merged with 3 personas
17:34	Human added a 4th persona (`scrum-master`) — 14 min after the PR had already merged

The 4th persona never reached main (recovered later via a separate PR #1305). The human had every reason to believe the PR was still theirs to complete.

Root cause

.github/workflows/dev-lead.yml fires on pull_request_review / pull_request_review_comment for any PR (only guard: skip fork PRs). scripts/dev-lead-intent.sh emits review-changes when the reviewer is a trusted human — but it never checks the PR author. So:

a review on a human-authored PR (from advisory bots, or the human's own inline comments) → review-changes → dev-lead checks out the branch, "fixes", pushes, and the PR is driven to merge.

dev-lead has no notion of "this is not my PR." The review-changes intent is meant for a human reviewing dev-lead's OWN work — not for dev-lead to seize a human's in-flight PR.

Impact

Lost work / surprise merges on human/session-authored PRs (this incident).
Concurrent-writer races on the shared branch: two writers (human + dev-lead) → lost updates. Also seen as dev-lead pushing defects onto human PRs (feat: implement issue #747 — [Phase 1] Make the Skill Eval Report visible — surface regression/error outcomes (no longer silent) #754, The persona runtime — dispatch side of the mention framework #1293) — same trigger, milder symptom.
Erodes trust: a human can't rely on their own open PR staying as they left it.

Fix

Primary: review-changes (and any fix/push/merge path) MUST be scoped to PRs dev-lead authored — branch matches dev-lead/issue-* OR pull_request.user.login is the dev-lead bot identity. On a human-authored PR, dev-lead must skip (emit skip "not-dev-lead-authored"), leaving it to the human. Advisory-only participation on human PRs is fine; pushing/merging is not.

Defense-in-depth (even on its own PRs): the post-agent push must git fetch and rebase onto the current remote head, or --force-with-lease and abort if the head moved — so dev-lead can never overwrite a commit it did not see. Discarding an unseen commit is never acceptable.

Acceptance criteria

dev-lead does NOT push to or merge a PR whose author is not the dev-lead identity (branch not dev-lead/issue-*). Verified: a human-authored PR that gets a bot review is left untouched by dev-lead.
dev-lead-intent.sh emits a skip with a clear reason for non-dev-lead-authored PRs; bats coverage for the human-authored-PR case.
The push path refuses to discard commits it did not fetch (force-with-lease that aborts on a moved head), with a test.
Fails closed: if authorship cannot be determined, dev-lead skips rather than acts.

Persona framework — pre-rollout review findings .github#755 (fail-open family — this is a "silent surprise" of the same genre)
PR comments from maintainers are mechanically inert — they neither block merge nor reach dev-lead #1290 (comments inert), pr-review claims 'Review posted' but no review lands — silent approval-submission failure #1297 (approval reports success while doing nothing) — the review-automation trust cluster.

Dev-Lead Implementation Plan

Issue: #1311 — dev-lead fixes-and-merges PRs it did NOT author — races the human driver (dropped a commit)

Scope

Scope every dev-lead fix/push/merge intent to PRs dev-lead authored (dev-lead/issue-* head branch OR pull_request.user.login/issue.user.login == the dev-lead bot identity); emit skip "not-dev-lead-authored" (failing closed on indeterminate authorship) for everyone else, and harden the post-agent push so it aborts rather than discards a commit it never fetched.

Implementation Checklist

scripts/dev-lead-intent.sh: add is_dev_lead_authored helper (branch-match OR author == BOT_USER; returns false when indeterminate → fail closed)
scripts/dev-lead-intent.sh: gate review-changes on pull_request opened/reopened/synchronize with the authorship check
scripts/dev-lead-intent.sh: gate pull_request_review (fix-reviews / enable-auto-merge / review-changes) with the authorship check
scripts/dev-lead-intent.sh: gate the trusted-bot fix-reviews path on pull_request_review_comment (leave the human @dev-lead on-mention path advisory)
scripts/dev-lead-intent.sh: gate the trusted-bot fix-bot-comment path on issue_comment (leave on-mention + rebase-sentinel)
scripts/lib/git-push-guard.sh (new): push_no_clobber — plain FF push, retry only diverged history with --force-with-lease (aborts if remote moved), never bare --force, refuse (non-zero) on a moved head
scripts/dev-lead-fix-reviews.sh: route commit_and_push's push through push_no_clobber
Update event fixtures so routing tests represent dev-lead-authored PRs; add human-authored fixtures for the skip path

Tests to Write

tests/dev-lead/unit/test_intent_authorship.bats — human-authored PR (review, review-comment, issue-comment, opened) → skip not-dev-lead-authored; dev-lead-authored → routes normally; indeterminate authorship → skip (fail closed)
tests/dev-lead/unit/test_push_no_clobber.bats — FF push succeeds; legit rebase push when remote unchanged succeeds; remote advanced under a diverged branch → push aborts and the unseen remote commit survives; never uses bare --force
Update test_intent_stub.bats / test_intent_reviews.bats for the corrected human-PR behavior

Test Command

bats tests/dev-lead/unit/ (plus bash .dev-lead/scripts/dev-lead-lint.sh and shellcheck --severity=warning scripts/*.sh)

✅ dev-lead completion report — Issue #1311

Fix: dev-lead no longer fixes/pushes/merges PRs it did not author, and its post-agent push can never silently discard an unseen commit.

Plan execution checklist

Authorship gate — new is_dev_lead_authored() helper in scripts/dev-lead-intent.sh (branch matches dev-lead/issue-* OR PR author login == $BOT_USER).
Gate wired into every fix/push/merge routing branch: pull_request opened/reopened, pull_request synchronize, pull_request_review, pull_request_review_comment, issue_comment.
Human-authored PRs → skip "not-dev-lead-authored" (advisory on-mention and rebase-sentinel paths remain exempt — explicit human authorization).
Fail-closed: indeterminate authorship → skip (never acts).
Defense-in-depth push guard — new scripts/lib/git-push-guard.sh :: push_no_clobber(): plain push (fast-forward only) → on non-FF divergence retries with --force-with-lease → aborts if the remote advanced beyond dev-lead's last-seen ref. Never a bare --force. Wired into commit_and_push in scripts/dev-lead-fix-reviews.sh.

Test results (TDD — red first, then green)

New/changed suites (authorship + push-guard + intent routing + stub + fix-reviews): ok=155, notok=0.
Full unit suite: ok=670, notok=1 — the single failure is test_engine_mcp.bats test 28, pre-existing and unrelated (confirmed failing on a clean tree via git stash).
.dev-lead/scripts/dev-lead-lint.sh: all checks passed. shellcheck on changed scripts: clean (only pre-existing info-level notes).

Key new coverage:

test_intent_authorship.bats (8 tests): bot review/review-comment/issue-comment on a human PR → skip not-dev-lead-authored; indeterminate author → skip; bot review on a dev-lead PR → fix-reviews; dev-lead-opened PR → review-changes; human @dev-lead on-mention still routes.
test_push_no_clobber.bats (5 tests): FF push succeeds; rewritten branch leases when remote unchanged; rewritten branch ABORTS when the remote moved — the unseen commit survives; non-FF without divergence refused (remote preserved); never invokes a bare --force.

Acceptance criteria

(a) No push/merge on non-dev-lead PRs — human PR + bot review left untouched (emits skip).
(b) Intent script emits skip with a clear reason + bats coverage for the human-authored case.
(c) Push path refuses to discard unfetched commits — --force-with-lease aborts on a moved head, with a test.
(d) Fails closed on indeterminate authorship.

Files changed

scripts/dev-lead-intent.sh — is_dev_lead_authored() + 4 routing gates.
scripts/lib/git-push-guard.sh — new no-clobber push helper.
scripts/dev-lead-fix-reviews.sh — source + use push_no_clobber in commit_and_push.
Tests: test_intent_authorship.bats (new), test_push_no_clobber.bats (new), test_intent_stub.bats (updated dev-lead fixes-and-merges PRs it did NOT author — races the human driver (dropped a commit) #1311 expectation), e2e/scenarios/05-skip-anti-loop.sh (updated Part B).
Fixtures: 6 new (bot/human/indeterminate/dev-lead-opened variants); existing routing fixtures made dev-lead-authored so they still exercise the "review on dev-lead's OWN PR" path.

Notes

Existing routing fixtures encoded the buggy behavior (non-dev-lead branches routed to fix/merge). Resolved by making them dev-lead-authored (preserving routing-test intent) and adding dedicated human/indeterminate fixtures for the new skip path.
Did not commit or push — per task constraints, CI handles all git operations.

feat: implement issue #1311 — dev-lead fixes-and-merges PRs it did NO…

fix: restore trunk scripts to single-definition state — surgical dedu…

feat: implement issue #1311 — dev-lead fixes-and-merges PRs it did NO…

fix: restore trunk scripts to single-definition state — surgical dedu…

Navigation Menu

Repository navigation

dev-lead fixes-and-merges PRs it did NOT author — races the human driver (dropped a commit) #1311

Summary

Evidence (#1303, from GitHub's timeline — absolute UTC)

Root cause

Impact

Fix

Acceptance criteria

Related

Dev-Lead Implementation Plan

Scope

Implementation Checklist

Tests to Write

Test Command

✅ dev-lead completion report — Issue #1311

Plan execution checklist

Test results (TDD — red first, then green)

Acceptance criteria

Files changed

Notes

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

dev-lead fixes-and-merges PRs it did NOT author — races the human driver (dropped a commit) #1311

Description

Summary

Evidence (#1303, from GitHub's timeline — absolute UTC)

Root cause

Impact

Fix

Acceptance criteria

Related

Activity

don-petry commented on Jul 18, 2026

Dev-Lead Implementation Plan

Scope

Implementation Checklist

Tests to Write

Test Command

don-petry commented on Jul 18, 2026

✅ dev-lead completion report — Issue #1311

Plan execution checklist

Test results (TDD — red first, then green)

Acceptance criteria

Files changed

Notes

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions