From 813a44990cc51913f314bc18c6fda0d47c5d8602 Mon Sep 17 00:00:00 2001 From: Matt Bishop Date: Wed, 26 Aug 2026 08:59:25 -0400 Subject: [PATCH] feat(bitwarden-delivery-tools): add stacked pull request support MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GitHub's gh-stack extension is now in use across the organization, but Bitwarden's PR conventions are per-PR and `gh stack submit` accepts no title, body, or label flag — so a stack submitted the documented way ships every layer with a generated title, no `t:` label, and no template body. The new `stacking-pull-requests` skill owns that conventions layer and delegates every `gh stack` command to GitHub's own `gh-stack` skill, gating on both the extension and the skill being present and falling back to a single-branch PR when they are not. The three existing mechanics skills gain the per-layer semantics a stack implies. --- .claude-plugin/marketplace.json | 4 +- .cspell.json | 1 + README.md | 2 +- .../.claude-plugin/plugin.json | 5 +- plugins/bitwarden-delivery-tools/CHANGELOG.md | 26 ++ plugins/bitwarden-delivery-tools/README.md | 23 +- .../skills/committing-changes/SKILL.md | 6 +- .../skills/committing-changes/evals/README.md | 2 +- .../evals/trigger-eval.json | 4 + .../skills/creating-pull-request/SKILL.md | 33 +- .../creating-pull-request/evals/README.md | 5 +- .../references/caller-integration.md | 59 +++ .../skills/force-multiplier/SKILL.md | 2 +- .../skills/perform-preflight/SKILL.md | 28 +- .../skills/stacking-pull-requests/SKILL.md | 138 +++++++ .../stacking-pull-requests/evals/README.md | 48 +++ .../evals/run_real_eval.py | 348 ++++++++++++++++++ .../evals/trigger-eval.json | 82 +++++ .../references/installing-gh-stack.md | 80 ++++ .../references/submitting-a-stack.md | 109 ++++++ 20 files changed, 986 insertions(+), 19 deletions(-) create mode 100644 plugins/bitwarden-delivery-tools/skills/creating-pull-request/references/caller-integration.md create mode 100644 plugins/bitwarden-delivery-tools/skills/stacking-pull-requests/SKILL.md create mode 100644 plugins/bitwarden-delivery-tools/skills/stacking-pull-requests/evals/README.md create mode 100644 plugins/bitwarden-delivery-tools/skills/stacking-pull-requests/evals/run_real_eval.py create mode 100644 plugins/bitwarden-delivery-tools/skills/stacking-pull-requests/evals/trigger-eval.json create mode 100644 plugins/bitwarden-delivery-tools/skills/stacking-pull-requests/references/installing-gh-stack.md create mode 100644 plugins/bitwarden-delivery-tools/skills/stacking-pull-requests/references/submitting-a-stack.md diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 62e17acf..af21dce7 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -78,8 +78,8 @@ { "name": "bitwarden-delivery-tools", "source": "./plugins/bitwarden-delivery-tools", - "version": "3.1.0", - "description": "Delivery lifecycle skills for Bitwarden initiatives — initiative funnel navigation, work transitions, architectural judgment, commits, pull requests, preflight checks, change labeling, and Jira ticket filing." + "version": "3.2.0", + "description": "Delivery lifecycle skills for Bitwarden initiatives — initiative funnel navigation, work transitions, architectural judgment, commits, pull requests, stacked pull requests, preflight checks, change labeling, and Jira ticket filing." }, { "name": "bitwarden-designer", diff --git a/.cspell.json b/.cspell.json index cf2ef723..5d7a182f 100644 --- a/.cspell.json +++ b/.cspell.json @@ -88,6 +88,7 @@ "mcp", "MECE", "metacharacters", + "mktemp", "modelcontextprotocol", "msword", "MVVM", diff --git a/README.md b/README.md index 98caa23d..58daf5c5 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ A curated collection of plugins for AI-assisted development at Bitwarden. Enable | [bitwarden-shepherd](plugins/bitwarden-shepherd/) | 1.0.1 | Champion of a technical strategy — shepherds a TSI through evaluation into the funnel, then through to adoption | | [bitwarden-atlassian-tools](plugins/bitwarden-atlassian-tools/) | 2.7.0 | Atlassian access via MCP server with deep Jira issue research skill and opt-in Jira write tools | | [bitwarden-code-review](plugins/bitwarden-code-review/) | 2.1.0 | Autonomous code review agent following Bitwarden engineering standards with GitHub integration | -| [bitwarden-delivery-tools](plugins/bitwarden-delivery-tools/) | 3.1.0 | Delivery lifecycle skills: initiative funnel navigation, work transitions, architectural judgment, commits, PRs, preflight, labeling, Jira ticket filing | +| [bitwarden-delivery-tools](plugins/bitwarden-delivery-tools/) | 3.2.0 | Delivery lifecycle skills: initiative funnel, work transitions, architectural judgment, commits, PRs and stacks, preflight, labeling, Jira filing | | [bitwarden-designer](plugins/bitwarden-designer/) | 0.1.0 | Product designer persona: Code of Conduct and 30/60/90 critique, critique facilitation; dispatches into bitwarden-design-tools | | [bitwarden-design-tools](plugins/bitwarden-design-tools/) | 0.1.0 | Design toolkit: content style guide, Figma Dev Mode MCP, Bitwarden brand application, handoff prep, Design System governance, Product and Design Jira | | [bitwarden-devops-engineer](plugins/bitwarden-devops-engineer/) | 0.3.0 | DevOps engineering assistant: workflow compliance linting, action security auditing, and org-wide CI/CD remediation | diff --git a/plugins/bitwarden-delivery-tools/.claude-plugin/plugin.json b/plugins/bitwarden-delivery-tools/.claude-plugin/plugin.json index 653cbf7a..9b65e293 100644 --- a/plugins/bitwarden-delivery-tools/.claude-plugin/plugin.json +++ b/plugins/bitwarden-delivery-tools/.claude-plugin/plugin.json @@ -1,7 +1,7 @@ { "name": "bitwarden-delivery-tools", - "version": "3.1.0", - "description": "Delivery lifecycle skills for Bitwarden initiatives — initiative funnel navigation, work transitions, architectural judgment, commits, pull requests, preflight checks, change labeling, and Jira ticket filing.", + "version": "3.2.0", + "description": "Delivery lifecycle skills for Bitwarden initiatives — initiative funnel navigation, work transitions, architectural judgment, commits, pull requests, stacked pull requests, preflight checks, change labeling, and Jira ticket filing.", "author": { "name": "Bitwarden", "url": "https://github.com/bitwarden" @@ -15,6 +15,7 @@ "work-transition", "commit", "pull-request", + "stacked-pull-requests", "preflight", "labeling", "jira" diff --git a/plugins/bitwarden-delivery-tools/CHANGELOG.md b/plugins/bitwarden-delivery-tools/CHANGELOG.md index 98df5280..531bd1a4 100644 --- a/plugins/bitwarden-delivery-tools/CHANGELOG.md +++ b/plugins/bitwarden-delivery-tools/CHANGELOG.md @@ -5,6 +5,32 @@ All notable changes to the `bitwarden-delivery-tools` plugin will be documented The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [3.2.0] - 2026-08-26 + +### Added + +- **`stacking-pull-requests` skill** — Bitwarden's per-PR conventions across a chain of dependent pull requests: layer planning, per-layer gates, a stack-level submission preview, lower-layer feedback, and merging. `gh stack` mechanics are delegated to GitHub's `gh-stack` skill and extension, with an availability check and a single-branch fallback +- `references/submitting-a-stack.md` — the two paths that carry a title, body, and `t:` label onto every layer; `gh stack link` is passed `--base` so the bottom layer's base is not silently rewritten to the repository default branch +- `references/installing-gh-stack.md` and `creating-pull-request/references/caller-integration.md`, holding install and caller-routing detail out of the always-loaded files +- `skills/stacking-pull-requests/evals/` — 20-case trigger eval, runner, and README. The runner denies the mutating tools, loads no MCP servers, and runs each subprocess in a temp directory, since the query set is imperative. No baseline is committed +- `perform-preflight`: Stacked Branches section covering the current layer, with its own stack-detection test; a skipped section is reported rather than passing silently +- `stacking-pull-requests` Step 1 registers the planned layers with `gh stack init`, so submission does not fail after every layer has been gated and previewed +- `stacking-pull-requests` Step 5 confirms before rebasing layers above a fix, and Step 6 confirms before both the draft flip and the merge +- `perform-preflight`: a failed rebase checkbox is reported rather than auto-fixed, since the fix force-pushes every layer above + +### Changed + +- `creating-pull-request`: routes to `stacking-pull-requests` on the user's intent to open a chain, and never when that skill is the caller; per-layer callbacks run its Steps 2–3, with the label question asked once for the stack +- `creating-pull-request`: Step 1b names the base ref instead of hardcoding `origin/HEAD`. Only the `Substantial` path can be scoped to one layer +- `creating-pull-request`: the nested-caller exemption applies only when the caller actually ran a review +- `creating-pull-request`: Step 6 passes `--base ` for a branch cut from `rc`, `hotfix-rc`, or another release branch, which `gh pr create` would otherwise point at the repository default branch +- `force-multiplier`: announces itself when invoking `creating-pull-request`, so that skill's review gate does not fire mid-campaign +- `committing-changes`: the full commit format applies per layer, and preflight is scoped to the layer being committed to +- Stack detection reads `gh stack view --json`'s exit code and requires a payload naming the current branch; `stacking-pull-requests` Step 0 owns what each status means +- `creating-pull-request`: stack exclusion added to `description`; its eval baseline needs regenerating before release +- `committing-changes` gains a single-layer eval case for its new scope clause. The stack boundary cannot be measured from these skills' own eval sets — correct routing goes to `stacking-pull-requests`, which then invokes them per layer, and the runner matches its token anywhere in the response — so it is measured from `stacking-pull-requests/evals/` only. Both baselines, and that one, still need recording before release +- `plugin.json`: description and `stacked-pull-requests` keyword added; marketplace and README catalog entries follow + ## [3.1.0] - 2026-08-19 ### Added diff --git a/plugins/bitwarden-delivery-tools/README.md b/plugins/bitwarden-delivery-tools/README.md index 8565c857..dc4c8f15 100644 --- a/plugins/bitwarden-delivery-tools/README.md +++ b/plugins/bitwarden-delivery-tools/README.md @@ -1,6 +1,6 @@ # Bitwarden Delivery Tools -Delivery lifecycle skills for Bitwarden initiatives — from routing work through the Software Initiative Funnel and running cross-team work transitions, down to the day-to-day mechanics of committing, opening pull requests, running preflight checks, and labeling changes. +Delivery lifecycle skills for Bitwarden initiatives — from routing work through the Software Initiative Funnel and running cross-team work transitions, down to the day-to-day mechanics of committing, opening pull requests and stacks of them, running preflight checks, and labeling changes. ## Overview @@ -40,7 +40,8 @@ Any agent (tech-lead, software-engineer, shepherds, others) can compose these sk | `filing-breakdown-tasks` | "tickets from tasks.md", "file the epic and stories" | Turn a breakdown's `tasks.md` into epic + child ticket drafts for `filing-jira-tickets` to file | | `force-multiplier` | "across all repos", "in bulk" | Fan one change across many repos or monorepo projects as isolated, piloted draft PRs | | `labeling-changes` | "label", "change type" | Conventional commit type keywords, CI label mapping | -| `perform-preflight` | "preflight", "self review" | Pre-commit quality gate checklist | +| `perform-preflight` | "preflight", "self review" | Pre-commit quality gate checklist, including the per-layer checks for a stacked branch | +| `stacking-pull-requests` | "stack these PRs", "stacked diffs" | Bitwarden conventions across a chain of dependent PRs; mechanics delegated to `gh-stack` | ## Design Principle @@ -56,6 +57,20 @@ Several skills in this plugin reference tools or skills provided by sibling plug - **`bitwarden-security-engineer`** — provides `Skill(bitwarden-security-context)`, referenced from `architecting-solutions`. - **`bitwarden-code-review`** — provides `/bitwarden-code-review:code-review-local` and `Skill(performing-multi-agent-code-review)`, the code-review gate `creating-pull-request` runs before opening a PR. If it is absent, `creating-pull-request` prompts you to install it rather than skip the review. +`stacking-pull-requests` also invokes `Skill(addressing-code-review-comments)` from `bitwarden-code-review` when handling feedback on a lower layer, and treats it as optional. + +It additionally depends on two pieces of tooling outside this marketplace, both from GitHub's [`gh-stack`](https://github.com/github/gh-stack) repository, which owns the `gh stack` command surface. Install both — with only the extension, every stack request takes the single-branch fallback. + +1. The CLI extension: + + ```bash + gh extension install github/gh-stack --pin v0.1.0 + ``` + +2. The `gh-stack` skill, from that repository's `skills/gh-stack/` directory. `gh extension install` fetches a platform binary only and never places the skill, so install it as a Claude skill separately. + +`stacking-pull-requests` checks for both up front and falls back to a single-branch PR rather than improvising the commands. The documented flag sets were verified against `gh-stack` v0.1.0, the first release containing `gh stack merge`. It is pre-1.0 and still moving. + ## Installation ```bash @@ -82,6 +97,10 @@ Commit these changes Create a PR for this branch ``` +``` +Split this into a stack of dependent PRs +``` + ``` Run preflight before I commit ``` diff --git a/plugins/bitwarden-delivery-tools/skills/committing-changes/SKILL.md b/plugins/bitwarden-delivery-tools/skills/committing-changes/SKILL.md index c3117764..43ec6912 100644 --- a/plugins/bitwarden-delivery-tools/skills/committing-changes/SKILL.md +++ b/plugins/bitwarden-delivery-tools/skills/committing-changes/SKILL.md @@ -1,6 +1,6 @@ --- name: committing-changes -description: Git commit conventions and workflow for Bitwarden repositories. Use when committing code, writing commit messages, or preparing changes for commit. Triggered by "commit", "git commit", "commit message", "prepare commit", "stage changes". +description: Git commit conventions and workflow for Bitwarden repositories. Use when committing code, writing commit messages, or preparing changes for commit. Triggered by "commit", "git commit", "commit message", "prepare commit", "stage changes", including commits on a layer you are still building. Not for planning, gating, submitting, merging a stack, or fixing a layer that already has an open pull request (all stacking-pull-requests). --- # Git Commit Conventions @@ -49,8 +49,10 @@ Only the first commit on a branch needs the full format (ticket prefix, type key Update error handling in login flow ``` +In a stack every layer becomes its own pull request, so "first commit on a branch" means per layer: each layer's first commit carries the full format, and the type keyword is chosen for that layer rather than inherited from the layer below. + --- ## Pre-Commit Quality Gate -Before staging, run the `perform-preflight` skill for the full quality gate checklist (tests, lint, security, architecture). Consult the repo's CLAUDE.md for platform-specific build and lint commands. +Before staging, run the `perform-preflight` skill for the full quality gate checklist (tests, lint, security, architecture). Consult the repo's CLAUDE.md for platform-specific build and lint commands. On a stack, that checklist covers the layer you are committing to; `Skill(stacking-pull-requests)` runs it across every layer. Once a layer has an open pull request, a fix on it belongs to that skill's Step 5, which rebases and force-pushes the layers above behind a confirmation — do not commit it here and stop. diff --git a/plugins/bitwarden-delivery-tools/skills/committing-changes/evals/README.md b/plugins/bitwarden-delivery-tools/skills/committing-changes/evals/README.md index 72ab1707..70b0ea70 100644 --- a/plugins/bitwarden-delivery-tools/skills/committing-changes/evals/README.md +++ b/plugins/bitwarden-delivery-tools/skills/committing-changes/evals/README.md @@ -9,7 +9,7 @@ Two eval sets. Run the one that matches what changed. ## Trigger eval -Does the skill activate on the right phrasings and stay silent on near-misses? `trigger-eval.json` holds 13 queries; `run_real_eval.py` runs them (see `../../creating-pull-request/evals/run_real_eval.py` for why this runner exists instead of the skill-creator harness); `baseline.json` is the last known-good run. Requires Python 3.10+ and an authenticated `claude` CLI. +Does the skill activate on the right phrasings and stay silent on near-misses? `trigger-eval.json` holds 14 queries; the `stacking-pull-requests` boundary is not measurable from here, since a correctly-routed stack phrasing invokes this skill per layer and the runner matches its token anywhere in the response; `run_real_eval.py` runs them (see `../../creating-pull-request/evals/run_real_eval.py` for why this runner exists instead of the skill-creator harness); `baseline.json` is the last known-good run — **stale as of the stack-scope description change**: it holds 13 results against the current 14 queries. Regenerate before relying on the diff below — and note this runner takes no `--plugin-dir`, so it loads the installed plugin cache. Record the baseline after this version ships, not before, or it captures the description the change replaced. Requires Python 3.10+ and an authenticated `claude` CLI. ```bash python3 run_real_eval.py --eval-set trigger-eval.json --runs-per-query 3 \ diff --git a/plugins/bitwarden-delivery-tools/skills/committing-changes/evals/trigger-eval.json b/plugins/bitwarden-delivery-tools/skills/committing-changes/evals/trigger-eval.json index 0eacd65f..bdb7344a 100644 --- a/plugins/bitwarden-delivery-tools/skills/committing-changes/evals/trigger-eval.json +++ b/plugins/bitwarden-delivery-tools/skills/committing-changes/evals/trigger-eval.json @@ -50,5 +50,9 @@ { "query": "create a new branch for the PM-33210 work before I start coding, nothing to commit yet", "should_trigger": false + }, + { + "query": "commit what I have on this layer before I start the next one", + "should_trigger": true } ] diff --git a/plugins/bitwarden-delivery-tools/skills/creating-pull-request/SKILL.md b/plugins/bitwarden-delivery-tools/skills/creating-pull-request/SKILL.md index 872c5a7c..7700ebb0 100644 --- a/plugins/bitwarden-delivery-tools/skills/creating-pull-request/SKILL.md +++ b/plugins/bitwarden-delivery-tools/skills/creating-pull-request/SKILL.md @@ -1,7 +1,7 @@ --- name: creating-pull-request -description: Open a pull request from a branch in a Bitwarden repository — pick the conventional commit type prefix that drives the t: label, fill in the repo's PR template, choose an ai-review label, and confirm a full submission preview before running gh pr create. -when_to_use: Use when the user is ready to open a pull request from a branch — phrasings like "create a PR", "open a PR", "ship a draft", "put it up for review", "ready for review", or "ship it". Also use when drafting a PR title or body, picking the conventional commit type prefix, or choosing the t: or ai-review label for a PR being opened (takes precedence over labeling-changes in PR-creation contexts). Do not use for conceptual questions ("how do PRs work") or managing existing PRs (status, merging, addressing comments). +description: "Open a pull request from a branch in a Bitwarden repository — pick the conventional commit type prefix that drives the t: label, fill in the repo's PR template, choose an ai-review label, and confirm a full submission preview before running gh pr create. Not for a chain of dependent pull requests (that is stacking-pull-requests)." +when_to_use: 'Use when the user is ready to open a pull request from a branch — phrasings like "create a PR", "open a PR", "ship a draft", "put it up for review", "ready for review", or "ship it". Also use when drafting a PR title or body, picking the conventional commit type prefix, or choosing the t: or ai-review label for a PR being opened (takes precedence over labeling-changes in PR-creation contexts). Do not use for a chain of dependent pull requests (that is stacking-pull-requests), conceptual questions ("how do PRs work"), or managing existing PRs (status, merging, addressing comments).' --- # Creating a Pull Request @@ -18,6 +18,14 @@ Missing any one of these is silent — CI won't reject the PR, and the reviewer Follow these steps in order. Each one produces information the next step needs, and the preview in Step 5 depends on all of them. +**First, is this one pull request or a stack?** A request for a chain of dependent pull requests belongs to `Skill(stacking-pull-requests)` — hand off, unless another skill is driving this one. Three signals say a skill is driving, and each says something different about what to run: + +1. **The invocation names which of this workflow's steps it wants.** Run those steps and nothing else. +2. **It names the calling skill and states that skill owns the code review.** Run the whole workflow from Step 2 and skip the Step 1b gate. An ownership claim with no named caller does not qualify — run the gate. +3. **It states the stack path was already tried and declined.** Run the whole workflow, gate included — nobody owns the review on that path. + +Never hand off on any of the three; signal 3 in particular is `stacking-pull-requests` handing work _here_, so bouncing it back is the loop these signals exist to prevent. The table in Step 1b is authoritative on whether the gate runs. `${CLAUDE_PLUGIN_ROOT}/skills/creating-pull-request/references/caller-integration.md` maps which callers enter where. + ### Step 1 — Confirm preflight, then run the code-review gate A PR opened on broken work, or on work that skipped review, wastes reviewer time and buries the real problem under comment threads. Settle preflight first, then run the review. @@ -36,7 +44,9 @@ If preflight cannot be made to pass, stop and report the failure rather than ope - **Question**: "How deep is this change? (sets review depth)" - **Options**: - `Standard` — a typical feature, fix, docs, or config change: run `/bitwarden-code-review:code-review-local` (tell it to review the current branch's changes; there is no PR yet) - - `Substantial` — architectural, cross-cutting, or security-touching: run `Skill(performing-multi-agent-code-review)`, telling it to review the full branch diff against `origin/HEAD` (not just uncommitted changes); there is no PR yet + - `Substantial` — architectural, cross-cutting, or security-touching: run `Skill(performing-multi-agent-code-review)`, telling it to review the full branch diff against the base ref (not just uncommitted changes); there is no PR yet + +The base ref is `origin/HEAD` for a branch cut from trunk. When `stacking-pull-requests` is driving this per layer, the review has to be scoped to that layer or it re-reviews every layer below. Only `Substantial` can be scoped: use its commit-range mode with the explicit range `..`. `Standard` resolves `origin/HEAD` itself and takes no base ref, so on any layer above the bottom its diff reaches below the layer. Say so when asking — name `Substantial` as the layer-scoped option — and if the user picks `Standard` anyway, record in that layer's PR body that the review was not layer-scoped. Present only these two options; do not add a skip option. Honor a skip only if the user volunteers one unprompted, then record it in the PR body's Objective section (Step 3) and surface it in the Step 5 preview. Never skip on your own initiative. @@ -48,7 +58,19 @@ After the review: Each review path checks its own prerequisites and reports what to install if something is missing. If a path can't run, install what it reports or fall back to the other path and note the limitation in the PR body. If neither path is available, stop and prompt the user to install `bitwarden-code-review` (`/plugin install bitwarden-code-review@bitwarden-marketplace`) before continuing. Never silently skip the review. -When `creating-pull-request` runs as a step inside another delivery skill's workflow (for example a bulk campaign or a prototype PR), that workflow owns whether and how a review runs; skip this gate. Today neither in-plugin caller runs a review — wiring it in is a tracked follow-up. +When another delivery skill drives this workflow, whether this gate runs depends on how it was entered: + +Skip it only when a caller has already handled the review and its decision stands — never merely because a caller exists: + +| Entry | Step 1b gate | +| ---------------------------------------- | ----------------------------------------------------------------------- | +| A user, directly | Runs | +| `stacking-pull-requests` Step 2 | Runs — that is what it is asking for | +| `stacking-pull-requests` Step 3 | Skipped — Step 2 already ran it for that layer | +| `stacking-pull-requests` Step 0 fallback | Runs — that path bailed before its Step 2, so no caller owns the review | +| `force-multiplier` | Skipped — it owns the decision and runs no per-PR review | + +`${CLAUDE_PLUGIN_ROOT}/skills/creating-pull-request/references/caller-integration.md` explains each row. ### Step 2 — Determine change type and propose the title @@ -153,7 +175,8 @@ Defaults that hold unless the user said otherwise: - create as **draft** — only skip `--draft` if the user explicitly asked for a ready-for-review PR, - include `--label` only if the user picked a label in Step 4 (omit it for "No label"), -- multiple labels can be passed by repeating `--label`. +- multiple labels can be passed by repeating `--label`, +- omit `--base` for a branch cut from trunk, and pass `--base ` when the branch was cut from `rc`, `hotfix-rc`, or another release branch — with no `--base`, `gh pr create` targets the repository default branch and silently points the PR at trunk. Stack layers are submitted by `Skill(stacking-pull-requests)`, which passes each layer's `--base` itself; Step 6 does not run per layer. After `gh pr create` returns, post the PR URL back to the user. diff --git a/plugins/bitwarden-delivery-tools/skills/creating-pull-request/evals/README.md b/plugins/bitwarden-delivery-tools/skills/creating-pull-request/evals/README.md index 8b67e85f..2e60805b 100644 --- a/plugins/bitwarden-delivery-tools/skills/creating-pull-request/evals/README.md +++ b/plugins/bitwarden-delivery-tools/skills/creating-pull-request/evals/README.md @@ -9,8 +9,11 @@ The upstream `skill-creator` harness measures triggering by registering a tempor ## Files - `trigger-eval.json` — 20-query test set: 10 should-trigger natural-language phrasings ("package this up into a PR", "ship a draft", "get this in front of reviewers", etc.) and 10 should-not-trigger near-misses against sibling delivery skills (`committing-changes`, `labeling-changes`, `perform-preflight`) and against existing-PR management queries. + + The `stacking-pull-requests` boundary is **not** measurable here. Correct routing for a stack phrasing is `Skill(stacking-pull-requests)`, which then invokes this skill once per layer — and the runner counts a trigger when the token appears anywhere in the response, so a correctly-routed query scores as a false trigger. That boundary is measured from the other side, in `stacking-pull-requests/evals/`. + - `run_real_eval.py` — runner. Spawns parallel `claude -p` subprocesses, parses streamed tool-use events, computes per-query trigger rates. -- `baseline.json` — last known-good run. Diff against this to spot regressions on future description changes. +- `baseline.json` — last known-good run. **Stale**: the skill's `description` and `when_to_use` both changed without a re-run, so the recorded rates predate the current routing. Regenerate before relying on the diff below — and note this runner takes no `--plugin-dir`, so it loads the installed plugin cache. Record the baseline after this version ships, not before, or it captures the frontmatter the change replaced. ## Running diff --git a/plugins/bitwarden-delivery-tools/skills/creating-pull-request/references/caller-integration.md b/plugins/bitwarden-delivery-tools/skills/creating-pull-request/references/caller-integration.md new file mode 100644 index 00000000..c43dfbf8 --- /dev/null +++ b/plugins/bitwarden-delivery-tools/skills/creating-pull-request/references/caller-integration.md @@ -0,0 +1,59 @@ +# Who Calls This Workflow, and Where They Enter + +`creating-pull-request` is invoked directly by a user and also driven by two other delivery +skills. This file maps those edges. The workflow itself is in `SKILL.md`. + +## Telling a caller from a user + +The guard against handing off in a loop cannot read provenance, so it turns on three observable +signals. An invocation is from a caller if it **names the steps it wants** (run those and no +others), if it **names the calling skill and states that skill already owns the review** (run everything +from Step 2, gate skipped) — an assertion of ownership without a named caller does not qualify, +and the gate runs, or if it **states the stack path was already tried and declined** (run +everything, gate included). Anything else is a user request, and a user asking for a chain of +dependent pull requests gets handed off to `Skill(stacking-pull-requests)`. + +The third signal matters most: it is `stacking-pull-requests` Step 0 handing a single ordinary +pull request back here after its tooling check failed. Without it, that invocation looks exactly +like a user asking for a chain, gets handed straight back, and the two skills bounce. + +The second signal exists for `force-multiplier`, which enters at the top and names no step. On +the first signal alone it would read as a user request, the Step 1b gate would run mid-campaign, +and since its `allowed-tools` lists neither review path the gate would land on "stop and prompt +the user to install `bitwarden-code-review`" — a prompt nobody is there to answer. + +## `stacking-pull-requests` + +Three edges arrive from it, and none hands back: + +| Its step | Enters at | Runs | +| -------- | --------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| Step 0 | The top | Everything. The stack tooling was unavailable and the user declined the install or it failed, so this is now an ordinary single pull request and no caller owns the review | +| Step 2 | Step 1b | The code-review gate for one layer, nothing else | +| Step 3 | Step 2 | Steps 2 and 3 per layer; Step 4 once for the whole stack | + +Step 5 and Step 6 never run per layer. That skill owns the whole-stack preview and the +submission sequence: running Step 5 per layer asks the user to authorize a submission the stack +has not been previewed for, and running Step 6 per layer pushes outside the submission sequence +and creates pull requests that sequence then tries to create again. + +## `force-multiplier` + +Enters for the conventions only, and at the top rather than at a numbered step, so it announces that it owns the review: it walks the +workflow once at its pilot target to lock the title format, body template, and labels, then +replicates that confirmed pattern across the fan-out. It deliberately runs no per-PR review, +since it cannot answer a prompt once per target, so the Step 1b gate does not run for it. + +## Whether the review gate runs + +| Entry | Step 1b gate | +| ---------------------------------------- | ----------------------------------------------------------------------- | +| A user, directly | Runs | +| Signal 3 — stack path tried and declined | Runs — that is what signal 3 says | +| `stacking-pull-requests` Step 2 | Runs — that is what it is asking for | +| `stacking-pull-requests` Step 3 | Skipped — Step 2 already ran it for that layer | +| `stacking-pull-requests` Step 0 fallback | Runs — that path bailed before its Step 2, so no caller owns the review | +| `force-multiplier` | Skipped — it owns the decision and runs no per-PR review | + +The rule underneath: skip the gate only when a caller has already handled the review and its +decision stands. Never skip it because a caller merely exists. diff --git a/plugins/bitwarden-delivery-tools/skills/force-multiplier/SKILL.md b/plugins/bitwarden-delivery-tools/skills/force-multiplier/SKILL.md index a2380d86..710a30cf 100644 --- a/plugins/bitwarden-delivery-tools/skills/force-multiplier/SKILL.md +++ b/plugins/bitwarden-delivery-tools/skills/force-multiplier/SKILL.md @@ -57,7 +57,7 @@ Force Multiplier is the **cross-target** layer. Per-target intelligence lives in - `Skill(labeling-changes)` — the conventional type keyword that drives the `t:` label. - `Skill(creating-pull-request)` — the draft-PR workflow, template, and `ai-review` label. -Of these, `creating-pull-request` is **interactive** — it prompts per PR, which you cannot answer dozens of times. Resolve it at **PILOT**: walk it once to lock the title format, body template, and labels, then replicate that confirmed pattern non-interactively across the fan-out as draft PRs. +Of these, `creating-pull-request` is **interactive** — it prompts per PR, which you cannot answer dozens of times. Resolve it at **PILOT**: walk it once to lock the title format, body template, and labels, then replicate that confirmed pattern non-interactively across the fan-out as draft PRs. Say so when invoking it — state that this is `force-multiplier` collecting conventions and that `force-multiplier` owns the code-review decision — or its Step 1b gate runs mid-campaign and stops to prompt for a review this workflow cannot answer. ## Safety defaults (non-negotiable unless explicitly overridden) diff --git a/plugins/bitwarden-delivery-tools/skills/perform-preflight/SKILL.md b/plugins/bitwarden-delivery-tools/skills/perform-preflight/SKILL.md index 5436c6af..ef57a2da 100644 --- a/plugins/bitwarden-delivery-tools/skills/perform-preflight/SKILL.md +++ b/plugins/bitwarden-delivery-tools/skills/perform-preflight/SKILL.md @@ -1,6 +1,6 @@ --- name: perform-preflight -description: Quality gate checklist to run before committing or creating a PR. Use when finishing implementation, checking work quality, or preparing to commit. Triggered by "preflight", "self review", "ready to commit", "check my work", "quality gate". +description: Quality gate checklist to run before committing or creating a PR, with a section covering the current branch when it is one layer of a stack. Use when finishing implementation, checking work quality, or preparing to commit. Triggered by "preflight", "self review", "ready to commit", "check my work", "quality gate". Gating a whole stack layer by layer belongs to stacking-pull-requests, which calls this per layer. --- # Preflight Checklist @@ -31,6 +31,30 @@ Run this checklist before committing or creating a PR. Consult the repo's CLAUDE - [ ] Dependency injection and error handling follow repo convention - [ ] String resources added to the correct location (if applicable) +## Stacked Branches + +Only applies when the current branch is one layer of a stack. Test that rather than assume it, since preflight is often invoked directly rather than from the stack workflow. Run `gh stack view --json`, then: + +- Do this section only if `gh stack view --json` exits `0` **and** its payload names the current branch. Any non-zero status skips it, and do not try to interpret which one you got — `Skill(stacking-pull-requests)` Step 0 owns what each status means and the install offer that follows. +- **Say when you skipped, and why.** A silent skip is indistinguishable from three satisfied checkboxes, and `stacking-pull-requests` Step 2 requires this section per layer. Report one of: + - `Stacked Branches: skipped, gh stack view --json exited ` + - `Stacked Branches: skipped, gh stack view --json exited 0 but its payload does not name this branch` +- **Leave the checkboxes unchecked on a skip.** A bare `exited 0` with ticks reads as a clean run to a human and to Step 2 alike. +- **A caller may assert this branch is a layer**, in which case run the section on that assertion instead of the probe. It must supply the parent branch with the assertion, since the rebase checkbox takes its parent from the payload that is absent on this path; without one, report that checkbox unverifiable rather than guessing. +- Do not pipe `gh stack view --json` when you need that status. The shell reports the last command's exit code, not `gh`'s, so a pipe silently turns the gate on the wrong process. + +A stack merges bottom-to-top and all-or-nothing, so a layer that is red on its own blocks every layer above it. + +- [ ] This layer builds, lints, and passes its tests with only the layers below it present +- [ ] This layer is rebased on its parent — `git merge-base --is-ancestor "" HEAD` exits 0, where `` is the layer immediately below this one in the `gh stack view --json` payload captured above. That name is repository data, so validate it against `^[A-Za-z0-9_][A-Za-z0-9._/-]*$` before composing the command and quote it as shown; a branch name is legal git syntax and can still be a shell payload. Report the checkbox unverifiable if it fails the pattern. On the bottom layer, whose parent is trunk, report this unverified rather than substituting trunk: the trunk-drift paragraph below deliberately keeps that off the checklist, and checking it here would reintroduce it +- [ ] This layer references no code that lands in a layer above it + +The last item has no command behind it; it is a read-through of what this layer calls. Report it as checked-by-inspection rather than verified. + +How far trunk has moved is deliberately not a checkbox. Trunk advances constantly, so gating a commit on it would fire most of the time and force a full-stack rebase and force-push, invalidating reviews on lower layers. + +See `Skill(stacking-pull-requests)` for the surrounding workflow, for walking the rest of the stack, and for whether the `gh-stack` tooling is available at all. + ## On Failure -If any check fails, fix the issue before proceeding. For test failures, diagnose the root cause rather than skipping. For lint/format failures, run the repo's auto-fix command if available. If a check cannot be resolved, flag it to the user with the specific failure output. +If any check fails, fix the issue before proceeding — with one conditional exception. A failed **rebase checkbox** depends on whether the layer already has an open pull request. Before submission, when it does not, rebase it on its parent as an ordinary fix; there is nothing to force-push and nothing above it to invalidate. Once it does, do not fix it here: the rebase rewrites and force-pushes every layer above, which belongs to `Skill(stacking-pull-requests)` Step 5 behind the confirmation it requires listing every affected PR. Report it and stop in that case. For test failures, diagnose the root cause rather than skipping. For lint/format failures, run the repo's auto-fix command if available. If a check cannot be resolved, flag it to the user with the specific failure output. diff --git a/plugins/bitwarden-delivery-tools/skills/stacking-pull-requests/SKILL.md b/plugins/bitwarden-delivery-tools/skills/stacking-pull-requests/SKILL.md new file mode 100644 index 00000000..e220daca --- /dev/null +++ b/plugins/bitwarden-delivery-tools/skills/stacking-pull-requests/SKILL.md @@ -0,0 +1,138 @@ +--- +name: stacking-pull-requests +description: 'Break one change into a stack of dependent pull requests in a Bitwarden repository — plan the layers, carry the ticket key, the conventional-commit type prefix that drives the t: label, the PR template, and the ai-review label onto every layer, gate each layer, submit the chain, address review feedback on a lower layer, and merge the stack. Triggered by "stack these PRs", "stacked diffs", "split this into dependent PRs", "comment on the bottom PR of my chain", "land the whole chain". Not for a single-branch pull request (that is creating-pull-request) or one change fanned across many repositories (that is force-multiplier).' +--- + +# Stacking Pull Requests + +A stack turns one large change into a chain of branches, each rooted on the one below it, where every layer is its own pull request reviewed against its parent rather than against trunk. Reviewers see one concern at a time. + +The mechanics belong to GitHub: `Skill(gh-stack)` documents the full `gh stack` command surface, its non-interactive flags, and the traps that hang an agent. This skill does not restate that surface. Aside from the availability and state checks in Step 0, the stack registration in Step 1, and the merge in Step 6 — whose flags come from `Skill(gh-stack)`, not from here — the only `gh stack` commands written down here are the submission sequence in `${CLAUDE_PLUGIN_ROOT}/skills/stacking-pull-requests/references/submitting-a-stack.md`, which stays usable on its own; navigation, rebasing, and conflict handling need `Skill(gh-stack)`. What this skill owns is the part GitHub's tooling knows nothing about: Bitwarden's per-PR conventions, which a stack multiplies by the number of layers, and which `gh stack submit` cannot carry at all. + +## Step 0 — Confirm stacks are available + +Stacks depend on an external, pre-1.0 extension that is not installed by default, plus the skill that ships with it. Check both before planning anything, because a plan built on unavailable tooling wastes the planning. + +- **Extension.** `gh extension list` must show a `github/gh-stack` row. If it is absent, offer `gh extension install github/gh-stack --pin v0.1.0`. Pin it: the extension is pre-1.0, the flag sets in the references were verified against v0.1.0, and this is the component that runs with live `git` and `gh` write credentials. +- **Skill.** A `gh-stack` skill must be resolvable — from a plugin, a `--plugin-dir` load, or a `SKILL.md` at `.claude/skills/gh-stack/` or `~/.claude/skills/gh-stack/`. Check the available-skills listing for the name, and only fall back to `Glob` over those two paths if it is absent there. Do not probe by invoking `Skill(gh-stack)` to see what happens: invoking a missing skill is itself an error, inside a gate whose whole job is to fail cleanly into the fallback. `gh extension install` fetches only a platform binary and never places the skill, so it has to be installed separately; `${CLAUDE_PLUGIN_ROOT}/skills/stacking-pull-requests/references/installing-gh-stack.md` has that procedure, including the commit the copy must be verified against. Extension present and skill absent is therefore the likeliest combination, so offer the procedure and take the single-branch fallback only if the user declines or the install fails. + +If both are present, continue. Otherwise name which one is missing and its remedy, and ask whether to install it. If the user declines or the install fails, the change ships as a single pull request: hand off to `Skill(creating-pull-request)` and state in the handoff that the stack path was already tried and declined, so it proceeds as one PR instead of routing back here. Say plainly that the change is shipping as one PR. Never half-attempt a stack. + +**Existing stack state.** Once both are present, invoke `Skill(gh-stack)` and have it report the current stack with `gh stack view --json`. Read the exit code: `0` means the current branch is in a stack, and anything else means treat it as not in one. Do not run it through a pipe when you need that status — the shell reports the last command's exit code, not `gh`'s. Exit `2` covers "not in a stack" but also "not a git repository" and a detached HEAD, so require that a successful payload names the current branch rather than trusting the code alone. Exit `9` is different and worth catching here: it means the repository does not have stacked PRs enabled, which otherwise surfaces only at submit, after every layer has been planned, gated, and titled. Treat it as unavailable tooling and take the single-branch fallback. + +## Step 1 — Plan the layers before writing code + +Layer boundaries are a design decision, and retrofitting them onto a finished branch is the expensive way to discover that. Decide the chain first. + +Two properties make a layer a real layer: + +- **Independently reviewable.** It is one coherent concern a reviewer can judge without the layers above it. +- **Independently green.** It builds, lints, and passes its tests with only the layers below it present. A layer that is red on its own is not reviewable and cannot merge, since `gh stack merge` lands the chain bottom-to-top. + +Foundational work goes at the bottom, consumers above it. If code in one layer depends on code in another, the dependency belongs in the same layer or a lower one — never a higher one. + +**Don't stack when:** the change fits comfortably in one PR; the layers cannot be made independently green; or the work is the same change repeated across many repositories, which is a fan-out rather than a chain and belongs to `Skill(force-multiplier)`. + +Confirm the planned layers with the user before creating branches. Name each layer's concern and the Jira ticket the stack serves. One ticket normally spans the whole stack. + +**Then build the layers.** Step 2 assumes every planned layer already exists as a branch with its commits on it, and three entry shapes reach it: + +- **Nothing written yet.** Create each branch on the one below it and write the layers bottom-up. +- **Already in a stack**, which is what exit `0` in Step 0 reports. The layers exist; add to them rather than rebuilding, and see the submission reference for appending a layer to a stack already on GitHub. +- **One oversized branch already written**, which is how most requests here arrive ("split this into dependent PRs", "I want stacked diffs instead of one 2000-line review"). Decide which existing commits belong to which layer before moving anything, then redistribute them. Whether a commit is foundation or consumer is the planning judgment this skill owns; the branch and rebase mechanics are `Skill(gh-stack)`'s. + +**Register the layers as a stack.** On the first and third shapes the branches are not yet a stack as far as `gh stack` is concerned, and `gh stack push` in the submission sequence exits `2` without one — after every layer has been gated, titled and previewed, which is the late failure Step 0 exists to prevent. Validate every branch name against the allowlist in `${CLAUDE_PLUGIN_ROOT}/skills/stacking-pull-requests/references/submitting-a-stack.md` before composing the command — two of the three shapes take names from the repository rather than names you chose. Then `gh stack init --base "" "" "" …` covers both: it adopts branches that already exist and creates the ones that do not. Confirm with `gh stack view --json` before moving on. + +Each layer's first commit takes the full format from `Skill(committing-changes)`. + +## Step 2 — Gate every layer, not just the top + +Both delivery gates are per-PR, so in a stack they run per layer. + +- `Skill(perform-preflight)` on each layer, including its Stacked Branches section. That skill checks the layer it is on; this step owns walking the stack, so check out each layer in turn and run it there. +- The code-review gate from `Skill(creating-pull-request)` Step 1b, per layer. Layer scoping is the wrinkle: both review paths default to `origin/HEAD`, so on layer 3 they re-review layers 1 and 2. That step's own base-ref paragraph is authoritative on which path can be scoped to a single layer and what to record when neither can — follow it rather than a second copy here. + +**Record what each gate produced.** Step 4's preview prints a review path and a deferred-finding count per layer, and nothing else carries them: `creating-pull-request` writes those into its own Step 5 preview, which never runs per layer. So as each layer's gate finishes, note the path taken (`Standard`, `Substantial`, or a user-volunteered skip), every deferred CRITICAL or IMPORTANT finding, and whatever `perform-preflight` reported for its Stacked Branches section — a skip there has to reach the preview, or a layer whose stack checks never ran renders as `preflight: pass`. Carry the review path and the deferred findings into that layer's PR body, and all three into its preview row — the preflight report has a slot only in the preview. + +**Tell the reviewer where the layer sits.** A lower layer legitimately adds code with no caller yet, because the caller lands above it. State the layer's position and what lands above it in the review request, otherwise a reviewer reasonably reads incomplete-by-design work as dead code. + +## Step 3 — Give every layer its Bitwarden conventions + +Each layer is a pull request, so each layer needs the full set from `Skill(creating-pull-request)`. Which of its steps run, and how often: + +| Its step | Runs | Why | +| --------------------------- | --------------- | ------------------------------------------------------------------------------------------------------ | +| 1 — preflight + review gate | Never from here | Step 2 above already ran both per layer; repeating costs another prompt and review | +| 2 — title | Once per layer | The type keyword is chosen per layer | +| 3 — template body | Once per layer | Each layer states its own position in the stack | +| 4 — `ai-review` label | Once per stack | A whole-stack decision, applied to every layer | +| 5 — per-PR preview | Never | Step 4 below previews the whole stack instead | +| 6 — push and `gh pr create` | Never | `${CLAUDE_PLUGIN_ROOT}/skills/stacking-pull-requests/references/submitting-a-stack.md` owns submission | + +Steps 5 and 6 are the two that break things if they leak: Step 5 asks the user to authorize a submission the stack has not been previewed for, and Step 6 pushes outside the submission sequence and creates PRs that sequence then tries to create again. + +What each layer needs: + +- a `[PM-XXXXX] : ` title, with the ticket key repeated on every layer and the type keyword chosen per layer (a stack often mixes `feat` and `refactor`), +- a body following the repo's `.github/PULL_REQUEST_TEMPLATE.md`, +- the stack's single `ai-review` label decision, applied to every layer. + +The label question is the one piece that is asked once for the stack rather than once per layer. + +**`gh stack submit` cannot carry any of this.** Its only flags are `--auto`, `--open`, and `--remote`; there is no title, body, or label flag, and `--auto` generates titles that carry no type prefix. A stack submitted that way ships N pull requests with no `t:` label and no template body. `${CLAUDE_PLUGIN_ROOT}/skills/stacking-pull-requests/references/submitting-a-stack.md` has the two paths that do work and the commands for each. + +## Step 4 — Preview the whole stack, then submit + +`creating-pull-request` shows one submission preview per PR. For a stack, show one preview covering every layer, because a wrong base is the failure this catch-net exists for and it is only visible when the chain is laid out together: + +``` +═══════════════════════════════════════ + STACK SUBMISSION PREVIEW +═══════════════════════════════════════ +Target repo: +Trunk: +Draft: (Step 6 runs `gh pr ready` on every draft before merging) +AI review: + + 1. base: → t: