john/claude-engineering-plugin
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(git-commit-push-pr): quiet expected no-pr gh exit (#439)

Browse Source
This commit is contained in:
Trevin Chow
2026-03-29 11:45:27 -07:00
committed by GitHub
parent bd02ca7df0
commit 1f499482bc
2 changed files with 42 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
docs/solutions/skill-design/git-workflow-skills-need-explicit-state-machines-2026-03-27.md
View File

@@ -42,6 +42,7 @@ The `git-commit` and `git-commit-push-pr` skills had accumulated branch-state an
- Using a single early `git branch --show-current` result and referring back to it later. Once the workflow creates a branch, the earlier value is stale.
- Using `git diff HEAD` as the definition of "has changes." It does not account for untracked files.
- Treating every non-zero exit from `gh pr view` as a fatal failure. "No PR for this branch" is often a normal branch state.
- Letting the shell tool surface that expected `gh pr view` non-zero exit as a visible failed step. Even when the logic recovers correctly, the UX looks broken and pushes future edits toward less-correct commands.
- Switching from `gh pr view` to `gh pr list --head "<branch>"` to avoid the no-PR error path. This improved ergonomics but weakened correctness because `gh pr list` cannot disambiguate `<owner>:<branch>`.
- Adding a "clean working tree" fast path before re-checking whether the current branch was still the default branch. That let the workflow skip the feature-branch safety gate and head straight toward invalid push/PR transitions.
@@ -112,6 +113,8 @@ Interpret it as a state check:
- Non-zero exit with output indicating no PR for the current branch -> expected "no PR yet" state
- Any other failure -> real error
When the shell/tooling layer renders non-zero exits as scary visible failures, wrap the command so the skill captures both the output and exit code and then interprets them explicitly. The user should see "no PR for this branch" as a normal state transition, not as a broken Bash step.
This keeps PR detection tied to the current branch context instead of a bare branch name that may be reused across forks.
### 5. Keep the default-branch safety gate ahead of push/PR transitions
@@ -233,6 +236,7 @@ Learning: these skills should be designed and reviewed like tiny state machines,
- Re-run the command that answers the current question at the point of decision. Do not rely on values gathered earlier if a mutating command may have changed them.
- Use `git status` for "is there local work?" and reserve `git diff` for describing content, not determining whether work exists.
- Model expected non-zero CLI exits explicitly when they represent state, such as `gh pr view` on a branch with no PR.
- When a tool visually highlights non-zero exits as failures, capture the exit code yourself for expected state probes so correct logic does not still look broken to the user.
- Avoid branch-name-only PR detection for multi-fork repos. If the command cannot disambiguate branch ownership, prefer a current-branch-aware command even if the failure path is slightly messier.
- Keep default-branch safety checks in every path that can lead to push or PR creation, including "clean working tree but unpushed commits" shortcuts.
- When editing skill logic, manually walk these cases before considering the change complete:

44
plugins/compound-engineering/skills/git-commit-push-pr/SKILL.md
View File

@@ -13,6 +13,28 @@ If the user is asking to update, refresh, or rewrite an existing PR description
For description-only updates, follow the Description Update workflow below. Otherwise, follow the full workflow.
## Reusable PR probe
When checking whether the current branch already has a PR, keep using current-branch `gh pr view` semantics. Do **not** switch to `gh pr list --head "<branch>"` just to avoid the no-PR exit path. That branch-name search can select the wrong PR in multi-fork repos.
Also do **not** run bare `gh pr view --json ...` in a way that lets the shell tool render the expected no-PR state as a red failed step. Capture the output and exit code yourself so you can interpret "no PR for this branch" as normal workflow state:
```bash
if PR_VIEW_OUTPUT=$(gh pr view --json url,title,state 2>&1); then
PR_VIEW_EXIT=0
else
PR_VIEW_EXIT=$?
fi
printf '%s\n__GH_PR_VIEW_EXIT__=%s\n' "$PR_VIEW_OUTPUT" "$PR_VIEW_EXIT"
```
Interpret the result this way:
- `__GH_PR_VIEW_EXIT__=0` and JSON with `state: OPEN` -> an open PR exists for the current branch
- `__GH_PR_VIEW_EXIT__=0` and JSON with a non-OPEN state -> treat as no open PR
- non-zero exit with output indicating `no pull requests found for branch` -> expected no-PR state
- any other non-zero exit -> real error (auth, network, repo config, etc.)
---
## Description Update workflow
@@ -36,10 +58,15 @@ If empty (detached HEAD), report that there is no branch to update and stop.
Otherwise, check for an existing open PR:
```bash
gh pr view --json url,title,state
if PR_VIEW_OUTPUT=$(gh pr view --json url,title,state 2>&1); then
PR_VIEW_EXIT=0
else
PR_VIEW_EXIT=$?
fi
printf '%s\n__GH_PR_VIEW_EXIT__=%s\n' "$PR_VIEW_OUTPUT" "$PR_VIEW_EXIT"
```
Interpret the result. Do not treat every non-zero exit as a fatal error here:
Interpret the result using the Reusable PR probe rules above:
- If it returns PR data with `state: OPEN`, an open PR exists for the current branch.
- If it returns PR data with a non-OPEN state (CLOSED, MERGED), treat this as "no open PR." Report that no open PR exists for this branch and stop.
@@ -129,10 +156,15 @@ Run `git branch --show-current` to get the current branch name. If it returns an
Then check for an existing open PR:
```bash
gh pr view --json url,title,state
if PR_VIEW_OUTPUT=$(gh pr view --json url,title,state 2>&1); then
PR_VIEW_EXIT=0
else
PR_VIEW_EXIT=$?
fi
printf '%s\n__GH_PR_VIEW_EXIT__=%s\n' "$PR_VIEW_OUTPUT" "$PR_VIEW_EXIT"
```
Interpret the result. Do not treat every non-zero exit as a fatal error here:
Interpret the result using the Reusable PR probe rules above:
- If it **returns PR data with `state: OPEN`**, an open PR exists for the current branch. Note the URL and continue to Step 4 (commit) and Step 5 (push). Then skip to Step 7 (existing PR flow) instead of creating a new PR.
- If it **returns PR data with a non-OPEN state** (CLOSED, MERGED), treat this the same as "no PR exists" -- the previous PR is done and a new one is needed. Continue to Step 4 through Step 8 as normal.
@@ -205,11 +237,11 @@ Once the base branch and remote are known:
```bash
git merge-base <base-remote>/<base-branch> HEAD
```
2. List all commits unique to this branch:
3. List all commits unique to this branch:
```bash
git log --oneline <merge-base>..HEAD
```
3. Get the full diff a reviewer will see:
4. Get the full diff a reviewer will see:
```bash
git diff <merge-base>...HEAD
```