Skip to content

Commit cdce148

Browse files
miltonhultgrenmiltonhultgren
committed
Add skill to split larger Kibana change to multiple PRs
1 parent 3a38cbb commit cdce148

5 files changed

Lines changed: 379 additions & 0 deletions

File tree

‎.agents/skills/kibana-split-to-prs/SKILL.md‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
---
2+
name: kibana-split-to-prs
3+
description: >-
4+
Analyzes local Kibana git changes and proposes (or executes after approval) a
5+
stacked chain of small, focused PRs. Each PR is a review product: one question,
6+
one owner, planned validation, no hidden risk. CODEOWNERS-aligned, risk-tagged
7+
hints, low-risk vs needs-scrutiny buckets, splittability edits. Diffs against
8+
upstream/main (fork workflow). Use when splitting a branch, dirty working tree,
9+
or large diff for faster time-to-merge.
10+
disable-model-invocation: true
11+
---
12+
13+
# Kibana split to PRs
14+
15+
**North star:** time-to-merged. Each PR = **one review question**, reviewable in one focused pass ([references/review.md](references/review.md)).
16+
17+
Turn local work into a **single stacked chain** (`upstream/main` → PR1 → PR2 → …). Deliverable is a **split plan**; git execution optional after approval. Design already reviewed.
18+
19+
**Not git-only:** when hunks or compile order block clean slices, **edit the code** (shims, dark-ship, extractions)—see [references/slicing.md](references/slicing.md) §splittability. Plan in the split plan; apply only after **execute split**.
20+
21+
Also read [AGENTS.md](../../../AGENTS.md) for module boundaries, plugin `server/index.ts`, testing, and mergeability guardrails.
22+
23+
## Hard rules
24+
25+
- **No git mutations** until the user approves the split plan or **execute split**.
26+
- **No draft PRs** without explicit ask ([references/execution.md](references/execution.md)).
27+
- **Never discard work.** No destructive git without explicit approval.
28+
- **Recoverable snapshot** before Phase 5 (see table).
29+
- **Stage only named files or hunks.** No `git add .` / `git add -A`.
30+
31+
## Workflow
32+
33+
```mermaid
34+
flowchart TD
35+
analyze[1_Analyze]
36+
slice[2_Slice_and_audit]
37+
plan[3_Write_plan]
38+
gate[4_Review_gate]
39+
execute[5_Execute]
40+
report[6_Report]
41+
42+
analyze --> slice --> plan --> gate
43+
gate -->|execute_split| execute --> report
44+
gate -->|revisions| slice
45+
gate -->|plan_only| report
46+
```
47+
48+
Phases 1–3 are **git read-only** (may propose splittability edits; apply in Phase 5). Plans record validation as `not run — slice not materialized`; after **execute split**, each slice must pass `node scripts/check` before the next ([references/execution.md](references/execution.md)).
49+
50+
| Phase | Read | Do |
51+
|-------|------|-----|
52+
| **1 Analyze** | [slicing.md](references/slicing.md) §when-not, §hotspots | **`upstream`** = diff baseline, **`origin`** = push only—not `origin/main` unless user asks ([fork docs](../../../docs/extend/development-github.md)). `git remote -v`; if no `upstream`, stop. `git fetch upstream` && `git rev-parse --verify upstream/main`. Diff: `git diff upstream/main...HEAD`, `git diff`, `git diff --cached`, `git log upstream/main..HEAD --oneline`, `git status`. Chat history for intent. §when-not: if one PR suffices, say so. Per changed path: walk to `kibana.jsonc` (`@kbn/...`, owner, group, visibility). Record hotspots. |
53+
| **2 Slice + audit** | [slicing.md](references/slicing.md), [review.md](references/review.md) | Slice per slicing order (CODEOWNERS → deps → mechanical/behavioral → BBA/dark-ship → noise). One chain only—no parallel PRs off `main`. Splittability edits when `git add -p` won't compile or yield one question. **Audit:** each PR passes [review.md](references/review.md) and has an answer for every Phase 4 row below. Revise or `NEEDS DECISION`. Flag mergeability per [AGENTS.md](../../../AGENTS.md) (SO: one model version/type/PR; privileges: deprecation skill ordering). |
54+
| **3 Plan** | [plan-template.md](references/plan-template.md) | Write `split-plan-<slug>.md` (repo root or primary plugin dir—state path). Follow schema; **self-check:** every PR has all 7 fields. |
55+
| **4 Gate** | — | **Stop.** No branches, commits, push, PRs. Present plan (see below). Ask: **Plan only** \| **Approve and execute split**. Do not bundle draft PRs. |
56+
| **5 Execute** | [execution.md](references/execution.md) | Backup: `SHA=$(git stash create "pre-split"); [ -n "$SHA" ] && git update-ref "refs/backup/pre-split-$(date +%s)" "$SHA"`. `git fetch upstream`. Per PR in order: splittability edits → branch from `upstream/main` or prior split branch → stage planned hunks only → commit → **`node scripts/check --scope branch --base-ref upstream/main`**—stop on fail → record branch + SHA + outcome. No push/`gh` unless asked (`origin` only). Leave original branch recoverable. |
57+
| **6 Report** | [execution.md](references/execution.md) | Plan path; branch names + SHAs; check outcomes; leftover hunks; suggest only: draft PRs on `origin`, or fix failed check before continuing. |
58+
59+
## Phase 4 — Review gate (mandatory)
60+
61+
Surface in chat — **lead with review questions**, not file lists:
62+
63+
1. Feature context + stack diagram + PR count (or single-PR recommendation)
64+
2. **PR # → review question → bucket → attention**
65+
3. Merge order (foundations vs behavioral—not approval urgency)
66+
4. Hotspots + cross-PR hunk maps
67+
5. Highest-attention PRs: one hint each
68+
6. Splittability edits not in current diff
69+
7. `NEEDS DECISION` items

‎.agents/skills/kibana-split-to-prs/references/execution.md‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
# Execution and validation
2+
3+
## Validation timing
4+
5+
| Phase | Run check on slice? | Plan outcome |
6+
|-------|---------------------|--------------|
7+
| 1–3 | **No** | `not run — slice not materialized; run in Phase 5 after commit` |
8+
| 5 | **Yes** — after commit | `pass` or `fail` (stop stack on fail) |
9+
10+
Do not claim checks passed during planning.
11+
12+
## Check gate (Phase 5)
13+
14+
After each slice commit:
15+
16+
```bash
17+
node scripts/check --scope branch --base-ref upstream/main
18+
```
19+
20+
Pass → proceed. Fail → stop; fix slice; re-run. Baseline: `upstream/main`.
21+
22+
Supplement commands: [AGENTS.md](../../../AGENTS.md) Testing section (`check_changes`, `jest`, `type_check`, `eslint`). Prefer `scripts/check` for the gate—do not substitute "CI will catch it" without naming deferred checks.
23+
24+
| PR type | Validation |
25+
|---------|------------|
26+
| Mechanical, unchanged | `scripts/check` |
27+
| Behavioral | `scripts/check` + tests in PR |
28+
| Dark-shipped (off) | `scripts/check` + tests for new path |
29+
| Flip | `scripts/check`; often small diff |
30+
31+
---
32+
33+
## Approval gates
34+
35+
| Action | Requires |
36+
|--------|----------|
37+
| Split plan | — |
38+
| Branches + commits | **execute split** |
39+
| `git push` to `origin` | Explicit ask (never `upstream`) |
40+
| Draft PRs | Explicit ask **after** slices exist |
41+
42+
Do not open draft PRs as a side effect of execute split.
43+
44+
## Stack bases
45+
46+
PR1 base `upstream/main`; PRn base branch for PR(n−1). After PR1 merges, retarget PR2 to `upstream/main` or rebase—note in plan if merging incrementally.
47+
48+
## Opening draft PRs
49+
50+
Push to **`origin`**, then:
51+
52+
```bash
53+
gh pr create --draft --base <base> --head <head> --title "..." --body-file -
54+
# PR2+: --base previous split branch until PR1 merges
55+
```
56+
57+
Use review question + stack position in body. Draft until check passed on branch.

‎.agents/skills/kibana-split-to-prs/references/plan-template.md‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
# Split plan schema
2+
3+
Write `split-plan-<slug>.md` following this schema. Definitions: [review.md](review.md). Execution: [execution.md](execution.md).
4+
5+
**Phase 3 self-check:** every PR block has all 7 fields below.
6+
7+
## Stack-level sections
8+
9+
1. **Header table** — baseline (`upstream/main`), branch, date, plan path, single chain note
10+
2. **Feature context** — 1–2 sentences; design reviewed
11+
3. **Hotspots** — omit if none; file, issue, resolution
12+
4. **Ownership map** — team, files summary, PR #, primary reviewer
13+
5. **Summary table** — PR #, title, review question, bucket, risk tags, attention, file count, merge priority, depends on
14+
6. **Stack mermaid** + suggested merge order (foundations vs behavioral)
15+
7. **Per-PR blocks** — repeat for each PR
16+
8. **Leftover / deferred** — omit if empty
17+
9. **NEEDS DECISION** — omit if empty
18+
10. **Splittability edits summary** — stack-level code not in current diff; omit if none
19+
20+
Bucket, attention, merge priority, depends-on live in **Summary** only—not duplicated per PR.
21+
22+
## Per-PR block (7 fields)
23+
24+
Each PR: `## PR n — <title>` then:
25+
26+
| # | Section | Content |
27+
|---|---------|---------|
28+
| 1 | **Review question** | One sentence |
29+
| 2 | **Out of scope** | What later PRs add |
30+
| 3 | **Surprise audit** | `None identified` or bullets + hints |
31+
| 4 | **Risk tags + hints** | Tags; 1–2 imperative hints |
32+
| 5 | **Files** | Paths; cross-PR hunk map inline if needed |
33+
| 6 | **Mergeability + splittability edits** | Why safe alone; flag/dark-ship state; `none` or edits in this PR; PR # removing scaffolding |
34+
| 7 | **Validation** | Command + `Outcome: not run — slice not materialized; run in Phase 5 after commit` |
35+
36+
Per-PR metadata (base, team, modules, change class) — one line under the title if needed:
37+
38+
`Base: upstream/main · Team: @elastic/... · Modules: @kbn/... · Class: mechanical|behavioral|mixed`
39+
40+
After Phase 5, append execution log (branch, SHA, outcome pass|fail) under Validation.
41+
42+
---
43+
44+
## Example PR block (cross-PR hunk map)
45+
46+
`## PR 3 — Dark-ship new backoff behind useNewRetry`
47+
48+
Base: PR2 branch · Team: @elastic/kibana-management · Modules: @kbn/task-manager · Class: behavioral
49+
50+
### Review question
51+
52+
Is the new backoff correct when enabled, and fully inactive when `useNewRetry` is false?
53+
54+
### Out of scope
55+
56+
Enabling flag (PR4); type extraction (PR1).
57+
58+
### Surprise audit
59+
60+
`task_runner.ts` also in PR2 (wiring) and PR4 (flip)—hunk map in Files.
61+
62+
### Risk tags + hints
63+
64+
`concurrency`, `tests`
65+
66+
- Trace `runTask()`—`useNewRetry` branch unreachable at default.
67+
- Unit tests cover both policies without timing flakes.
68+
69+
### Files
70+
71+
- `packages/.../retry_policy.ts` — new backoff impl
72+
- `packages/.../task_runner.ts` — **PR3 only:** `useNewRetry` branch (default false)
73+
- Hunk map: `task_runner.ts` — PR2: wiring; PR3: `useNewRetry`; PR4: remove legacy
74+
75+
### Mergeability + splittability edits
76+
77+
Dark-ship off at default; production path unchanged. Add `const useNewRetry = false` and `NewRetryPolicy`. Scaffolding removed in PR4.
78+
79+
### Validation
80+
81+
- Command: `node scripts/check --scope branch --base-ref upstream/main`
82+
- Outcome: not run — slice not materialized; run in Phase 5 after commit

‎.agents/skills/kibana-split-to-prs/references/review.md‎

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
# Review product
2+
3+
A PR is a **review product**, not a git hunk. Canonical vocabulary:
4+
5+
| Term | Meaning |
6+
|------|---------|
7+
| **One question** | Single sentence the reviewer decides—not a file list; split if "and" joins unrelated concerns |
8+
| **One owner** | Primary `@elastic/team`; secondary minimal and named |
9+
| **Planned validation** | `node scripts/check --scope branch --base-ref upstream/main`; outcome only after Phase 5 commit |
10+
| **No hidden risk** | Surprise audit empty or every surprise named + hinted |
11+
| **Bucket** | `low risk` \| `needs scrutiny`—review surface, not approval urgency |
12+
13+
Apply to each PR after slicing, before Phase 3. Fail → reslice or `NEEDS DECISION`.
14+
15+
**Pass when:** scannable (title + question + tags tell the story); standalone readable (no later PR required); single-pass review (assessable without rest of stack).
16+
17+
## Buckets
18+
19+
| Bucket | Signals |
20+
|--------|---------|
21+
| **Low risk** | Attention `low`, ≤2 tags, mechanical or dark-ship off |
22+
| **Needs scrutiny** | Attention `medium`/`high`, `privileges`, `saved-objects`, `http-api`, `concurrency`, flip PR |
23+
24+
Do not hide needs scrutiny inside low risk.
25+
26+
## Hidden risk
27+
28+
*What would make a reviewer say "I didn't expect that"?*
29+
30+
| Risk | Fix |
31+
|------|-----|
32+
| Trojan mechanical | Split behavioral hunk |
33+
| Silent default | Own PR or `config` tag + hint |
34+
| Type-only runtime change | `public-api` + call-site hint |
35+
| Cross-stack file | Reslice, extract, hunk map ([slicing.md](slicing.md) §hotspots) |
36+
| Stack-only readable | **Out of scope** + standalone narrative |
37+
| Deferred validation | Name check or run on branch |
38+
| Owner mismatch | Split or document coupling |
39+
| Drive-by fixes / unrelated tags in one PR | Remove or split |
40+
| Cross-PR hunks without map | Reslice or map |
41+
42+
Record in **Surprise audit** (`None identified` if empty).
43+
44+
## Stack self-check
45+
46+
Titles + questions predict the arc? ≤5 PRs or documented why? Same file 3+ re-reads → extract or exception? No silent leftover hunks?
47+
48+
## Risk tags
49+
50+
Tag every applicable area. Prefer 1–3; more → split further.
51+
52+
`public-api`, `http-api`, `saved-objects` (one model version per type per PR), `privileges`, `auth`, `concurrency`, `state-machine`, `config`, `ui`, `a11y`, `i18n`, `plugin-entry`, `dependencies`, `tests`, `generated`, `noise`
53+
54+
## Hints
55+
56+
Imperative, short; files/symbols; tie to review question. No ignore lists or diff prose.
57+
58+
| Signal | Attention |
59+
|--------|-----------|
60+
| ~80 lines privilege mapping | high |
61+
| ~800 lines moves | low (`noise`) |
62+
| HTTP route + authz | high |
63+
| `useNew = false` + new impl | medium |
64+
| Flip PR | low–medium |
65+
66+
## PR blurb (generate when opening PRs, not required in plan)
67+
68+
```
69+
<One-line summary>
70+
Review question: <question>
71+
Stack: n/N
72+
```
73+
74+
Optional: `Focus: <hint>`. If check passed: may note `node scripts/check` passed—verification only, not skip review.

‎.agents/skills/kibana-split-to-prs/references/slicing.md‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
# Slicing
2+
3+
North star is **time-to-merged**, not maximum PR count. Splitting costs stack overhead, splittability scaffolding, context-switching, GitHub mechanics.
4+
5+
## When not to split
6+
7+
**One PR when:** one [review question](review.md), one owner, low surprise; small scope (one team, few files, no SO/privilege/http-api risk); heavy splittability edits for little gain; time-sensitive work where stack delays first reviewable unit.
8+
9+
**Stack when:** separable CODEOWNERS teams; risk isolation (dark-ship, SO model version, privilege deprecation, HTTP+authz); mechanical noise would poison behavioral review; one file mixes mechanical + behavioral and extraction wins.
10+
11+
### Decision gate
12+
13+
1. Would one coherent PR stall review more than a stack stalls shipping the first slice?
14+
2. Does each extra PR remove a **distinct review question** or just chop history?
15+
3. Will splittability edits add more total review than they save?
16+
17+
If (2) and (3) favor splitting → stack. Else → **single PR**.
18+
19+
**Depth:** target ≤5 PRs; merge coherent slices or document why not.
20+
21+
**Stop splitting** when compile breaks without reasonable shims, hunks won't separate, or re-read tax exceeds benefit—propose shims (§splittability) before one big PR.
22+
23+
---
24+
25+
## Hotspots
26+
27+
Find in Phase 1 before boundaries.
28+
29+
| Signal | Action |
30+
|--------|--------|
31+
| Mechanical + behavioral in one file | Extract or dark-ship |
32+
| Same file in 2+ PRs | Avoid—see cross-PR |
33+
| Two review questions in one file | Extract or `const useNew = false` |
34+
| Mixed team ownership | Split by file, or one PR + coupling note |
35+
| God file | One-time extraction PR |
36+
37+
**Cross-PR default:** each file in at most one PR. Prefer extract, dedicated move/rename PR, or splittability edit.
38+
39+
**Acceptable** (Surprise audit + hunk map): flag/dark-ship sequence; poor prior factoring (`NEEDS DECISION`); generated + hand edit (`generated` tag).
40+
41+
```markdown
42+
- `path/task_runner.ts` — PR2: wiring; PR3: `useNewRetry` branch (default false)
43+
```
44+
45+
Anti-patterns (trojan mechanical, hidden defaults, stack-only readable, etc.): [review.md](review.md) §hidden-risk.
46+
47+
---
48+
49+
## Slice order
50+
51+
0. CODEOWNERS / team boundaries
52+
1. Module / dependency order (package API before consumer; shims if needed)
53+
2. Mechanical vs behavioral (not line-count driven)
54+
3. Unrelated concepts (feature/API/privilege, not directory)
55+
4. BBA + dark-ship: types → wire legacy → dark-ship new (`const useNew = false`) → flip → remove legacy
56+
5. Noise early: lockfile, generated mass updates, i18n JSON, Scout scaffold, bulk `kibana.jsonc`
57+
6. See [review.md](review.md) §hidden-risk for anti-patterns
58+
59+
**Exception:** prettier/eslint from the behavioral edit in the same files stays in that PR.
60+
61+
**Titles:** one [review question](review.md)—not "Part 2" or file lists. Split if the title needs "and" for unrelated concerns.
62+
63+
### CODEOWNERS
64+
65+
1. [`.github/CODEOWNERS`](../../../.github/CODEOWNERS) · nested wins · `kibana.jsonc` `owner` · `node src/dev/stage_by_owner.ts`
66+
67+
Rules: one PR per team when separable; same team + related feature → one slice; no owner → `NEEDS DECISION`.
68+
69+
### Stack
70+
71+
`upstream/main` → PR1 → PR2 → … only. Build: types/shims/APIs first. Merge: foundations before behavioral flips.
72+
73+
### Mergeability
74+
75+
Flag per [AGENTS.md](../../../AGENTS.md): plugin `server/index.ts` lazy import; Saved Objects one model version per type per PR ([validate](../../../docs/extend/saved-objects/validate.md)); privilege deprecations ([skill](../../kibana-privilege-deprecation/SKILL.md)); tests with behavior.
76+
77+
---
78+
79+
## Splittability edits
80+
81+
Not pure git surgery—**edit code** when hunks, compile order, or review boundaries block a clean stack. Scaffolding for review only; design approved.
82+
83+
| Signal | Edit |
84+
|--------|------|
85+
| Consumer needs package exports | Temporary re-export/shim |
86+
| Old + new impl in one file | BBA + dark-ship |
87+
| Two questions in one file | Extract or `const useNew = false` |
88+
| SO / privilege separate landing | Split schema from behavior |
89+
| Mechanical + behavioral same hunk | Separate files or gated branches |
90+
| PR N needs PR N+1 symbols | Stub, deprecated wrapper, `import type` |
91+
92+
**In scope:** dark-ship toggles, temp re-exports/no-ops, extracted files, lazy plugin entry, test doubles, hunk moves.
93+
**Out of scope:** unrelated fixes, permanent API you wouldn't ship as one PR, early behavior enablement.
94+
95+
**Workflow:** list in plan (per-PR under Mergeability + splittability); call out at Phase 4 gate; apply in Phase 5 before staging. Plan only → no code edits.
96+
97+
Record: `none` or edits **in this PR**; note PR # that removes scaffolding.

0 commit comments

Comments
 (0)