Skip to content

#501 Default merge-pr to remote-only branch deletion via tristate flag #504

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
williamthorsen merged 2 commits into from
May 2, 2026
Merged

#501 Default merge-pr to remote-only branch deletion via tristate flag #504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0fa5f92
agents|fix: Default merge-pr to remote-only branch deletion via trist…
williamthorsen May 2, 2026
1828a48
agents|fix: Clarify --delete-branch is conditional in merge-gh-pr exa…
williamthorsen May 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 15 additions & 15 deletions packages/agents/content/skills/merge-bb-pr/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,28 +12,28 @@ Internal delegate that would merge a pull request on Bitbucket. Called by `merge

## Delegate interface

| Input | Type | Description |
| ------------------- | ------------------------------- | ----------------------------------------------- |
| `pr_number` | number | PR to merge |
| `title` | string | Pre-rendered merge-commit title |
| `body` | string | Pre-composed merge-commit body |
| `strategy` | `squash` \| `merge` \| `rebase` | Concrete strategy (no `prompt` sentinel) |
| `delete_branch` | boolean | Whether to delete the source branch after merge |
| `ticket_id` | string | Ticket ID for artifact path resolution |
| `project_slug` | string | Project slug for artifact path resolution |
| `artifact_base_dir` | string | Base directory for artifact storage |
| Input | Type | Description |
| ------------------- | ------------------------------- | ----------------------------------------- |
| `pr_number` | number | PR to merge |
| `title` | string | Pre-rendered merge-commit title |
| `body` | string | Pre-composed merge-commit body |
| `strategy` | `squash` \| `merge` \| `rebase` | Concrete strategy (no `prompt` sentinel) |
| `deletion_strategy` | `both` \| `remote` \| `none` | Which branches to delete after merge |
| `ticket_id` | string | Ticket ID for artifact path resolution |
| `project_slug` | string | Project slug for artifact path resolution |
| `artifact_base_dir` | string | Base directory for artifact storage |

## Process

Print a notice listing the resolved title, body, strategy, and delete-branch decision, then exit successfully without invoking any Bitbucket API. The user merges manually using the Bitbucket UI or CLI.
Print a notice listing the resolved title, body, strategy, and deletion strategy, then exit successfully without invoking any Bitbucket API. The user merges manually using the Bitbucket UI or CLI.

```
Bitbucket merge is not yet implemented. Resolved values:

PR: {pr_number}
Title: {title}
Strategy: {strategy}
Delete branch: {delete_branch}
PR: {pr_number}
Title: {title}
Strategy: {strategy}
Deletion strategy: {deletion_strategy}

Body:
{body}
Expand Down
64 changes: 47 additions & 17 deletions packages/agents/content/skills/merge-gh-pr/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,16 @@ Internal delegate that merges a pull request on GitHub. Called by `merge-pr` wit

## Delegate interface

| Input | Type | Description |
| ------------------- | ------------------------------- | ----------------------------------------------- |
| `pr_number` | number | PR to merge |
| `title` | string | Pre-rendered merge-commit title |
| `body` | string | Pre-composed merge-commit body |
| `strategy` | `squash` \| `merge` \| `rebase` | Concrete strategy (no `prompt` sentinel) |
| `delete_branch` | boolean | Whether to delete the source branch after merge |
| `ticket_id` | string | Ticket ID for artifact path resolution |
| `project_slug` | string | Project slug for artifact path resolution |
| `artifact_base_dir` | string | Base directory for artifact storage |
| Input | Type | Description |
| ------------------- | ------------------------------- | ----------------------------------------- |
| `pr_number` | number | PR to merge |
| `title` | string | Pre-rendered merge-commit title |
| `body` | string | Pre-composed merge-commit body |
| `strategy` | `squash` \| `merge` \| `rebase` | Concrete strategy (no `prompt` sentinel) |
| `deletion_strategy` | `both` \| `remote` \| `none` | Which branches to delete after merge |
| `ticket_id` | string | Ticket ID for artifact path resolution |
| `project_slug` | string | Project slug for artifact path resolution |
| `artifact_base_dir` | string | Base directory for artifact storage |

## Process

Expand All @@ -28,11 +28,13 @@ Internal delegate that merges a pull request on GitHub. Called by `merge-pr` wit
Use a single `gh pr view` call to fetch every field needed for validation:

```bash
gh pr view {pr_number} --json state,isDraft,mergeable,mergeStateStatus,reviewDecision,headRefName,isCrossRepository,baseRefName
gh pr view {pr_number} --json state,isDraft,mergeable,mergeStateStatus,reviewDecision,headRefName,isCrossRepository,baseRefName,headRepository,headRepositoryOwner
```

Parse the JSON with a real parser (`python3 -c "import sys,json; ..."` or `jq`). Do not regex-extract.

`headRepository` and `headRepositoryOwner` are needed by step 6's remote-deletion API call so that cross-repo PRs (`isCrossRepository == true`) target the correct head repo rather than the base repo.

### 2. Run pre-merge checks

Refuse the merge with a specific reason on any of the following. Each refusal exits non-zero and prints the reason on stderr:
Expand Down Expand Up @@ -93,19 +95,40 @@ For `squash`, pass `--subject "{title}"` so the rendered title becomes the merge

For `body`, pass `--body-file "$body_path"` only when `strategy` is `squash` or `merge`. Skip the flag for `rebase` — rebased commits retain their original messages, so the composed merge body has nothing to attach to. Passing `--body-file` to `gh pr merge --rebase` may surface a CLI error depending on the `gh` version, so omit it defensively.

For `delete_branch`, append `--delete-branch` iff the boolean is true.
For `deletion_strategy`, append `--delete-branch` iff the value is `both`. Skip for `remote` and `none` — `remote` is handled by the new post-merge step below; `none` skips deletion entirely.

Example invocation (shown for `strategy=squash`, `deletion_strategy=both` — `--delete-branch` is included **only** when `deletion_strategy == 'both'`):

```bash
gh pr merge {pr_number} \
--squash \
--subject "{title}" \
--body-file "$body_path" \
--delete-branch
--delete-branch # only when deletion_strategy == 'both'
```

If `gh pr merge` exits non-zero, surface its stderr to the user and exit non-zero. Do not retry, do not bypass with `--admin`.

### 6. Capture merge result
### 6. Delete remote branch (when deletion_strategy is `remote`)

Skip this step entirely when `deletion_strategy` is not `remote` — `both` is handled by step 5's `--delete-branch`, and `none` requests no deletion.

When `deletion_strategy == 'remote'`, resolve the head-repo coordinates and call the GitHub refs API directly. The head repo is the source of the branch — for same-repo PRs it equals the base repo; for cross-repo PRs (`isCrossRepository == true`) it lives on the contributor's fork. Use `headRepositoryOwner.login` and `headRepository.name` from the step 1 response:

```bash
gh api -X DELETE "repos/{headRepositoryOwner.login}/{headRepository.name}/git/refs/heads/{headRefName}"
```

Capture stdout, stderr, and the exit code.

- **On success:** set `remote_deletion_status = "deleted"`.
- **On failure:** print `warning: failed to delete remote branch '{headRefName}': {stderr}` to stderr, but **do not** exit non-zero — the merge itself succeeded, and re-deleting a leftover branch is trivial. Set `remote_deletion_status = "deletion-failed: {first line of stderr}"`.

For `deletion_strategy in {both, none}`, set `remote_deletion_status` directly from the strategy without making an API call: `both` → `"deleted"` (the prior `gh pr merge --delete-branch` handled it; if `gh pr merge` succeeded, the remote ref is gone); `none` → `"preserved"`.

Symmetrically, set `local_deletion_status`: `both` → `"deleted"`; `remote` or `none` → `"preserved"`.

### 7. Capture merge result

Fetch the resulting commit SHA after the merge:

Expand All @@ -115,7 +138,7 @@ gh pr view {pr_number} --json mergeCommit,url,mergedAt

Extract `mergeCommit.oid` (the merge commit SHA), `url` (PR URL), and `mergedAt` (ISO 8601 timestamp).

### 7. Save merge artifact
### 8. Save merge artifact

Save a `merge` artifact in the ticket directory.

Expand All @@ -142,20 +165,27 @@ PR: {url}
Merged at: {mergedAt}
Merge commit: {mergeCommit.oid}
Strategy: {strategy}
Branch: {headRefName} ({deleted | preserved})
Branch: {headRefName} (local: {local_deletion_status}, remote: {remote_deletion_status})

## Body

{body as submitted}
```

The branch line has four canonical renderings, depending on `deletion_strategy` and the outcome of the post-merge API call:

- `Branch: my-branch (local: deleted, remote: deleted)` — `both`, success
- `Branch: my-branch (local: preserved, remote: deleted)` — `remote`, success
- `Branch: my-branch (local: preserved, remote: deletion-failed: {reason})` — `remote`, API call failed
- `Branch: my-branch (local: preserved, remote: preserved)` — `none`

## Completion

```
Merged: {url}
Commit: {mergeCommit.oid}
Strategy: {strategy}
Branch: {headRefName} ({deleted | preserved})
Branch: {headRefName} (local: {local_deletion_status}, remote: {remote_deletion_status})
Artifact saved: {artifact path}
```

Expand Down
32 changes: 16 additions & 16 deletions packages/agents/content/skills/merge-pr/SKILL.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,17 @@ Merge a pull request on the appropriate platform. Composes the merge-commit titl

## Optional arguments

| Flag | Effect | Default |
| --------------------- | ------------------------------------------------------------ | ------------------------------- |
| `--pr {n}` | Merge PR `{n}` instead of the PR for the current branch. | PR for the current branch |
| `--scope {scope}` | Override the inferred scope. | inferred (see resolution below) |
| `--type {type}` | Override the inferred work type. | inferred (see resolution below) |
| `--strategy {s}` | Override the merge strategy: `squash`, `merge`, or `rebase`. | `squash` |
| `--delete-branch {v}` | Override branch deletion: `yes` or `no`. | `yes` |
| Flag | Effect | Default |
| ----------------- | ------------------------------------------------------------ | ------------------------------- |
| `--pr {n}` | Merge PR `{n}` instead of the PR for the current branch. | PR for the current branch |
| `--scope {scope}` | Override the inferred scope. | inferred (see resolution below) |
| `--type {type}` | Override the inferred work type. | inferred (see resolution below) |
| `--strategy {s}` | Override the merge strategy: `squash`, `merge`, or `rebase`. | `squash` |
| `--delete {v}` | Override branch deletion: `both`, `remote`, or `none`. | `remote` |

## Reserved preference keys

`merge.strategy` and `merge.delete_branch` are **reserved keys** in `.agents/preferences.yaml` and `~/.agents/preferences.yaml`. They are not yet honored — this iteration uses the hard-coded defaults above. Setting them in preferences has no effect; CLI overrides are the only way to change the values today. The keys are reserved so that adding preference-file lookup later is a localized, additive change that does not require renaming or re-shaping the configuration surface.
`merge.strategy` and `merge.deletion_strategy` are **reserved keys** in `.agents/preferences.yaml` and `~/.agents/preferences.yaml`. They are not yet honored — this iteration uses the hard-coded defaults above. Setting them in preferences has no effect; CLI overrides are the only way to change the values today. The keys are reserved so that adding preference-file lookup later is a localized, additive change that does not require renaming or re-shaping the configuration surface.

## Process

Expand Down Expand Up @@ -68,14 +68,14 @@ The output is a JSON object with one entry per dimension:

Read `.scope.status` and `.type.status` with python3 (or jq). When `status` is `"resolved"`, use `.value` as the concrete value. When `status` is `"ambiguous"`, carry the `candidates` array forward to the approval gate.

### 4. Resolve strategy and delete-branch
### 4. Resolve strategy and deletion strategy

```
resolveStrategy(cliOverride): return cliOverride ?? 'squash'
resolveDeleteBranch(cliOverride): return cliOverride ?? true
resolveStrategy(cliOverride): return cliOverride ?? 'squash'
resolveDeletionStrategy(cliOverride): return cliOverride ?? 'remote'
```

These are intentionally written as named functions with an explicit pipeline so adding preference-file lookup later means inserting one stage. Map `--delete-branch yes` → `true`, `--delete-branch no` → `false`.
These are intentionally written as named functions with an explicit pipeline so adding preference-file lookup later means inserting one stage. `--delete both|remote|none` map directly to the same string values.

### 5. Render merge-commit title

Expand Down Expand Up @@ -135,7 +135,7 @@ Proposed merge for PR #{pr_number}:

Title: {merge_title}
Strategy: {strategy}
Delete: {delete_branch ? 'yes' : 'no'}
Delete: {deletion_strategy}

Body:
{body}
Expand Down Expand Up @@ -163,7 +163,7 @@ Pass the following inputs to the selected delegate per the delegate interface:
| `title` | Rendered `merge_title` from step 5 (re-rendered after gate resolution) |
| `body` | Composed body from step 6 |
| `strategy` | Resolved strategy from step 4 |
| `delete_branch` | Resolved boolean from step 4 |
| `deletion_strategy` | Resolved value from step 4 (`both` \| `remote` \| `none`) |
| `ticket_id` | From session context |
| `project_slug` | From session context |
| `artifact_base_dir` | From session context |
Expand All @@ -172,7 +172,7 @@ The orchestrator never passes ambiguous-status dimensions or `prompt` sentinels

## Important

- The orchestrator owns all decisions (PR resolution, scope/type/strategy/delete-branch resolution, body composition, approval gate). Delegates own only execution (platform API calls + state validation).
- Local state is intentionally untouched after the merge. Branch deletion happens on the remote per the resolved decision; the local working copy and current branch are not modified. A separate skill may handle local cleanup later.
- The orchestrator owns all decisions (PR resolution, scope/type/strategy/deletion-strategy resolution, body composition, approval gate). Delegates own only execution (platform API calls + state validation).
- Local state is intentionally untouched after the merge. Branch deletion happens on the remote per the resolved decision; the local working copy and current branch are not modified. A separate skill may handle local cleanup later. The default `remote` mode deletes the remote branch via a post-merge `gh api -X DELETE` call (delegated to `merge-gh-pr`); `both` mode passes `--delete-branch` to `gh pr merge`, which is incompatible with worktree-based workflows — `gh pr merge --delete-branch` fails when the base branch is held by another worktree.
- Never bypass branch protections. The orchestrator does not expose `--admin`; users who need that capability run `gh pr merge --admin` directly.
- Never list automated checks (formatting, linting, typechecking, unit tests) in the merge body. They run automatically in CI.
Loading