Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
69 changes: 69 additions & 0 deletions .agents/skills/kibana-split-to-prs/SKILL.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
---
name: kibana-split-to-prs
description: >-
Analyzes local Kibana git changes and proposes (or executes after approval) a
stacked chain of small, focused PRs. Each PR is a review product: one question,
one owner, planned validation, no hidden risk. CODEOWNERS-aligned, risk-tagged
hints, low-risk vs needs-scrutiny buckets, splittability edits. Diffs against
upstream/main (fork workflow). Use when splitting a branch, dirty working tree,
or large diff for faster time-to-merge.
disable-model-invocation: true
---

# Kibana split to PRs

**North star:** time-to-merged. Each PR = **one review question**, reviewable in one focused pass ([references/review.md](references/review.md)).

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.

**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**.

Also read [AGENTS.md](../../../AGENTS.md) for module boundaries, plugin `server/index.ts`, testing, and mergeability guardrails.

## Hard rules

- **No git mutations** until the user approves the split plan or **execute split**.
- **No draft PRs** without explicit ask ([references/execution.md](references/execution.md)).
- **Never discard work.** No destructive git without explicit approval.
- **Recoverable snapshot** before Phase 5 (see table).
- **Stage only named files or hunks.** No `git add .` / `git add -A`.

## Workflow

```mermaid
flowchart TD
analyze[1_Analyze]
slice[2_Slice_and_audit]
plan[3_Write_plan]
gate[4_Review_gate]
execute[5_Execute]
report[6_Report]

analyze --> slice --> plan --> gate
gate -->|execute_split| execute --> report
gate -->|revisions| slice
gate -->|plan_only| report
```

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)).

| Phase | Read | Do |
|-------|------|-----|
| **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. |
| **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). |
| **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. |
| **4 Gate** | — | **Stop.** No branches, commits, push, PRs. Present plan (see below). Ask: **Plan only** \| **Approve and execute split**. Do not bundle draft PRs. |
| **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. |
| **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. |

## Phase 4 — Review gate (mandatory)

Surface in chat — **lead with review questions**, not file lists:

1. Feature context + stack diagram + PR count (or single-PR recommendation)
2. **PR # → review question → bucket → attention**
3. Merge order (foundations vs behavioral—not approval urgency)
4. Hotspots + cross-PR hunk maps
5. Highest-attention PRs: one hint each
6. Splittability edits not in current diff
7. `NEEDS DECISION` items
57 changes: 57 additions & 0 deletions .agents/skills/kibana-split-to-prs/references/execution.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
# Execution and validation

## Validation timing

| Phase | Run check on slice? | Plan outcome |
|-------|---------------------|--------------|
| 1–3 | **No** | `not run — slice not materialized; run in Phase 5 after commit` |
| 5 | **Yes** — after commit | `pass` or `fail` (stop stack on fail) |
Comment on lines +5 to +8

Do not claim checks passed during planning.

## Check gate (Phase 5)

After each slice commit:

```bash
node scripts/check --scope branch --base-ref upstream/main
```

Pass → proceed. Fail → stop; fix slice; re-run. Baseline: `upstream/main`.

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.

| PR type | Validation |
|---------|------------|
| Mechanical, unchanged | `scripts/check` |
| Behavioral | `scripts/check` + tests in PR |
| Dark-shipped (off) | `scripts/check` + tests for new path |
| Flip | `scripts/check`; often small diff |

---

## Approval gates

| Action | Requires |
|--------|----------|
| Split plan | — |
| Branches + commits | **execute split** |
| `git push` to `origin` | Explicit ask (never `upstream`) |
| Draft PRs | Explicit ask **after** slices exist |

Do not open draft PRs as a side effect of execute split.

## Stack bases

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.

## Opening draft PRs

Push to **`origin`**, then:

```bash
gh pr create --draft --base <base> --head <head> --title "..." --body-file -
# PR2+: --base previous split branch until PR1 merges
```

Use review question + stack position in body. Draft until check passed on branch.
82 changes: 82 additions & 0 deletions .agents/skills/kibana-split-to-prs/references/plan-template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
# Split plan schema

Write `split-plan-<slug>.md` following this schema. Definitions: [review.md](review.md). Execution: [execution.md](execution.md).

**Phase 3 self-check:** every PR block has all 7 fields below.

## Stack-level sections

1. **Header table** — baseline (`upstream/main`), branch, date, plan path, single chain note
2. **Feature context** — 1–2 sentences; design reviewed
3. **Hotspots** — omit if none; file, issue, resolution
4. **Ownership map** — team, files summary, PR #, primary reviewer
5. **Summary table** — PR #, title, review question, bucket, risk tags, attention, file count, merge priority, depends on
6. **Stack mermaid** + suggested merge order (foundations vs behavioral)
7. **Per-PR blocks** — repeat for each PR
8. **Leftover / deferred** — omit if empty
9. **NEEDS DECISION** — omit if empty
10. **Splittability edits summary** — stack-level code not in current diff; omit if none

Bucket, attention, merge priority, depends-on live in **Summary** only—not duplicated per PR.

## Per-PR block (7 fields)

Each PR: `## PR n — <title>` then:

| # | Section | Content |
|---|---------|---------|
| 1 | **Review question** | One sentence |
| 2 | **Out of scope** | What later PRs add |
| 3 | **Surprise audit** | `None identified` or bullets + hints |
| 4 | **Risk tags + hints** | Tags; 1–2 imperative hints |
| 5 | **Files** | Paths; cross-PR hunk map inline if needed |
| 6 | **Mergeability + splittability edits** | Why safe alone; flag/dark-ship state; `none` or edits in this PR; PR # removing scaffolding |
| 7 | **Validation** | Command + `Outcome: not run — slice not materialized; run in Phase 5 after commit` |
Comment on lines +26 to +34

Per-PR metadata (base, team, modules, change class) — one line under the title if needed:

`Base: upstream/main · Team: @elastic/... · Modules: @kbn/... · Class: mechanical|behavioral|mixed`

After Phase 5, append execution log (branch, SHA, outcome pass|fail) under Validation.

---

## Example PR block (cross-PR hunk map)

`## PR 3 — Dark-ship new backoff behind useNewRetry`

Base: PR2 branch · Team: @elastic/kibana-management · Modules: @kbn/task-manager · Class: behavioral

### Review question

Is the new backoff correct when enabled, and fully inactive when `useNewRetry` is false?

### Out of scope

Enabling flag (PR4); type extraction (PR1).

### Surprise audit

`task_runner.ts` also in PR2 (wiring) and PR4 (flip)—hunk map in Files.

### Risk tags + hints

`concurrency`, `tests`

- Trace `runTask()`—`useNewRetry` branch unreachable at default.
- Unit tests cover both policies without timing flakes.

### Files

- `packages/.../retry_policy.ts` — new backoff impl
- `packages/.../task_runner.ts` — **PR3 only:** `useNewRetry` branch (default false)
- Hunk map: `task_runner.ts` — PR2: wiring; PR3: `useNewRetry`; PR4: remove legacy

### Mergeability + splittability edits

Dark-ship off at default; production path unchanged. Add `const useNewRetry = false` and `NewRetryPolicy`. Scaffolding removed in PR4.

### Validation

- Command: `node scripts/check --scope branch --base-ref upstream/main`
- Outcome: not run — slice not materialized; run in Phase 5 after commit
74 changes: 74 additions & 0 deletions .agents/skills/kibana-split-to-prs/references/review.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# Review product

A PR is a **review product**, not a git hunk. Canonical vocabulary:

| Term | Meaning |
|------|---------|
| **One question** | Single sentence the reviewer decides—not a file list; split if "and" joins unrelated concerns |
| **One owner** | Primary `@elastic/team`; secondary minimal and named |
| **Planned validation** | `node scripts/check --scope branch --base-ref upstream/main`; outcome only after Phase 5 commit |
| **No hidden risk** | Surprise audit empty or every surprise named + hinted |
| **Bucket** | `low risk` \| `needs scrutiny`—review surface, not approval urgency |
Comment on lines +5 to +11

Apply to each PR after slicing, before Phase 3. Fail → reslice or `NEEDS DECISION`.

**Pass when:** scannable (title + question + tags tell the story); standalone readable (no later PR required); single-pass review (assessable without rest of stack).

## Buckets

| Bucket | Signals |
|--------|---------|
| **Low risk** | Attention `low`, ≤2 tags, mechanical or dark-ship off |
| **Needs scrutiny** | Attention `medium`/`high`, `privileges`, `saved-objects`, `http-api`, `concurrency`, flip PR |

Do not hide needs scrutiny inside low risk.

## Hidden risk

*What would make a reviewer say "I didn't expect that"?*

| Risk | Fix |
|------|-----|
| Trojan mechanical | Split behavioral hunk |
| Silent default | Own PR or `config` tag + hint |
| Type-only runtime change | `public-api` + call-site hint |
| Cross-stack file | Reslice, extract, hunk map ([slicing.md](slicing.md) §hotspots) |
| Stack-only readable | **Out of scope** + standalone narrative |
| Deferred validation | Name check or run on branch |
| Owner mismatch | Split or document coupling |
| Drive-by fixes / unrelated tags in one PR | Remove or split |
| Cross-PR hunks without map | Reslice or map |

Record in **Surprise audit** (`None identified` if empty).

## Stack self-check

Titles + questions predict the arc? ≤5 PRs or documented why? Same file 3+ re-reads → extract or exception? No silent leftover hunks?

## Risk tags

Tag every applicable area. Prefer 1–3; more → split further.

`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`

## Hints

Imperative, short; files/symbols; tie to review question. No ignore lists or diff prose.

| Signal | Attention |
|--------|-----------|
| ~80 lines privilege mapping | high |
| ~800 lines moves | low (`noise`) |
| HTTP route + authz | high |
| `useNew = false` + new impl | medium |
| Flip PR | low–medium |

## PR blurb (generate when opening PRs, not required in plan)

```
<One-line summary>
Review question: <question>
Stack: n/N
```

Optional: `Focus: <hint>`. If check passed: may note `node scripts/check` passed—verification only, not skip review.
97 changes: 97 additions & 0 deletions .agents/skills/kibana-split-to-prs/references/slicing.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
# Slicing

North star is **time-to-merged**, not maximum PR count. Splitting costs stack overhead, splittability scaffolding, context-switching, GitHub mechanics.

## When not to split

**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.

**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.

### Decision gate

1. Would one coherent PR stall review more than a stack stalls shipping the first slice?
2. Does each extra PR remove a **distinct review question** or just chop history?
3. Will splittability edits add more total review than they save?

If (2) and (3) favor splitting → stack. Else → **single PR**.

**Depth:** target ≤5 PRs; merge coherent slices or document why not.

**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.

---

## Hotspots

Find in Phase 1 before boundaries.

| Signal | Action |
|--------|--------|
| Mechanical + behavioral in one file | Extract or dark-ship |
| Same file in 2+ PRs | Avoid—see cross-PR |
| Two review questions in one file | Extract or `const useNew = false` |
| Mixed team ownership | Split by file, or one PR + coupling note |
| God file | One-time extraction PR |
Comment on lines +29 to +35

**Cross-PR default:** each file in at most one PR. Prefer extract, dedicated move/rename PR, or splittability edit.

**Acceptable** (Surprise audit + hunk map): flag/dark-ship sequence; poor prior factoring (`NEEDS DECISION`); generated + hand edit (`generated` tag).

```markdown
- `path/task_runner.ts` — PR2: wiring; PR3: `useNewRetry` branch (default false)
```

Anti-patterns (trojan mechanical, hidden defaults, stack-only readable, etc.): [review.md](review.md) §hidden-risk.

---

## Slice order

0. CODEOWNERS / team boundaries
1. Module / dependency order (package API before consumer; shims if needed)
2. Mechanical vs behavioral (not line-count driven)
3. Unrelated concepts (feature/API/privilege, not directory)
4. BBA + dark-ship: types → wire legacy → dark-ship new (`const useNew = false`) → flip → remove legacy
5. Noise early: lockfile, generated mass updates, i18n JSON, Scout scaffold, bulk `kibana.jsonc`
6. See [review.md](review.md) §hidden-risk for anti-patterns

**Exception:** prettier/eslint from the behavioral edit in the same files stays in that PR.

**Titles:** one [review question](review.md)—not "Part 2" or file lists. Split if the title needs "and" for unrelated concerns.

### CODEOWNERS

1. [`.github/CODEOWNERS`](../../../.github/CODEOWNERS) · nested wins · `kibana.jsonc` `owner` · `node src/dev/stage_by_owner.ts`

Rules: one PR per team when separable; same team + related feature → one slice; no owner → `NEEDS DECISION`.

### Stack

`upstream/main` → PR1 → PR2 → … only. Build: types/shims/APIs first. Merge: foundations before behavioral flips.

### Mergeability

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.

---

## Splittability edits

Not pure git surgery—**edit code** when hunks, compile order, or review boundaries block a clean stack. Scaffolding for review only; design approved.

| Signal | Edit |
|--------|------|
| Consumer needs package exports | Temporary re-export/shim |
| Old + new impl in one file | BBA + dark-ship |
| Two questions in one file | Extract or `const useNew = false` |
| SO / privilege separate landing | Split schema from behavior |
| Mechanical + behavioral same hunk | Separate files or gated branches |
| PR N needs PR N+1 symbols | Stub, deprecated wrapper, `import type` |

**In scope:** dark-ship toggles, temp re-exports/no-ops, extracted files, lazy plugin entry, test doubles, hunk moves.
**Out of scope:** unrelated fixes, permanent API you wouldn't ship as one PR, early behavior enablement.

**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.

Record: `none` or edits **in this PR**; note PR # that removes scaffolding.
Loading