Repository navigation
fix(dev-lead): check out PR branches in an isolated worktree (#448) - #449
Merged
Merged
Conversation
The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 40 minutes and 35 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to check out PR branches in isolated git worktrees via a new helper script pr-worktree.sh, resolving issue #448 where switching branches could overwrite or delete the agent's own prompts and scripts. The main scripts (dev-lead-fix-ci.sh and dev-lead-fix-reviews.sh) have been updated to use this isolated checkout and to pin PROMPTS_DIR to an absolute path. Additionally, comprehensive unit tests have been added and updated. Feedback on these changes suggests registering the EXIT trap earlier in checkout_pr_in_worktree to prevent temporary directory leaks if the worktree creation fails, and adding a defensive guard before executing rm -rf on the parent directory during cleanup to avoid accidental catastrophic deletions.
don-petry
enabled auto-merge (squash)
June 7, 2026 00:42
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes dev-lead’s self-referential checkout hazard by moving PR branch checkouts into an isolated git worktree, ensuring the agent’s own prompts and scripts (living in the same repo) remain available even when operating on older PR branches.
Changes:
- Added a reusable
scripts/lib/pr-worktree.shhelper to create/enter a temporary PR worktree and clean it up on exit. - Updated
dev-lead-fix-reviews.shanddev-lead-fix-ci.shto pinPROMPTS_DIRto an absolute path before entering the worktree and to use the new worktree-based checkout. - Added/updated Bats unit tests to cover the regression and align commit-and-push tests with worktree CWD behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
tests/dev-lead/unit/test_pr_worktree.bats |
New unit tests covering absolute path pinning + worktree isolation/cleanup regression for #448. |
tests/dev-lead/unit/test_fix_reviews.bats |
Adjusts tests so the engine stub mutates files in the worktree CWD, matching the new execution model. |
scripts/lib/pr-worktree.sh |
Introduces the worktree checkout/cleanup helper and resolve_abs used by dev-lead scripts. |
scripts/dev-lead-fix-reviews.sh |
Switches PR checkout to the isolated worktree approach; pins PROMPTS_DIR before cd. |
scripts/dev-lead-fix-ci.sh |
Same worktree checkout + PROMPTS_DIR pinning changes for fix-ci runs. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Collaborator
Author
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
|
Collaborator
Author
Dev-Lead — review-changes (applied)Changes committed and pushed. |
This was referenced Jun 7, 2026
Merged
don-petry
added a commit
that referenced
this pull request
Jun 7, 2026
…450, unblocked by #449) (#451) * test(dev-lead): assert per-issue/per-PR concurrency lanes, not per-repo serialization #450 restored per-issue / per-PR concurrency lanes (the fix for issue pickups being cancelled by PR follow-up traffic, petry-projects/.github#402), but test_concurrency_config.sh still asserted the old single 'dev-lead' per-repo group from #278 — so it fails against the current dev-lead.yml (3 checks). It didn't go red on the #450 merge only because the push-to-main path filter excludes .github/workflows/dev-lead*.yml; it would fail on the next dev-lead PR. The #278 serialization existed only to stop concurrent in-place PR checkouts from corrupting the working tree. #449 (#448) removed that hazard by checking out each PR branch in an isolated git worktree, so parallel per-lane runs are now safe. Invert the assertions to require the per-issue / per-PR / ci-relay lanes and forbid a repo-wide 'dev-lead' group, and extend coverage to the reusable workflow (source of truth for all consumer stubs). Update the CI step label. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * ci(dev-lead): run concurrency-config on dev-lead workflow changes pushed to main The push-to-main path filter only watched scripts/tests/prompts, so a change to .github/workflows/dev-lead*.yml could land on main without ever running the concurrency-config test — exactly how the per-repo->per-lane mismatch stayed hidden after #450 merged. Add the dev-lead workflow files to the push paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: Claude Code Bot <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
pushed a commit
that referenced
this pull request
Jun 7, 2026
…452) dev-lead enables auto-merge (squash) by default (#447). GitHub's auto-merge is asynchronous and not gated by the workflow concurrency lane, so an approval landing while the agent is mid-edit can merge — and delete — the branch out from under the run, which then fails on push: ! [remote rejected] ... cannot lock ref ... unable to resolve reference (run 27078382804: a review-changes run committed 1s after PR #449 auto-merged). Per-PR serialization (#450) stops two dev-lead runs fighting, but can't stop GitHub's async merge. So: - scripts/lib/auto-merge.sh: hold_auto_merge disables auto-merge at the start of a modifying run; restore_auto_merge (EXIT trap) restores it on any exit, but only if this run disabled it — never newly enables it. push_with_merge_guard treats a push that fails because the PR merged/closed mid-run as a benign no-op (exit 0) instead of a red-X failure. - fix-reviews.sh / fix-ci.sh: hold auto-merge + register the restore trap before checkout, and push via push_with_merge_guard. pr-worktree.sh now chains its cleanup onto the caller's EXIT trap instead of clobbering it. - test_concurrency_config.sh: was stale (asserted the pre-#450 repo-wide group, failing against main); now asserts the per-PR/per-issue serialization lanes + cancel-in-progress:false on both the inline and reusable workflows. Tests: 284 unit tests pass (new test_auto_merge.bats; trap-chaining case added to test_pr_worktree.bats); concurrency + prompt-coverage integration pass; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
don-petry
added a commit
that referenced
this pull request
Jun 7, 2026
…452) (#453) * fix(dev-lead): hold auto-merge while working a PR + push defensively (#452) dev-lead enables auto-merge (squash) by default (#447). GitHub's auto-merge is asynchronous and not gated by the workflow concurrency lane, so an approval landing while the agent is mid-edit can merge — and delete — the branch out from under the run, which then fails on push: ! [remote rejected] ... cannot lock ref ... unable to resolve reference (run 27078382804: a review-changes run committed 1s after PR #449 auto-merged). Per-PR serialization (#450) stops two dev-lead runs fighting, but can't stop GitHub's async merge. So: - scripts/lib/auto-merge.sh: hold_auto_merge disables auto-merge at the start of a modifying run; restore_auto_merge (EXIT trap) restores it on any exit, but only if this run disabled it — never newly enables it. push_with_merge_guard treats a push that fails because the PR merged/closed mid-run as a benign no-op (exit 0) instead of a red-X failure. - fix-reviews.sh / fix-ci.sh: hold auto-merge + register the restore trap before checkout, and push via push_with_merge_guard. pr-worktree.sh now chains its cleanup onto the caller's EXIT trap instead of clobbering it. - test_concurrency_config.sh: was stale (asserted the pre-#450 repo-wide group, failing against main); now asserts the per-PR/per-issue serialization lanes + cancel-in-progress:false on both the inline and reusable workflows. Tests: 284 unit tests pass (new test_auto_merge.bats; trap-chaining case added to test_pr_worktree.bats); concurrency + prompt-coverage integration pass; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): reply with the specific fix on each review thread, then resolve (#452) Two gaps left fixed review threads stuck open and blocking re-review: 1. The review prompts told the engine to RESOLVE a fixed thread but never to REPLY with what it changed — reviewers got a silent resolve. 2. fix-reviews gated resolution to ${TRIGGERING_REVIEWER} only, so a thread fixed during a different reviewer's run (e.g. a Copilot thread fixed in a Codex-triggered run) was never resolved and blocked approval. Update the three review prompts (fix-reviews, review-changes, fix-bot-comment): - For every thread it fixes, the engine now posts a reply naming the specific file/function changed and how it addresses the concern (addPullRequestReviewThreadReply with the body passed as a GraphQL variable), THEN resolves the thread. - fix-reviews now resolves every BOT thread it fixed regardless of which reviewer triggered the run (human threads still gated to the trigger). - Rubber-duck phase verifies every fixed thread was replied-to and resolved. test_prompt_coverage.sh asserts each review prompt instructs reply-with-specifics + resolve. Full unit suite (292), prompt-coverage, and concurrency tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): restore auto-merge against the PR's current head, re-fetched at restore time (#452) Codex follow-up: the rebase intent force-pushes directly from the engine (prompts/dev-lead/rebase.md: `git push --force-with-lease`), bypassing push_with_merge_guard, so _AM_HEAD_SHA is never refreshed and the EXIT-trap restore pins --match-head-commit to the stale hold-time SHA → GitHub rejects it and auto-merge is left off. Make restore_auto_merge re-fetch the PR's CURRENT head at restore time and match against that, falling back to the hold-time SHA only if the re-fetch fails. This subsumes every head-advances-during-run path (own commits, rebase force-push, external pushes, failure-after-push) instead of patching each one. Tests: new cases for current-head preference and the re-fetch-failure fallback. Full suite 295 unit + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): expose author.__typename in the review-changes thread query too (#452) The fix-reviews OPEN_THREADS_JSON build was updated to select author.__typename (GraphQL omits the [bot] suffix from bot logins, so bots are identified by __typename == "Bot"), but the review-changes build still selected only author.login. Without __typename the review-changes engine cannot reliably tell bots from humans, so its "resolve only human threads, never bots" rule could misclassify coderabbitai / chatgpt-codex-connector / etc. as human. - Add __typename to the review-changes OPEN_THREADS_JSON query. - review-changes.md now identifies humans/bots by author.__typename (User/Bot), with an explicit note not to rely on a [bot] login suffix. - test_prompt_coverage.sh now asserts EVERY OPEN_THREADS_JSON build selects author.__typename, so a future build can't silently omit it. Full unit suite + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Jun 7, 2026
…449) * fix(dev-lead): check out PR branches in an isolated worktree (#448) The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Jun 7, 2026
…450, unblocked by #449) (#451) * test(dev-lead): assert per-issue/per-PR concurrency lanes, not per-repo serialization #450 restored per-issue / per-PR concurrency lanes (the fix for issue pickups being cancelled by PR follow-up traffic, petry-projects/.github#402), but test_concurrency_config.sh still asserted the old single 'dev-lead' per-repo group from #278 — so it fails against the current dev-lead.yml (3 checks). It didn't go red on the #450 merge only because the push-to-main path filter excludes .github/workflows/dev-lead*.yml; it would fail on the next dev-lead PR. The #278 serialization existed only to stop concurrent in-place PR checkouts from corrupting the working tree. #449 (#448) removed that hazard by checking out each PR branch in an isolated git worktree, so parallel per-lane runs are now safe. Invert the assertions to require the per-issue / per-PR / ci-relay lanes and forbid a repo-wide 'dev-lead' group, and extend coverage to the reusable workflow (source of truth for all consumer stubs). Update the CI step label. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * ci(dev-lead): run concurrency-config on dev-lead workflow changes pushed to main The push-to-main path filter only watched scripts/tests/prompts, so a change to .github/workflows/dev-lead*.yml could land on main without ever running the concurrency-config test — exactly how the per-repo->per-lane mismatch stayed hidden after #450 merged. Add the dev-lead workflow files to the push paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: Claude Code Bot <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Jun 7, 2026
…449) * fix(dev-lead): check out PR branches in an isolated worktree (#448) The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Jun 7, 2026
…449) * fix(dev-lead): check out PR branches in an isolated worktree (#448) The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Jun 7, 2026
…450, unblocked by #449) (#451) * test(dev-lead): assert per-issue/per-PR concurrency lanes, not per-repo serialization #450 restored per-issue / per-PR concurrency lanes (the fix for issue pickups being cancelled by PR follow-up traffic, petry-projects/.github#402), but test_concurrency_config.sh still asserted the old single 'dev-lead' per-repo group from #278 — so it fails against the current dev-lead.yml (3 checks). It didn't go red on the #450 merge only because the push-to-main path filter excludes .github/workflows/dev-lead*.yml; it would fail on the next dev-lead PR. The #278 serialization existed only to stop concurrent in-place PR checkouts from corrupting the working tree. #449 (#448) removed that hazard by checking out each PR branch in an isolated git worktree, so parallel per-lane runs are now safe. Invert the assertions to require the per-issue / per-PR / ci-relay lanes and forbid a repo-wide 'dev-lead' group, and extend coverage to the reusable workflow (source of truth for all consumer stubs). Update the CI step label. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * ci(dev-lead): run concurrency-config on dev-lead workflow changes pushed to main The push-to-main path filter only watched scripts/tests/prompts, so a change to .github/workflows/dev-lead*.yml could land on main without ever running the concurrency-config test — exactly how the per-repo->per-lane mismatch stayed hidden after #450 merged. Add the dev-lead workflow files to the push paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: Claude Code Bot <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
This was referenced Jun 8, 2026
don-petry
added a commit
that referenced
this pull request
Jun 23, 2026
…449) * fix(dev-lead): check out PR branches in an isolated worktree (#448) The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Jun 23, 2026
…452) (#453) * fix(dev-lead): hold auto-merge while working a PR + push defensively (#452) dev-lead enables auto-merge (squash) by default (#447). GitHub's auto-merge is asynchronous and not gated by the workflow concurrency lane, so an approval landing while the agent is mid-edit can merge — and delete — the branch out from under the run, which then fails on push: ! [remote rejected] ... cannot lock ref ... unable to resolve reference (run 27078382804: a review-changes run committed 1s after PR #449 auto-merged). Per-PR serialization (#450) stops two dev-lead runs fighting, but can't stop GitHub's async merge. So: - scripts/lib/auto-merge.sh: hold_auto_merge disables auto-merge at the start of a modifying run; restore_auto_merge (EXIT trap) restores it on any exit, but only if this run disabled it — never newly enables it. push_with_merge_guard treats a push that fails because the PR merged/closed mid-run as a benign no-op (exit 0) instead of a red-X failure. - fix-reviews.sh / fix-ci.sh: hold auto-merge + register the restore trap before checkout, and push via push_with_merge_guard. pr-worktree.sh now chains its cleanup onto the caller's EXIT trap instead of clobbering it. - test_concurrency_config.sh: was stale (asserted the pre-#450 repo-wide group, failing against main); now asserts the per-PR/per-issue serialization lanes + cancel-in-progress:false on both the inline and reusable workflows. Tests: 284 unit tests pass (new test_auto_merge.bats; trap-chaining case added to test_pr_worktree.bats); concurrency + prompt-coverage integration pass; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): reply with the specific fix on each review thread, then resolve (#452) Two gaps left fixed review threads stuck open and blocking re-review: 1. The review prompts told the engine to RESOLVE a fixed thread but never to REPLY with what it changed — reviewers got a silent resolve. 2. fix-reviews gated resolution to ${TRIGGERING_REVIEWER} only, so a thread fixed during a different reviewer's run (e.g. a Copilot thread fixed in a Codex-triggered run) was never resolved and blocked approval. Update the three review prompts (fix-reviews, review-changes, fix-bot-comment): - For every thread it fixes, the engine now posts a reply naming the specific file/function changed and how it addresses the concern (addPullRequestReviewThreadReply with the body passed as a GraphQL variable), THEN resolves the thread. - fix-reviews now resolves every BOT thread it fixed regardless of which reviewer triggered the run (human threads still gated to the trigger). - Rubber-duck phase verifies every fixed thread was replied-to and resolved. test_prompt_coverage.sh asserts each review prompt instructs reply-with-specifics + resolve. Full unit suite (292), prompt-coverage, and concurrency tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): restore auto-merge against the PR's current head, re-fetched at restore time (#452) Codex follow-up: the rebase intent force-pushes directly from the engine (prompts/dev-lead/rebase.md: `git push --force-with-lease`), bypassing push_with_merge_guard, so _AM_HEAD_SHA is never refreshed and the EXIT-trap restore pins --match-head-commit to the stale hold-time SHA → GitHub rejects it and auto-merge is left off. Make restore_auto_merge re-fetch the PR's CURRENT head at restore time and match against that, falling back to the hold-time SHA only if the re-fetch fails. This subsumes every head-advances-during-run path (own commits, rebase force-push, external pushes, failure-after-push) instead of patching each one. Tests: new cases for current-head preference and the re-fetch-failure fallback. Full suite 295 unit + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): expose author.__typename in the review-changes thread query too (#452) The fix-reviews OPEN_THREADS_JSON build was updated to select author.__typename (GraphQL omits the [bot] suffix from bot logins, so bots are identified by __typename == "Bot"), but the review-changes build still selected only author.login. Without __typename the review-changes engine cannot reliably tell bots from humans, so its "resolve only human threads, never bots" rule could misclassify coderabbitai / chatgpt-codex-connector / etc. as human. - Add __typename to the review-changes OPEN_THREADS_JSON query. - review-changes.md now identifies humans/bots by author.__typename (User/Bot), with an explicit note not to rely on a [bot] login suffix. - test_prompt_coverage.sh now asserts EVERY OPEN_THREADS_JSON build selects author.__typename, so a future build can't silently omit it. Full unit suite + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Jun 25, 2026
…449) * fix(dev-lead): check out PR branches in an isolated worktree (#448) The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Jun 25, 2026
…452) (#453) * fix(dev-lead): hold auto-merge while working a PR + push defensively (#452) dev-lead enables auto-merge (squash) by default (#447). GitHub's auto-merge is asynchronous and not gated by the workflow concurrency lane, so an approval landing while the agent is mid-edit can merge — and delete — the branch out from under the run, which then fails on push: ! [remote rejected] ... cannot lock ref ... unable to resolve reference (run 27078382804: a review-changes run committed 1s after PR #449 auto-merged). Per-PR serialization (#450) stops two dev-lead runs fighting, but can't stop GitHub's async merge. So: - scripts/lib/auto-merge.sh: hold_auto_merge disables auto-merge at the start of a modifying run; restore_auto_merge (EXIT trap) restores it on any exit, but only if this run disabled it — never newly enables it. push_with_merge_guard treats a push that fails because the PR merged/closed mid-run as a benign no-op (exit 0) instead of a red-X failure. - fix-reviews.sh / fix-ci.sh: hold auto-merge + register the restore trap before checkout, and push via push_with_merge_guard. pr-worktree.sh now chains its cleanup onto the caller's EXIT trap instead of clobbering it. - test_concurrency_config.sh: was stale (asserted the pre-#450 repo-wide group, failing against main); now asserts the per-PR/per-issue serialization lanes + cancel-in-progress:false on both the inline and reusable workflows. Tests: 284 unit tests pass (new test_auto_merge.bats; trap-chaining case added to test_pr_worktree.bats); concurrency + prompt-coverage integration pass; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): reply with the specific fix on each review thread, then resolve (#452) Two gaps left fixed review threads stuck open and blocking re-review: 1. The review prompts told the engine to RESOLVE a fixed thread but never to REPLY with what it changed — reviewers got a silent resolve. 2. fix-reviews gated resolution to ${TRIGGERING_REVIEWER} only, so a thread fixed during a different reviewer's run (e.g. a Copilot thread fixed in a Codex-triggered run) was never resolved and blocked approval. Update the three review prompts (fix-reviews, review-changes, fix-bot-comment): - For every thread it fixes, the engine now posts a reply naming the specific file/function changed and how it addresses the concern (addPullRequestReviewThreadReply with the body passed as a GraphQL variable), THEN resolves the thread. - fix-reviews now resolves every BOT thread it fixed regardless of which reviewer triggered the run (human threads still gated to the trigger). - Rubber-duck phase verifies every fixed thread was replied-to and resolved. test_prompt_coverage.sh asserts each review prompt instructs reply-with-specifics + resolve. Full unit suite (292), prompt-coverage, and concurrency tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): restore auto-merge against the PR's current head, re-fetched at restore time (#452) Codex follow-up: the rebase intent force-pushes directly from the engine (prompts/dev-lead/rebase.md: `git push --force-with-lease`), bypassing push_with_merge_guard, so _AM_HEAD_SHA is never refreshed and the EXIT-trap restore pins --match-head-commit to the stale hold-time SHA → GitHub rejects it and auto-merge is left off. Make restore_auto_merge re-fetch the PR's CURRENT head at restore time and match against that, falling back to the hold-time SHA only if the re-fetch fails. This subsumes every head-advances-during-run path (own commits, rebase force-push, external pushes, failure-after-push) instead of patching each one. Tests: new cases for current-head preference and the re-fetch-failure fallback. Full suite 295 unit + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): expose author.__typename in the review-changes thread query too (#452) The fix-reviews OPEN_THREADS_JSON build was updated to select author.__typename (GraphQL omits the [bot] suffix from bot logins, so bots are identified by __typename == "Bot"), but the review-changes build still selected only author.login. Without __typename the review-changes engine cannot reliably tell bots from humans, so its "resolve only human threads, never bots" rule could misclassify coderabbitai / chatgpt-codex-connector / etc. as human. - Add __typename to the review-changes OPEN_THREADS_JSON query. - review-changes.md now identifies humans/bots by author.__typename (User/Bot), with an explicit note not to rely on a [bot] login suffix. - test_prompt_coverage.sh now asserts EVERY OPEN_THREADS_JSON build selects author.__typename, so a future build can't silently omit it. Full unit suite + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Jun 25, 2026
…449) * fix(dev-lead): check out PR branches in an isolated worktree (#448) The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Jun 25, 2026
…450, unblocked by #449) (#451) * test(dev-lead): assert per-issue/per-PR concurrency lanes, not per-repo serialization being cancelled by PR follow-up traffic, petry-projects/.github#402), but test_concurrency_config.sh still asserted the old single 'dev-lead' per-repo group from #278 — so it fails against the current dev-lead.yml (3 checks). It didn't go red on the #450 merge only because the push-to-main path filter excludes .github/workflows/dev-lead*.yml; it would fail on the next dev-lead PR. The #278 serialization existed only to stop concurrent in-place PR checkouts from corrupting the working tree. #449 (#448) removed that hazard by checking out each PR branch in an isolated git worktree, so parallel per-lane runs are now safe. Invert the assertions to require the per-issue / per-PR / ci-relay lanes and forbid a repo-wide 'dev-lead' group, and extend coverage to the reusable workflow (source of truth for all consumer stubs). Update the CI step label. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * ci(dev-lead): run concurrency-config on dev-lead workflow changes pushed to main The push-to-main path filter only watched scripts/tests/prompts, so a change to .github/workflows/dev-lead*.yml could land on main without ever running the concurrency-config test — exactly how the per-repo->per-lane mismatch stayed hidden after #450 merged. Add the dev-lead workflow files to the push paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: Claude Code Bot <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Jun 25, 2026
…452) (#453) * fix(dev-lead): hold auto-merge while working a PR + push defensively (#452) dev-lead enables auto-merge (squash) by default (#447). GitHub's auto-merge is asynchronous and not gated by the workflow concurrency lane, so an approval landing while the agent is mid-edit can merge — and delete — the branch out from under the run, which then fails on push: ! [remote rejected] ... cannot lock ref ... unable to resolve reference (run 27078382804: a review-changes run committed 1s after PR #449 auto-merged). Per-PR serialization (#450) stops two dev-lead runs fighting, but can't stop GitHub's async merge. So: - scripts/lib/auto-merge.sh: hold_auto_merge disables auto-merge at the start of a modifying run; restore_auto_merge (EXIT trap) restores it on any exit, but only if this run disabled it — never newly enables it. push_with_merge_guard treats a push that fails because the PR merged/closed mid-run as a benign no-op (exit 0) instead of a red-X failure. - fix-reviews.sh / fix-ci.sh: hold auto-merge + register the restore trap before checkout, and push via push_with_merge_guard. pr-worktree.sh now chains its cleanup onto the caller's EXIT trap instead of clobbering it. - test_concurrency_config.sh: was stale (asserted the pre-#450 repo-wide group, failing against main); now asserts the per-PR/per-issue serialization lanes + cancel-in-progress:false on both the inline and reusable workflows. Tests: 284 unit tests pass (new test_auto_merge.bats; trap-chaining case added to test_pr_worktree.bats); concurrency + prompt-coverage integration pass; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): reply with the specific fix on each review thread, then resolve (#452) Two gaps left fixed review threads stuck open and blocking re-review: 1. The review prompts told the engine to RESOLVE a fixed thread but never to REPLY with what it changed — reviewers got a silent resolve. 2. fix-reviews gated resolution to ${TRIGGERING_REVIEWER} only, so a thread fixed during a different reviewer's run (e.g. a Copilot thread fixed in a Codex-triggered run) was never resolved and blocked approval. Update the three review prompts (fix-reviews, review-changes, fix-bot-comment): - For every thread it fixes, the engine now posts a reply naming the specific file/function changed and how it addresses the concern (addPullRequestReviewThreadReply with the body passed as a GraphQL variable), THEN resolves the thread. - fix-reviews now resolves every BOT thread it fixed regardless of which reviewer triggered the run (human threads still gated to the trigger). - Rubber-duck phase verifies every fixed thread was replied-to and resolved. test_prompt_coverage.sh asserts each review prompt instructs reply-with-specifics + resolve. Full unit suite (292), prompt-coverage, and concurrency tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): restore auto-merge against the PR's current head, re-fetched at restore time (#452) Codex follow-up: the rebase intent force-pushes directly from the engine (prompts/dev-lead/rebase.md: `git push --force-with-lease`), bypassing push_with_merge_guard, so _AM_HEAD_SHA is never refreshed and the EXIT-trap restore pins --match-head-commit to the stale hold-time SHA → GitHub rejects it and auto-merge is left off. Make restore_auto_merge re-fetch the PR's CURRENT head at restore time and match against that, falling back to the hold-time SHA only if the re-fetch fails. This subsumes every head-advances-during-run path (own commits, rebase force-push, external pushes, failure-after-push) instead of patching each one. Tests: new cases for current-head preference and the re-fetch-failure fallback. Full suite 295 unit + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): expose author.__typename in the review-changes thread query too (#452) The fix-reviews OPEN_THREADS_JSON build was updated to select author.__typename (GraphQL omits the [bot] suffix from bot logins, so bots are identified by __typename == "Bot"), but the review-changes build still selected only author.login. Without __typename the review-changes engine cannot reliably tell bots from humans, so its "resolve only human threads, never bots" rule could misclassify coderabbitai / chatgpt-codex-connector / etc. as human. - Add __typename to the review-changes OPEN_THREADS_JSON query. - review-changes.md now identifies humans/bots by author.__typename (User/Bot), with an explicit note not to rely on a [bot] login suffix. - test_prompt_coverage.sh now asserts EVERY OPEN_THREADS_JSON build selects author.__typename, so a future build can't silently omit it. Full unit suite + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Jun 25, 2026
…449) * fix(dev-lead): check out PR branches in an isolated worktree (#448) The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Jun 25, 2026
…452) (#453) * fix(dev-lead): hold auto-merge while working a PR + push defensively (#452) dev-lead enables auto-merge (squash) by default (#447). GitHub's auto-merge is asynchronous and not gated by the workflow concurrency lane, so an approval landing while the agent is mid-edit can merge — and delete — the branch out from under the run, which then fails on push: ! [remote rejected] ... cannot lock ref ... unable to resolve reference (run 27078382804: a review-changes run committed 1s after PR #449 auto-merged). Per-PR serialization (#450) stops two dev-lead runs fighting, but can't stop GitHub's async merge. So: - scripts/lib/auto-merge.sh: hold_auto_merge disables auto-merge at the start of a modifying run; restore_auto_merge (EXIT trap) restores it on any exit, but only if this run disabled it — never newly enables it. push_with_merge_guard treats a push that fails because the PR merged/closed mid-run as a benign no-op (exit 0) instead of a red-X failure. - fix-reviews.sh / fix-ci.sh: hold auto-merge + register the restore trap before checkout, and push via push_with_merge_guard. pr-worktree.sh now chains its cleanup onto the caller's EXIT trap instead of clobbering it. - test_concurrency_config.sh: was stale (asserted the pre-#450 repo-wide group, failing against main); now asserts the per-PR/per-issue serialization lanes + cancel-in-progress:false on both the inline and reusable workflows. Tests: 284 unit tests pass (new test_auto_merge.bats; trap-chaining case added to test_pr_worktree.bats); concurrency + prompt-coverage integration pass; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): reply with the specific fix on each review thread, then resolve (#452) Two gaps left fixed review threads stuck open and blocking re-review: 1. The review prompts told the engine to RESOLVE a fixed thread but never to REPLY with what it changed — reviewers got a silent resolve. 2. fix-reviews gated resolution to ${TRIGGERING_REVIEWER} only, so a thread fixed during a different reviewer's run (e.g. a Copilot thread fixed in a Codex-triggered run) was never resolved and blocked approval. Update the three review prompts (fix-reviews, review-changes, fix-bot-comment): - For every thread it fixes, the engine now posts a reply naming the specific file/function changed and how it addresses the concern (addPullRequestReviewThreadReply with the body passed as a GraphQL variable), THEN resolves the thread. - fix-reviews now resolves every BOT thread it fixed regardless of which reviewer triggered the run (human threads still gated to the trigger). - Rubber-duck phase verifies every fixed thread was replied-to and resolved. test_prompt_coverage.sh asserts each review prompt instructs reply-with-specifics + resolve. Full unit suite (292), prompt-coverage, and concurrency tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): restore auto-merge against the PR's current head, re-fetched at restore time (#452) Codex follow-up: the rebase intent force-pushes directly from the engine (prompts/dev-lead/rebase.md: `git push --force-with-lease`), bypassing push_with_merge_guard, so _AM_HEAD_SHA is never refreshed and the EXIT-trap restore pins --match-head-commit to the stale hold-time SHA → GitHub rejects it and auto-merge is left off. Make restore_auto_merge re-fetch the PR's CURRENT head at restore time and match against that, falling back to the hold-time SHA only if the re-fetch fails. This subsumes every head-advances-during-run path (own commits, rebase force-push, external pushes, failure-after-push) instead of patching each one. Tests: new cases for current-head preference and the re-fetch-failure fallback. Full suite 295 unit + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): expose author.__typename in the review-changes thread query too (#452) The fix-reviews OPEN_THREADS_JSON build was updated to select author.__typename (GraphQL omits the [bot] suffix from bot logins, so bots are identified by __typename == "Bot"), but the review-changes build still selected only author.login. Without __typename the review-changes engine cannot reliably tell bots from humans, so its "resolve only human threads, never bots" rule could misclassify coderabbitai / chatgpt-codex-connector / etc. as human. - Add __typename to the review-changes OPEN_THREADS_JSON query. - review-changes.md now identifies humans/bots by author.__typename (User/Bot), with an explicit note not to rely on a [bot] login suffix. - test_prompt_coverage.sh now asserts EVERY OPEN_THREADS_JSON build selects author.__typename, so a future build can't silently omit it. Full unit suite + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Jun 25, 2026
…449) * fix(dev-lead): check out PR branches in an isolated worktree (#448) The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Jun 25, 2026
…450, unblocked by #449) (#451) * test(dev-lead): assert per-issue/per-PR concurrency lanes, not per-repo serialization being cancelled by PR follow-up traffic, petry-projects/.github#402), but test_concurrency_config.sh still asserted the old single 'dev-lead' per-repo group from #278 — so it fails against the current dev-lead.yml (3 checks). It didn't go red on the #450 merge only because the push-to-main path filter excludes .github/workflows/dev-lead*.yml; it would fail on the next dev-lead PR. The #278 serialization existed only to stop concurrent in-place PR checkouts from corrupting the working tree. #449 (#448) removed that hazard by checking out each PR branch in an isolated git worktree, so parallel per-lane runs are now safe. Invert the assertions to require the per-issue / per-PR / ci-relay lanes and forbid a repo-wide 'dev-lead' group, and extend coverage to the reusable workflow (source of truth for all consumer stubs). Update the CI step label. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * ci(dev-lead): run concurrency-config on dev-lead workflow changes pushed to main The push-to-main path filter only watched scripts/tests/prompts, so a change to .github/workflows/dev-lead*.yml could land on main without ever running the concurrency-config test — exactly how the per-repo->per-lane mismatch stayed hidden after #450 merged. Add the dev-lead workflow files to the push paths. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: Claude Code Bot <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Jun 25, 2026
…452) (#453) * fix(dev-lead): hold auto-merge while working a PR + push defensively (#452) dev-lead enables auto-merge (squash) by default (#447). GitHub's auto-merge is asynchronous and not gated by the workflow concurrency lane, so an approval landing while the agent is mid-edit can merge — and delete — the branch out from under the run, which then fails on push: ! [remote rejected] ... cannot lock ref ... unable to resolve reference (run 27078382804: a review-changes run committed 1s after PR #449 auto-merged). Per-PR serialization (#450) stops two dev-lead runs fighting, but can't stop GitHub's async merge. So: - scripts/lib/auto-merge.sh: hold_auto_merge disables auto-merge at the start of a modifying run; restore_auto_merge (EXIT trap) restores it on any exit, but only if this run disabled it — never newly enables it. push_with_merge_guard treats a push that fails because the PR merged/closed mid-run as a benign no-op (exit 0) instead of a red-X failure. - fix-reviews.sh / fix-ci.sh: hold auto-merge + register the restore trap before checkout, and push via push_with_merge_guard. pr-worktree.sh now chains its cleanup onto the caller's EXIT trap instead of clobbering it. - test_concurrency_config.sh: was stale (asserted the pre-#450 repo-wide group, failing against main); now asserts the per-PR/per-issue serialization lanes + cancel-in-progress:false on both the inline and reusable workflows. Tests: 284 unit tests pass (new test_auto_merge.bats; trap-chaining case added to test_pr_worktree.bats); concurrency + prompt-coverage integration pass; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): reply with the specific fix on each review thread, then resolve (#452) Two gaps left fixed review threads stuck open and blocking re-review: 1. The review prompts told the engine to RESOLVE a fixed thread but never to REPLY with what it changed — reviewers got a silent resolve. 2. fix-reviews gated resolution to ${TRIGGERING_REVIEWER} only, so a thread fixed during a different reviewer's run (e.g. a Copilot thread fixed in a Codex-triggered run) was never resolved and blocked approval. Update the three review prompts (fix-reviews, review-changes, fix-bot-comment): - For every thread it fixes, the engine now posts a reply naming the specific file/function changed and how it addresses the concern (addPullRequestReviewThreadReply with the body passed as a GraphQL variable), THEN resolves the thread. - fix-reviews now resolves every BOT thread it fixed regardless of which reviewer triggered the run (human threads still gated to the trigger). - Rubber-duck phase verifies every fixed thread was replied-to and resolved. test_prompt_coverage.sh asserts each review prompt instructs reply-with-specifics + resolve. Full unit suite (292), prompt-coverage, and concurrency tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): restore auto-merge against the PR's current head, re-fetched at restore time (#452) Codex follow-up: the rebase intent force-pushes directly from the engine (prompts/dev-lead/rebase.md: `git push --force-with-lease`), bypassing push_with_merge_guard, so _AM_HEAD_SHA is never refreshed and the EXIT-trap restore pins --match-head-commit to the stale hold-time SHA → GitHub rejects it and auto-merge is left off. Make restore_auto_merge re-fetch the PR's CURRENT head at restore time and match against that, falling back to the hold-time SHA only if the re-fetch fails. This subsumes every head-advances-during-run path (own commits, rebase force-push, external pushes, failure-after-push) instead of patching each one. Tests: new cases for current-head preference and the re-fetch-failure fallback. Full suite 295 unit + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): expose author.__typename in the review-changes thread query too (#452) The fix-reviews OPEN_THREADS_JSON build was updated to select author.__typename (GraphQL omits the [bot] suffix from bot logins, so bots are identified by __typename == "Bot"), but the review-changes build still selected only author.login. Without __typename the review-changes engine cannot reliably tell bots from humans, so its "resolve only human threads, never bots" rule could misclassify coderabbitai / chatgpt-codex-connector / etc. as human. - Add __typename to the review-changes OPEN_THREADS_JSON query. - review-changes.md now identifies humans/bots by author.__typename (User/Bot), with an explicit note not to rely on a [bot] login suffix. - test_prompt_coverage.sh now asserts EVERY OPEN_THREADS_JSON build selects author.__typename, so a future build can't silently omit it. Full unit suite + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 2, 2026
…449) * fix(dev-lead): check out PR branches in an isolated worktree (#448) The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 2, 2026
…452) (#453) * fix(dev-lead): hold auto-merge while working a PR + push defensively (#452) dev-lead enables auto-merge (squash) by default (#447). GitHub's auto-merge is asynchronous and not gated by the workflow concurrency lane, so an approval landing while the agent is mid-edit can merge — and delete — the branch out from under the run, which then fails on push: ! [remote rejected] ... cannot lock ref ... unable to resolve reference (run 27078382804: a review-changes run committed 1s after PR #449 auto-merged). Per-PR serialization (#450) stops two dev-lead runs fighting, but can't stop GitHub's async merge. So: - scripts/lib/auto-merge.sh: hold_auto_merge disables auto-merge at the start of a modifying run; restore_auto_merge (EXIT trap) restores it on any exit, but only if this run disabled it — never newly enables it. push_with_merge_guard treats a push that fails because the PR merged/closed mid-run as a benign no-op (exit 0) instead of a red-X failure. - fix-reviews.sh / fix-ci.sh: hold auto-merge + register the restore trap before checkout, and push via push_with_merge_guard. pr-worktree.sh now chains its cleanup onto the caller's EXIT trap instead of clobbering it. - test_concurrency_config.sh: was stale (asserted the pre-#450 repo-wide group, failing against main); now asserts the per-PR/per-issue serialization lanes + cancel-in-progress:false on both the inline and reusable workflows. Tests: 284 unit tests pass (new test_auto_merge.bats; trap-chaining case added to test_pr_worktree.bats); concurrency + prompt-coverage integration pass; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): reply with the specific fix on each review thread, then resolve (#452) Two gaps left fixed review threads stuck open and blocking re-review: 1. The review prompts told the engine to RESOLVE a fixed thread but never to REPLY with what it changed — reviewers got a silent resolve. 2. fix-reviews gated resolution to ${TRIGGERING_REVIEWER} only, so a thread fixed during a different reviewer's run (e.g. a Copilot thread fixed in a Codex-triggered run) was never resolved and blocked approval. Update the three review prompts (fix-reviews, review-changes, fix-bot-comment): - For every thread it fixes, the engine now posts a reply naming the specific file/function changed and how it addresses the concern (addPullRequestReviewThreadReply with the body passed as a GraphQL variable), THEN resolves the thread. - fix-reviews now resolves every BOT thread it fixed regardless of which reviewer triggered the run (human threads still gated to the trigger). - Rubber-duck phase verifies every fixed thread was replied-to and resolved. test_prompt_coverage.sh asserts each review prompt instructs reply-with-specifics + resolve. Full unit suite (292), prompt-coverage, and concurrency tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): restore auto-merge against the PR's current head, re-fetched at restore time (#452) Codex follow-up: the rebase intent force-pushes directly from the engine (prompts/dev-lead/rebase.md: `git push --force-with-lease`), bypassing push_with_merge_guard, so _AM_HEAD_SHA is never refreshed and the EXIT-trap restore pins --match-head-commit to the stale hold-time SHA → GitHub rejects it and auto-merge is left off. Make restore_auto_merge re-fetch the PR's CURRENT head at restore time and match against that, falling back to the hold-time SHA only if the re-fetch fails. This subsumes every head-advances-during-run path (own commits, rebase force-push, external pushes, failure-after-push) instead of patching each one. Tests: new cases for current-head preference and the re-fetch-failure fallback. Full suite 295 unit + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): expose author.__typename in the review-changes thread query too (#452) The fix-reviews OPEN_THREADS_JSON build was updated to select author.__typename (GraphQL omits the [bot] suffix from bot logins, so bots are identified by __typename == "Bot"), but the review-changes build still selected only author.login. Without __typename the review-changes engine cannot reliably tell bots from humans, so its "resolve only human threads, never bots" rule could misclassify coderabbitai / chatgpt-codex-connector / etc. as human. - Add __typename to the review-changes OPEN_THREADS_JSON query. - review-changes.md now identifies humans/bots by author.__typename (User/Bot), with an explicit note not to rely on a [bot] login suffix. - test_prompt_coverage.sh now asserts EVERY OPEN_THREADS_JSON build selects author.__typename, so a future build can't silently omit it. Full unit suite + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 3, 2026
…449) * fix(dev-lead): check out PR branches in an isolated worktree (#448) The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 3, 2026
…452) (#453) * fix(dev-lead): hold auto-merge while working a PR + push defensively (#452) dev-lead enables auto-merge (squash) by default (#447). GitHub's auto-merge is asynchronous and not gated by the workflow concurrency lane, so an approval landing while the agent is mid-edit can merge — and delete — the branch out from under the run, which then fails on push: ! [remote rejected] ... cannot lock ref ... unable to resolve reference (run 27078382804: a review-changes run committed 1s after PR #449 auto-merged). Per-PR serialization (#450) stops two dev-lead runs fighting, but can't stop GitHub's async merge. So: - scripts/lib/auto-merge.sh: hold_auto_merge disables auto-merge at the start of a modifying run; restore_auto_merge (EXIT trap) restores it on any exit, but only if this run disabled it — never newly enables it. push_with_merge_guard treats a push that fails because the PR merged/closed mid-run as a benign no-op (exit 0) instead of a red-X failure. - fix-reviews.sh / fix-ci.sh: hold auto-merge + register the restore trap before checkout, and push via push_with_merge_guard. pr-worktree.sh now chains its cleanup onto the caller's EXIT trap instead of clobbering it. - test_concurrency_config.sh: was stale (asserted the pre-#450 repo-wide group, failing against main); now asserts the per-PR/per-issue serialization lanes + cancel-in-progress:false on both the inline and reusable workflows. Tests: 284 unit tests pass (new test_auto_merge.bats; trap-chaining case added to test_pr_worktree.bats); concurrency + prompt-coverage integration pass; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): reply with the specific fix on each review thread, then resolve (#452) Two gaps left fixed review threads stuck open and blocking re-review: 1. The review prompts told the engine to RESOLVE a fixed thread but never to REPLY with what it changed — reviewers got a silent resolve. 2. fix-reviews gated resolution to ${TRIGGERING_REVIEWER} only, so a thread fixed during a different reviewer's run (e.g. a Copilot thread fixed in a Codex-triggered run) was never resolved and blocked approval. Update the three review prompts (fix-reviews, review-changes, fix-bot-comment): - For every thread it fixes, the engine now posts a reply naming the specific file/function changed and how it addresses the concern (addPullRequestReviewThreadReply with the body passed as a GraphQL variable), THEN resolves the thread. - fix-reviews now resolves every BOT thread it fixed regardless of which reviewer triggered the run (human threads still gated to the trigger). - Rubber-duck phase verifies every fixed thread was replied-to and resolved. test_prompt_coverage.sh asserts each review prompt instructs reply-with-specifics + resolve. Full unit suite (292), prompt-coverage, and concurrency tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): restore auto-merge against the PR's current head, re-fetched at restore time (#452) Codex follow-up: the rebase intent force-pushes directly from the engine (prompts/dev-lead/rebase.md: `git push --force-with-lease`), bypassing push_with_merge_guard, so _AM_HEAD_SHA is never refreshed and the EXIT-trap restore pins --match-head-commit to the stale hold-time SHA → GitHub rejects it and auto-merge is left off. Make restore_auto_merge re-fetch the PR's CURRENT head at restore time and match against that, falling back to the hold-time SHA only if the re-fetch fails. This subsumes every head-advances-during-run path (own commits, rebase force-push, external pushes, failure-after-push) instead of patching each one. Tests: new cases for current-head preference and the re-fetch-failure fallback. Full suite 295 unit + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): expose author.__typename in the review-changes thread query too (#452) The fix-reviews OPEN_THREADS_JSON build was updated to select author.__typename (GraphQL omits the [bot] suffix from bot logins, so bots are identified by __typename == "Bot"), but the review-changes build still selected only author.login. Without __typename the review-changes engine cannot reliably tell bots from humans, so its "resolve only human threads, never bots" rule could misclassify coderabbitai / chatgpt-codex-connector / etc. as human. - Add __typename to the review-changes OPEN_THREADS_JSON query. - review-changes.md now identifies humans/bots by author.__typename (User/Bot), with an explicit note not to rely on a [bot] login suffix. - test_prompt_coverage.sh now asserts EVERY OPEN_THREADS_JSON build selects author.__typename, so a future build can't silently omit it. Full unit suite + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 3, 2026
…449) * fix(dev-lead): check out PR branches in an isolated worktree (#448) The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 3, 2026
…452) (#453) * fix(dev-lead): hold auto-merge while working a PR + push defensively (#452) dev-lead enables auto-merge (squash) by default (#447). GitHub's auto-merge is asynchronous and not gated by the workflow concurrency lane, so an approval landing while the agent is mid-edit can merge — and delete — the branch out from under the run, which then fails on push: ! [remote rejected] ... cannot lock ref ... unable to resolve reference (run 27078382804: a review-changes run committed 1s after PR #449 auto-merged). Per-PR serialization (#450) stops two dev-lead runs fighting, but can't stop GitHub's async merge. So: - scripts/lib/auto-merge.sh: hold_auto_merge disables auto-merge at the start of a modifying run; restore_auto_merge (EXIT trap) restores it on any exit, but only if this run disabled it — never newly enables it. push_with_merge_guard treats a push that fails because the PR merged/closed mid-run as a benign no-op (exit 0) instead of a red-X failure. - fix-reviews.sh / fix-ci.sh: hold auto-merge + register the restore trap before checkout, and push via push_with_merge_guard. pr-worktree.sh now chains its cleanup onto the caller's EXIT trap instead of clobbering it. - test_concurrency_config.sh: was stale (asserted the pre-#450 repo-wide group, failing against main); now asserts the per-PR/per-issue serialization lanes + cancel-in-progress:false on both the inline and reusable workflows. Tests: 284 unit tests pass (new test_auto_merge.bats; trap-chaining case added to test_pr_worktree.bats); concurrency + prompt-coverage integration pass; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): reply with the specific fix on each review thread, then resolve (#452) Two gaps left fixed review threads stuck open and blocking re-review: 1. The review prompts told the engine to RESOLVE a fixed thread but never to REPLY with what it changed — reviewers got a silent resolve. 2. fix-reviews gated resolution to ${TRIGGERING_REVIEWER} only, so a thread fixed during a different reviewer's run (e.g. a Copilot thread fixed in a Codex-triggered run) was never resolved and blocked approval. Update the three review prompts (fix-reviews, review-changes, fix-bot-comment): - For every thread it fixes, the engine now posts a reply naming the specific file/function changed and how it addresses the concern (addPullRequestReviewThreadReply with the body passed as a GraphQL variable), THEN resolves the thread. - fix-reviews now resolves every BOT thread it fixed regardless of which reviewer triggered the run (human threads still gated to the trigger). - Rubber-duck phase verifies every fixed thread was replied-to and resolved. test_prompt_coverage.sh asserts each review prompt instructs reply-with-specifics + resolve. Full unit suite (292), prompt-coverage, and concurrency tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): restore auto-merge against the PR's current head, re-fetched at restore time (#452) Codex follow-up: the rebase intent force-pushes directly from the engine (prompts/dev-lead/rebase.md: `git push --force-with-lease`), bypassing push_with_merge_guard, so _AM_HEAD_SHA is never refreshed and the EXIT-trap restore pins --match-head-commit to the stale hold-time SHA → GitHub rejects it and auto-merge is left off. Make restore_auto_merge re-fetch the PR's CURRENT head at restore time and match against that, falling back to the hold-time SHA only if the re-fetch fails. This subsumes every head-advances-during-run path (own commits, rebase force-push, external pushes, failure-after-push) instead of patching each one. Tests: new cases for current-head preference and the re-fetch-failure fallback. Full suite 295 unit + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): expose author.__typename in the review-changes thread query too (#452) The fix-reviews OPEN_THREADS_JSON build was updated to select author.__typename (GraphQL omits the [bot] suffix from bot logins, so bots are identified by __typename == "Bot"), but the review-changes build still selected only author.login. Without __typename the review-changes engine cannot reliably tell bots from humans, so its "resolve only human threads, never bots" rule could misclassify coderabbitai / chatgpt-codex-connector / etc. as human. - Add __typename to the review-changes OPEN_THREADS_JSON query. - review-changes.md now identifies humans/bots by author.__typename (User/Bot), with an explicit note not to rely on a [bot] login suffix. - test_prompt_coverage.sh now asserts EVERY OPEN_THREADS_JSON build selects author.__typename, so a future build can't silently omit it. Full unit suite + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 7, 2026
…449) * fix(dev-lead): check out PR branches in an isolated worktree (#448) The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 7, 2026
…452) (#453) * fix(dev-lead): hold auto-merge while working a PR + push defensively (#452) dev-lead enables auto-merge (squash) by default (#447). GitHub's auto-merge is asynchronous and not gated by the workflow concurrency lane, so an approval landing while the agent is mid-edit can merge — and delete — the branch out from under the run, which then fails on push: ! [remote rejected] ... cannot lock ref ... unable to resolve reference (run 27078382804: a review-changes run committed 1s after PR #449 auto-merged). Per-PR serialization (#450) stops two dev-lead runs fighting, but can't stop GitHub's async merge. So: - scripts/lib/auto-merge.sh: hold_auto_merge disables auto-merge at the start of a modifying run; restore_auto_merge (EXIT trap) restores it on any exit, but only if this run disabled it — never newly enables it. push_with_merge_guard treats a push that fails because the PR merged/closed mid-run as a benign no-op (exit 0) instead of a red-X failure. - fix-reviews.sh / fix-ci.sh: hold auto-merge + register the restore trap before checkout, and push via push_with_merge_guard. pr-worktree.sh now chains its cleanup onto the caller's EXIT trap instead of clobbering it. - test_concurrency_config.sh: was stale (asserted the pre-#450 repo-wide group, failing against main); now asserts the per-PR/per-issue serialization lanes + cancel-in-progress:false on both the inline and reusable workflows. Tests: 284 unit tests pass (new test_auto_merge.bats; trap-chaining case added to test_pr_worktree.bats); concurrency + prompt-coverage integration pass; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): reply with the specific fix on each review thread, then resolve (#452) Two gaps left fixed review threads stuck open and blocking re-review: 1. The review prompts told the engine to RESOLVE a fixed thread but never to REPLY with what it changed — reviewers got a silent resolve. 2. fix-reviews gated resolution to ${TRIGGERING_REVIEWER} only, so a thread fixed during a different reviewer's run (e.g. a Copilot thread fixed in a Codex-triggered run) was never resolved and blocked approval. Update the three review prompts (fix-reviews, review-changes, fix-bot-comment): - For every thread it fixes, the engine now posts a reply naming the specific file/function changed and how it addresses the concern (addPullRequestReviewThreadReply with the body passed as a GraphQL variable), THEN resolves the thread. - fix-reviews now resolves every BOT thread it fixed regardless of which reviewer triggered the run (human threads still gated to the trigger). - Rubber-duck phase verifies every fixed thread was replied-to and resolved. test_prompt_coverage.sh asserts each review prompt instructs reply-with-specifics + resolve. Full unit suite (292), prompt-coverage, and concurrency tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): restore auto-merge against the PR's current head, re-fetched at restore time (#452) Codex follow-up: the rebase intent force-pushes directly from the engine (prompts/dev-lead/rebase.md: `git push --force-with-lease`), bypassing push_with_merge_guard, so _AM_HEAD_SHA is never refreshed and the EXIT-trap restore pins --match-head-commit to the stale hold-time SHA → GitHub rejects it and auto-merge is left off. Make restore_auto_merge re-fetch the PR's CURRENT head at restore time and match against that, falling back to the hold-time SHA only if the re-fetch fails. This subsumes every head-advances-during-run path (own commits, rebase force-push, external pushes, failure-after-push) instead of patching each one. Tests: new cases for current-head preference and the re-fetch-failure fallback. Full suite 295 unit + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): expose author.__typename in the review-changes thread query too (#452) The fix-reviews OPEN_THREADS_JSON build was updated to select author.__typename (GraphQL omits the [bot] suffix from bot logins, so bots are identified by __typename == "Bot"), but the review-changes build still selected only author.login. Without __typename the review-changes engine cannot reliably tell bots from humans, so its "resolve only human threads, never bots" rule could misclassify coderabbitai / chatgpt-codex-connector / etc. as human. - Add __typename to the review-changes OPEN_THREADS_JSON query. - review-changes.md now identifies humans/bots by author.__typename (User/Bot), with an explicit note not to rely on a [bot] login suffix. - test_prompt_coverage.sh now asserts EVERY OPEN_THREADS_JSON build selects author.__typename, so a future build can't silently omit it. Full unit suite + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 7, 2026
…449) * fix(dev-lead): check out PR branches in an isolated worktree (#448) The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 7, 2026
…452) (#453) * fix(dev-lead): hold auto-merge while working a PR + push defensively (#452) dev-lead enables auto-merge (squash) by default (#447). GitHub's auto-merge is asynchronous and not gated by the workflow concurrency lane, so an approval landing while the agent is mid-edit can merge — and delete — the branch out from under the run, which then fails on push: ! [remote rejected] ... cannot lock ref ... unable to resolve reference (run 27078382804: a review-changes run committed 1s after PR #449 auto-merged). Per-PR serialization (#450) stops two dev-lead runs fighting, but can't stop GitHub's async merge. So: - scripts/lib/auto-merge.sh: hold_auto_merge disables auto-merge at the start of a modifying run; restore_auto_merge (EXIT trap) restores it on any exit, but only if this run disabled it — never newly enables it. push_with_merge_guard treats a push that fails because the PR merged/closed mid-run as a benign no-op (exit 0) instead of a red-X failure. - fix-reviews.sh / fix-ci.sh: hold auto-merge + register the restore trap before checkout, and push via push_with_merge_guard. pr-worktree.sh now chains its cleanup onto the caller's EXIT trap instead of clobbering it. - test_concurrency_config.sh: was stale (asserted the pre-#450 repo-wide group, failing against main); now asserts the per-PR/per-issue serialization lanes + cancel-in-progress:false on both the inline and reusable workflows. Tests: 284 unit tests pass (new test_auto_merge.bats; trap-chaining case added to test_pr_worktree.bats); concurrency + prompt-coverage integration pass; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): reply with the specific fix on each review thread, then resolve (#452) Two gaps left fixed review threads stuck open and blocking re-review: 1. The review prompts told the engine to RESOLVE a fixed thread but never to REPLY with what it changed — reviewers got a silent resolve. 2. fix-reviews gated resolution to ${TRIGGERING_REVIEWER} only, so a thread fixed during a different reviewer's run (e.g. a Copilot thread fixed in a Codex-triggered run) was never resolved and blocked approval. Update the three review prompts (fix-reviews, review-changes, fix-bot-comment): - For every thread it fixes, the engine now posts a reply naming the specific file/function changed and how it addresses the concern (addPullRequestReviewThreadReply with the body passed as a GraphQL variable), THEN resolves the thread. - fix-reviews now resolves every BOT thread it fixed regardless of which reviewer triggered the run (human threads still gated to the trigger). - Rubber-duck phase verifies every fixed thread was replied-to and resolved. test_prompt_coverage.sh asserts each review prompt instructs reply-with-specifics + resolve. Full unit suite (292), prompt-coverage, and concurrency tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): restore auto-merge against the PR's current head, re-fetched at restore time (#452) Codex follow-up: the rebase intent force-pushes directly from the engine (prompts/dev-lead/rebase.md: `git push --force-with-lease`), bypassing push_with_merge_guard, so _AM_HEAD_SHA is never refreshed and the EXIT-trap restore pins --match-head-commit to the stale hold-time SHA → GitHub rejects it and auto-merge is left off. Make restore_auto_merge re-fetch the PR's CURRENT head at restore time and match against that, falling back to the hold-time SHA only if the re-fetch fails. This subsumes every head-advances-during-run path (own commits, rebase force-push, external pushes, failure-after-push) instead of patching each one. Tests: new cases for current-head preference and the re-fetch-failure fallback. Full suite 295 unit + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): expose author.__typename in the review-changes thread query too (#452) The fix-reviews OPEN_THREADS_JSON build was updated to select author.__typename (GraphQL omits the [bot] suffix from bot logins, so bots are identified by __typename == "Bot"), but the review-changes build still selected only author.login. Without __typename the review-changes engine cannot reliably tell bots from humans, so its "resolve only human threads, never bots" rule could misclassify coderabbitai / chatgpt-codex-connector / etc. as human. - Add __typename to the review-changes OPEN_THREADS_JSON query. - review-changes.md now identifies humans/bots by author.__typename (User/Bot), with an explicit note not to rely on a [bot] login suffix. - test_prompt_coverage.sh now asserts EVERY OPEN_THREADS_JSON build selects author.__typename, so a future build can't silently omit it. Full unit suite + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 7, 2026
…449) * fix(dev-lead): check out PR branches in an isolated worktree (#448) The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 7, 2026
…452) (#453) * fix(dev-lead): hold auto-merge while working a PR + push defensively (#452) dev-lead enables auto-merge (squash) by default (#447). GitHub's auto-merge is asynchronous and not gated by the workflow concurrency lane, so an approval landing while the agent is mid-edit can merge — and delete — the branch out from under the run, which then fails on push: ! [remote rejected] ... cannot lock ref ... unable to resolve reference (run 27078382804: a review-changes run committed 1s after PR #449 auto-merged). Per-PR serialization (#450) stops two dev-lead runs fighting, but can't stop GitHub's async merge. So: - scripts/lib/auto-merge.sh: hold_auto_merge disables auto-merge at the start of a modifying run; restore_auto_merge (EXIT trap) restores it on any exit, but only if this run disabled it — never newly enables it. push_with_merge_guard treats a push that fails because the PR merged/closed mid-run as a benign no-op (exit 0) instead of a red-X failure. - fix-reviews.sh / fix-ci.sh: hold auto-merge + register the restore trap before checkout, and push via push_with_merge_guard. pr-worktree.sh now chains its cleanup onto the caller's EXIT trap instead of clobbering it. - test_concurrency_config.sh: was stale (asserted the pre-#450 repo-wide group, failing against main); now asserts the per-PR/per-issue serialization lanes + cancel-in-progress:false on both the inline and reusable workflows. Tests: 284 unit tests pass (new test_auto_merge.bats; trap-chaining case added to test_pr_worktree.bats); concurrency + prompt-coverage integration pass; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): reply with the specific fix on each review thread, then resolve (#452) Two gaps left fixed review threads stuck open and blocking re-review: 1. The review prompts told the engine to RESOLVE a fixed thread but never to REPLY with what it changed — reviewers got a silent resolve. 2. fix-reviews gated resolution to ${TRIGGERING_REVIEWER} only, so a thread fixed during a different reviewer's run (e.g. a Copilot thread fixed in a Codex-triggered run) was never resolved and blocked approval. Update the three review prompts (fix-reviews, review-changes, fix-bot-comment): - For every thread it fixes, the engine now posts a reply naming the specific file/function changed and how it addresses the concern (addPullRequestReviewThreadReply with the body passed as a GraphQL variable), THEN resolves the thread. - fix-reviews now resolves every BOT thread it fixed regardless of which reviewer triggered the run (human threads still gated to the trigger). - Rubber-duck phase verifies every fixed thread was replied-to and resolved. test_prompt_coverage.sh asserts each review prompt instructs reply-with-specifics + resolve. Full unit suite (292), prompt-coverage, and concurrency tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): restore auto-merge against the PR's current head, re-fetched at restore time (#452) Codex follow-up: the rebase intent force-pushes directly from the engine (prompts/dev-lead/rebase.md: `git push --force-with-lease`), bypassing push_with_merge_guard, so _AM_HEAD_SHA is never refreshed and the EXIT-trap restore pins --match-head-commit to the stale hold-time SHA → GitHub rejects it and auto-merge is left off. Make restore_auto_merge re-fetch the PR's CURRENT head at restore time and match against that, falling back to the hold-time SHA only if the re-fetch fails. This subsumes every head-advances-during-run path (own commits, rebase force-push, external pushes, failure-after-push) instead of patching each one. Tests: new cases for current-head preference and the re-fetch-failure fallback. Full suite 295 unit + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): expose author.__typename in the review-changes thread query too (#452) The fix-reviews OPEN_THREADS_JSON build was updated to select author.__typename (GraphQL omits the [bot] suffix from bot logins, so bots are identified by __typename == "Bot"), but the review-changes build still selected only author.login. Without __typename the review-changes engine cannot reliably tell bots from humans, so its "resolve only human threads, never bots" rule could misclassify coderabbitai / chatgpt-codex-connector / etc. as human. - Add __typename to the review-changes OPEN_THREADS_JSON query. - review-changes.md now identifies humans/bots by author.__typename (User/Bot), with an explicit note not to rely on a [bot] login suffix. - test_prompt_coverage.sh now asserts EVERY OPEN_THREADS_JSON build selects author.__typename, so a future build can't silently omit it. Full unit suite + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 7, 2026
…449) * fix(dev-lead): check out PR branches in an isolated worktree (#448) The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 7, 2026
…452) (#453) * fix(dev-lead): hold auto-merge while working a PR + push defensively (#452) dev-lead enables auto-merge (squash) by default (#447). GitHub's auto-merge is asynchronous and not gated by the workflow concurrency lane, so an approval landing while the agent is mid-edit can merge — and delete — the branch out from under the run, which then fails on push: ! [remote rejected] ... cannot lock ref ... unable to resolve reference (run 27078382804: a review-changes run committed 1s after PR #449 auto-merged). Per-PR serialization (#450) stops two dev-lead runs fighting, but can't stop GitHub's async merge. So: - scripts/lib/auto-merge.sh: hold_auto_merge disables auto-merge at the start of a modifying run; restore_auto_merge (EXIT trap) restores it on any exit, but only if this run disabled it — never newly enables it. push_with_merge_guard treats a push that fails because the PR merged/closed mid-run as a benign no-op (exit 0) instead of a red-X failure. - fix-reviews.sh / fix-ci.sh: hold auto-merge + register the restore trap before checkout, and push via push_with_merge_guard. pr-worktree.sh now chains its cleanup onto the caller's EXIT trap instead of clobbering it. - test_concurrency_config.sh: was stale (asserted the pre-#450 repo-wide group, failing against main); now asserts the per-PR/per-issue serialization lanes + cancel-in-progress:false on both the inline and reusable workflows. Tests: 284 unit tests pass (new test_auto_merge.bats; trap-chaining case added to test_pr_worktree.bats); concurrency + prompt-coverage integration pass; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): reply with the specific fix on each review thread, then resolve (#452) Two gaps left fixed review threads stuck open and blocking re-review: 1. The review prompts told the engine to RESOLVE a fixed thread but never to REPLY with what it changed — reviewers got a silent resolve. 2. fix-reviews gated resolution to ${TRIGGERING_REVIEWER} only, so a thread fixed during a different reviewer's run (e.g. a Copilot thread fixed in a Codex-triggered run) was never resolved and blocked approval. Update the three review prompts (fix-reviews, review-changes, fix-bot-comment): - For every thread it fixes, the engine now posts a reply naming the specific file/function changed and how it addresses the concern (addPullRequestReviewThreadReply with the body passed as a GraphQL variable), THEN resolves the thread. - fix-reviews now resolves every BOT thread it fixed regardless of which reviewer triggered the run (human threads still gated to the trigger). - Rubber-duck phase verifies every fixed thread was replied-to and resolved. test_prompt_coverage.sh asserts each review prompt instructs reply-with-specifics + resolve. Full unit suite (292), prompt-coverage, and concurrency tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): restore auto-merge against the PR's current head, re-fetched at restore time (#452) Codex follow-up: the rebase intent force-pushes directly from the engine (prompts/dev-lead/rebase.md: `git push --force-with-lease`), bypassing push_with_merge_guard, so _AM_HEAD_SHA is never refreshed and the EXIT-trap restore pins --match-head-commit to the stale hold-time SHA → GitHub rejects it and auto-merge is left off. Make restore_auto_merge re-fetch the PR's CURRENT head at restore time and match against that, falling back to the hold-time SHA only if the re-fetch fails. This subsumes every head-advances-during-run path (own commits, rebase force-push, external pushes, failure-after-push) instead of patching each one. Tests: new cases for current-head preference and the re-fetch-failure fallback. Full suite 295 unit + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): expose author.__typename in the review-changes thread query too (#452) The fix-reviews OPEN_THREADS_JSON build was updated to select author.__typename (GraphQL omits the [bot] suffix from bot logins, so bots are identified by __typename == "Bot"), but the review-changes build still selected only author.login. Without __typename the review-changes engine cannot reliably tell bots from humans, so its "resolve only human threads, never bots" rule could misclassify coderabbitai / chatgpt-codex-connector / etc. as human. - Add __typename to the review-changes OPEN_THREADS_JSON query. - review-changes.md now identifies humans/bots by author.__typename (User/Bot), with an explicit note not to rely on a [bot] login suffix. - test_prompt_coverage.sh now asserts EVERY OPEN_THREADS_JSON build selects author.__typename, so a future build can't silently omit it. Full unit suite + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 8, 2026
…449) * fix(dev-lead): check out PR branches in an isolated worktree (#448) The agent's prompt templates and helper scripts live in the same repo it operates on. fix-reviews/fix-ci ran `gh pr checkout` in place, switching the working tree to the PR branch; when that branch predated a referenced prompt (e.g. on-mention.md), the file vanished mid-run and the script aborted: prompts/dev-lead/on-mention.md: No such file or directory (petry-projects/.github-private run 27077585464.) Add scripts/lib/pr-worktree.sh, which checks the PR out in a throwaway git worktree and removes it on exit. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch. PROMPTS_DIR is pinned to an absolute path before entering the worktree so templates remain readable after the cd. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 8, 2026
…452) (#453) * fix(dev-lead): hold auto-merge while working a PR + push defensively (#452) dev-lead enables auto-merge (squash) by default (#447). GitHub's auto-merge is asynchronous and not gated by the workflow concurrency lane, so an approval landing while the agent is mid-edit can merge — and delete — the branch out from under the run, which then fails on push: ! [remote rejected] ... cannot lock ref ... unable to resolve reference (run 27078382804: a review-changes run committed 1s after PR #449 auto-merged). Per-PR serialization (#450) stops two dev-lead runs fighting, but can't stop GitHub's async merge. So: - scripts/lib/auto-merge.sh: hold_auto_merge disables auto-merge at the start of a modifying run; restore_auto_merge (EXIT trap) restores it on any exit, but only if this run disabled it — never newly enables it. push_with_merge_guard treats a push that fails because the PR merged/closed mid-run as a benign no-op (exit 0) instead of a red-X failure. - fix-reviews.sh / fix-ci.sh: hold auto-merge + register the restore trap before checkout, and push via push_with_merge_guard. pr-worktree.sh now chains its cleanup onto the caller's EXIT trap instead of clobbering it. - test_concurrency_config.sh: was stale (asserted the pre-#450 repo-wide group, failing against main); now asserts the per-PR/per-issue serialization lanes + cancel-in-progress:false on both the inline and reusable workflows. Tests: 284 unit tests pass (new test_auto_merge.bats; trap-chaining case added to test_pr_worktree.bats); concurrency + prompt-coverage integration pass; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): reply with the specific fix on each review thread, then resolve (#452) Two gaps left fixed review threads stuck open and blocking re-review: 1. The review prompts told the engine to RESOLVE a fixed thread but never to REPLY with what it changed — reviewers got a silent resolve. 2. fix-reviews gated resolution to ${TRIGGERING_REVIEWER} only, so a thread fixed during a different reviewer's run (e.g. a Copilot thread fixed in a Codex-triggered run) was never resolved and blocked approval. Update the three review prompts (fix-reviews, review-changes, fix-bot-comment): - For every thread it fixes, the engine now posts a reply naming the specific file/function changed and how it addresses the concern (addPullRequestReviewThreadReply with the body passed as a GraphQL variable), THEN resolves the thread. - fix-reviews now resolves every BOT thread it fixed regardless of which reviewer triggered the run (human threads still gated to the trigger). - Rubber-duck phase verifies every fixed thread was replied-to and resolved. test_prompt_coverage.sh asserts each review prompt instructs reply-with-specifics + resolve. Full unit suite (292), prompt-coverage, and concurrency tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): restore auto-merge against the PR's current head, re-fetched at restore time (#452) Codex follow-up: the rebase intent force-pushes directly from the engine (prompts/dev-lead/rebase.md: `git push --force-with-lease`), bypassing push_with_merge_guard, so _AM_HEAD_SHA is never refreshed and the EXIT-trap restore pins --match-head-commit to the stale hold-time SHA → GitHub rejects it and auto-merge is left off. Make restore_auto_merge re-fetch the PR's CURRENT head at restore time and match against that, falling back to the hold-time SHA only if the re-fetch fails. This subsumes every head-advances-during-run path (own commits, rebase force-push, external pushes, failure-after-push) instead of patching each one. Tests: new cases for current-head preference and the re-fetch-failure fallback. Full suite 295 unit + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): expose author.__typename in the review-changes thread query too (#452) The fix-reviews OPEN_THREADS_JSON build was updated to select author.__typename (GraphQL omits the [bot] suffix from bot logins, so bots are identified by __typename == "Bot"), but the review-changes build still selected only author.login. Without __typename the review-changes engine cannot reliably tell bots from humans, so its "resolve only human threads, never bots" rule could misclassify coderabbitai / chatgpt-codex-connector / etc. as human. - Add __typename to the review-changes OPEN_THREADS_JSON query. - review-changes.md now identifies humans/bots by author.__typename (User/Bot), with an explicit note not to rely on a [bot] login suffix. - test_prompt_coverage.sh now asserts EVERY OPEN_THREADS_JSON build selects author.__typename, so a future build can't silently omit it. Full unit suite + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
don-petry
added a commit
that referenced
this pull request
Aug 8, 2026
…452) (#453) * fix(dev-lead): hold auto-merge while working a PR + push defensively (#452) dev-lead enables auto-merge (squash) by default (#447). GitHub's auto-merge is asynchronous and not gated by the workflow concurrency lane, so an approval landing while the agent is mid-edit can merge — and delete — the branch out from under the run, which then fails on push: ! [remote rejected] ... cannot lock ref ... unable to resolve reference (run 27078382804: a review-changes run committed 1s after PR #449 auto-merged). Per-PR serialization (#450) stops two dev-lead runs fighting, but can't stop GitHub's async merge. So: - scripts/lib/auto-merge.sh: hold_auto_merge disables auto-merge at the start of a modifying run; restore_auto_merge (EXIT trap) restores it on any exit, but only if this run disabled it — never newly enables it. push_with_merge_guard treats a push that fails because the PR merged/closed mid-run as a benign no-op (exit 0) instead of a red-X failure. - fix-reviews.sh / fix-ci.sh: hold auto-merge + register the restore trap before checkout, and push via push_with_merge_guard. pr-worktree.sh now chains its cleanup onto the caller's EXIT trap instead of clobbering it. - test_concurrency_config.sh: was stale (asserted the pre-#450 repo-wide group, failing against main); now asserts the per-PR/per-issue serialization lanes + cancel-in-progress:false on both the inline and reusable workflows. Tests: 284 unit tests pass (new test_auto_merge.bats; trap-chaining case added to test_pr_worktree.bats); concurrency + prompt-coverage integration pass; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * chore: apply manual instructions [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): reply with the specific fix on each review thread, then resolve (#452) Two gaps left fixed review threads stuck open and blocking re-review: 1. The review prompts told the engine to RESOLVE a fixed thread but never to REPLY with what it changed — reviewers got a silent resolve. 2. fix-reviews gated resolution to ${TRIGGERING_REVIEWER} only, so a thread fixed during a different reviewer's run (e.g. a Copilot thread fixed in a Codex-triggered run) was never resolved and blocked approval. Update the three review prompts (fix-reviews, review-changes, fix-bot-comment): - For every thread it fixes, the engine now posts a reply naming the specific file/function changed and how it addresses the concern (addPullRequestReviewThreadReply with the body passed as a GraphQL variable), THEN resolves the thread. - fix-reviews now resolves every BOT thread it fixed regardless of which reviewer triggered the run (human threads still gated to the trigger). - Rubber-duck phase verifies every fixed thread was replied-to and resolved. test_prompt_coverage.sh asserts each review prompt instructs reply-with-specifics + resolve. Full unit suite (292), prompt-coverage, and concurrency tests pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): restore auto-merge against the PR's current head, re-fetched at restore time (#452) Codex follow-up: the rebase intent force-pushes directly from the engine (prompts/dev-lead/rebase.md: `git push --force-with-lease`), bypassing push_with_merge_guard, so _AM_HEAD_SHA is never refreshed and the EXIT-trap restore pins --match-head-commit to the stale hold-time SHA → GitHub rejects it and auto-merge is left off. Make restore_auto_merge re-fetch the PR's CURRENT head at restore time and match against that, falling back to the hold-time SHA only if the re-fetch fails. This subsumes every head-advances-during-run path (own commits, rebase force-push, external pushes, failure-after-push) instead of patching each one. Tests: new cases for current-head preference and the re-fetch-failure fallback. Full suite 295 unit + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] * fix(reviews): address review comments [skip ci-relay] * fix(dev-lead): expose author.__typename in the review-changes thread query too (#452) The fix-reviews OPEN_THREADS_JSON build was updated to select author.__typename (GraphQL omits the [bot] suffix from bot logins, so bots are identified by __typename == "Bot"), but the review-changes build still selected only author.login. Without __typename the review-changes engine cannot reliably tell bots from humans, so its "resolve only human threads, never bots" rule could misclassify coderabbitai / chatgpt-codex-connector / etc. as human. - Add __typename to the review-changes OPEN_THREADS_JSON query. - review-changes.md now identifies humans/bots by author.__typename (User/Bot), with an explicit note not to rely on a [bot] login suffix. - test_prompt_coverage.sh now asserts EVERY OPEN_THREADS_JSON build selects author.__typename, so a future build can't silently omit it. Full unit suite + prompt-coverage + concurrency green; shellcheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(reviews): address review comments [skip ci-relay] --------- Co-authored-by: donpetry-bot <{}+donpetry-bot@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: donpetry-bot <281750570+donpetry-bot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Fixes #448.
Check out PR branches in an isolated git worktree instead of switching the agent's own working tree in place.
Why
The dev-lead agent's prompt templates (
prompts/dev-lead/*.md) and scripts (scripts/*) live in the same repository it operates on.dev-lead-fix-reviews.shanddev-lead-fix-ci.shrangh pr checkoutin place, switching the working tree to the PR branch. When the PR branch predated a referenced prompt, that file disappeared mid-run and the script aborted:Observed in petry-projects/.github-private run 27077585464 (
@dev-leadon PR #226, whose branch predateson-mention.mdfrom #358).How
scripts/lib/pr-worktree.sh—checkout_pr_in_worktreecreates a throwaway worktree (git worktree add --detach+gh pr checkoutinside it), cds into it, and registers anEXITtrap that removes it. The agent checkout stays on the agent ref, so its prompts/scripts survive; the engine runs inside the worktree against the PR branch.resolve_abspinsPROMPTS_DIRto an absolute path before entering the worktree, so templates stay readable after thecd.dev-lead-fix-reviews.shanddev-lead-fix-ci.shnow callcheckout_pr_in_worktreein place of the in-placegh pr checkout.This eliminates the whole class of "agent assets disappear when checking out an old PR branch" failures, which is most acute when
.github-privateoperates on its own PRs.Testing
bats tests/dev-lead/unit/— 273 pass (full suite).tests/dev-lead/unit/test_pr_worktree.bats(6 tests) reproduces issue dev-lead: in-placegh pr checkoutclobbers the agent's own prompt/script files on older PR branches #448: a PR branch missing a prompt is checked out in the worktree while the absolutePROMPTS_DIRstays readable; verifies cleanup removes the worktree and returns to the agent checkout.commit_and_pushtests intest_fix_reviews.batsso the engine stub makes its change inside the worktree (its CWD).shellcheck -S warningclean on all three scripts.prompt-coverage,concurrency-config) and e2e06-exhaustion-guardpass. (e2e07A/B/D/E fail identically onorigin/main— pre-existing, they need real engine CLIs locally.)🤖 Generated with Claude Code