Run address-pr-reviews from a detached HEAD - #211
Merged
Merged
Conversation
An agent harness (PostHog Desktop, cloud runners) checks the PR head out detached, where `git branch --show-current` is empty. git-pr resolves the PR from HEAD's commit there, and repo-context.sh's new resolve_branch_name names the branch for log-step-done.sh and ran-report.sh. log-command.sh keeps its network-free contract and skips the new tier. address-pr-reviews asks its three approval gates through the harness's structured question tool, because a task runner reads a prose question as the end of the turn. Its new --unattended flag takes each gate's default: apply every fix, reply to and resolve bot threads, commit, and push the PR head ref. babysit-prs passes it on dispatch. Generated-By: PostHog Desktop Task-Id: 4e81ba70-85c0-47fa-9cf7-26fce96f35a1
A CI runner has no global user.name or user.email, so the fixture's commit failed and left HEAD unborn. The detach then read "HEAD" as a path. Generated-By: PostHog Desktop Task-Id: 4e81ba70-85c0-47fa-9cf7-26fce96f35a1
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical HEAD-safety and moderate post-commit detached-branch issues remain.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Updates PR/review automation for detached HEAD checkouts and unattended review handling.
Changes:
- Adds SHA-based PR and branch resolution.
- Adds unattended approval gates and explicit pushes.
- Expands offline tests and CI coverage.
File summaries
| File | Summary and review notes |
|---|---|
bin/lib/test-git-pr.sh |
Adds detached-HEAD PR lookup tests. |
bin/git-pr |
Resolves PRs by exact HEAD SHA. |
ai/tests/test-log-step-done.sh |
Tests detached completion logging. |
ai/skills/wait-for-pr-reviews/SKILL.md |
Clarifies attended workflow behavior. |
ai/skills/wait-for-pr-reviews/scripts/git-pr |
Syncs detached PR lookup logic. |
ai/skills/ran/scripts/tests/test-ran-report.sh |
Tests detached reporting. |
ai/skills/ran/scripts/ran-report.sh |
Adds detached reporting. Nit (1 vote): update exit-code documentation. Moderate (2 votes): persist or pass branch identity after post-commit detached runs and add coverage. |
ai/skills/babysit-prs/SKILL.md |
Passes unattended mode. |
ai/skills/address-pr-reviews/SKILL.md |
Adds approval gates and explicit pushes. Critical (1 vote): retain and verify HEAD_SHA before pushing. |
ai/helpers/tests/test-repo-context.sh |
Tests branch-resolution tiers. |
ai/helpers/repo-context.sh |
Adds shared branch-resolution logic. |
ai/bin/log-step-done.sh |
Supports detached completion logging. Moderate (2 votes): pass or persist RAN_BRANCH for post-commit detached runs and add coverage. |
ai/bin/log-command.sh |
Preserves network-free behavior. |
ai/AGENTS.md |
Documents unattended bot replies. Nit (3 votes): align the referenced policy with the explicit exception. |
.github/workflows/test.yml |
Registers the new test suite. |
Review details
Suppressed comments (2)
ai/skills/address-pr-reviews/SKILL.md:141
- This new unattended dispatch is followed by the generic Step 4 instruction to push resulting commits. That conflicts with
address-pr-reviews's fork rule immediately below: fork PRs must be left unpushed because the head repository is not$REPO. For an authored fork PR, the sweep can therefore override this guard and push a local commit to the wrong remote or branch. Make the caller's final push conditional on the delegated skill confirming a same-repository head, and carry the fork case through as explicitly unpushed.
- Push with `git push <remote> HEAD:refs/heads/$HEAD_BRANCH`, naming the ref. A worktree checked out at the PR head has no current branch, and a bare `git push` there fails with no upstream.
- Push only when the PR's head repo is `$REPO`. A fork PR's head repo has no local remote, so report that and leave the commit unpushed.
ai/skills/ran/scripts/ran-report.sh:59
- The exit-code header above still lists “detached HEAD” as an unconditional error, but this new path succeeds for a detached checkout when
RAN_BRANCHor a PR head lookup resolves the branch. Update that documentation so callers do not treat the newly supported case as a failure.
branch=$(resolve_branch_name network) ||
fail "No branch to report on: HEAD is detached, RAN_BRANCH is unset, and no PR has this commit as its head"
- Files reviewed: 11/15 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+140
to
+141
| - Push with `git push <remote> HEAD:refs/heads/$HEAD_BRANCH`, naming the ref. A worktree checked out at the PR head has no current branch, and a bare `git push` there fails with no upstream. | ||
| - Push only when the PR's head repo is `$REPO`. A fork PR's head repo has no local remote, so report that and leave the commit unpushed. |
Comment on lines
+58
to
+59
| branch=$(resolve_branch_name network) || | ||
| fail "No branch to report on: HEAD is detached, RAN_BRANCH is unset, and no PR has this commit as its head" |
| **Always use `gh` CLI** for GitHub operations. Never use GitHub MCP server tools. | ||
|
|
||
| **Never post PR review comments without explicit user approval.** See the `github-pr-operations` skill for endpoint reference and thread-resolution commands. | ||
| **Never post a PR review comment or a reply to a human reviewer without explicit user approval.** A reply to a bot reviewer's comment (Copilot, ReviewHog, Greptile, Graphite) may also go out unasked under a skill's explicit `--unattended` mode, which is what lets an unattended sweep leave bot threads resolved. See the `github-pr-operations` skill for endpoint reference and thread-resolution commands. |
Abort address-pr-reviews Step 1 when HEAD has moved off the PR head it resolved, so an invocation from a stale or different checkout can't push unrelated commits onto the PR's branch. Give resolve_branch_name a persisted-file cache tier, scoped to the checkout's private git dir, so a branch resolved via RAN_BRANCH or the network tier survives into a later process in the same checkout. This is what lets /ran read a detached checkout's branch after a commit moves HEAD off the PR head it was resolved from, such as under address-pr-reviews' --no-push path. Stop restating AGENTS.md's posting rule in github-pr-operations/SKILL.md, so the two can't drift out of sync again. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Generated-By: PostHog Desktop Task-Id: 4e81ba70-85c0-47fa-9cf7-26fce96f35a1
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.
Problem
An agent harness (PostHog Desktop, cloud runners) checks the PR head out detached.
git branch --show-currentgives an empty string there. Three tools stop:git-prcannot find the PR, because it looks the PR up by branch name.log-step-done.shrefuses to write a record, so/ranand/gosee the pass as never run.ran-report.shrefuses to report.address-pr-reviewsalso stops in a task runner. It asks for approval in prose at three steps. A task runner reads a prose question as the end of the turn, so the run stops with the work half done.Changes
Branch resolution.
git-prasks the commit-to-PR endpoint for HEAD's SHA when no branch is checked out. Only an exacthead.shamatch counts, because the endpoint also lists the PRs that a commit merged into.repo-context.shgets a newresolve_branch_namewith three tiers: the current branch, thenRAN_BRANCH, then the same endpoint.log-step-done.shandran-report.shcall it.log-command.shkeeps its network-free contract and stays on tier one.Approval gates.
address-pr-reviewsasks its three gates through the harness question tool (AskUserQuestion) when the harness has one. A new--unattendedflag takes each gate's default: apply every fix, reply to and resolve bot threads, commit, and push. The skill never replies to a human reviewer, attended or not.babysit-prspasses the flag on dispatch.wait-for-pr-reviewsdoes not, because/gochains it in front of a person.PR resolution.
address-pr-reviewsresolves the PR once, in Step 1, withdetect-pr.sh --json. It keepsHEAD_BRANCHand pushes with an explicit ref. A second lookup would fail, because the first commit moves HEAD off the PR head.Tests
New cases cover the detached path in four suites:
test-git-pr.sh(new file),test-repo-context.sh,test-log-step-done.sh, andtest-ran-report.sh. Each one shimsghon PATH, so the suite stays offline.test-git-pr.shjoins the CI list.All 12 CI suites pass locally.
Created with PostHog Desktop