diff --git a/packages/agents/content/skills/merge-bb-pr/SKILL.md b/packages/agents/content/skills/merge-bb-pr/SKILL.md index a3cda44f..f3d9d37d 100644 --- a/packages/agents/content/skills/merge-bb-pr/SKILL.md +++ b/packages/agents/content/skills/merge-bb-pr/SKILL.md @@ -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} diff --git a/packages/agents/content/skills/merge-gh-pr/SKILL.md b/packages/agents/content/skills/merge-gh-pr/SKILL.md index 2ac90987..93d6c78e 100644 --- a/packages/agents/content/skills/merge-gh-pr/SKILL.md +++ b/packages/agents/content/skills/merge-gh-pr/SKILL.md @@ -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 @@ -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: @@ -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: @@ -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. @@ -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} ``` diff --git a/packages/agents/content/skills/merge-pr/SKILL.md b/packages/agents/content/skills/merge-pr/SKILL.md index a6ac5a22..382f9f5c 100644 --- a/packages/agents/content/skills/merge-pr/SKILL.md +++ b/packages/agents/content/skills/merge-pr/SKILL.md @@ -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 @@ -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 @@ -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} @@ -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 | @@ -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.