mirror of
https://github.com/n8n-io/n8n.git
synced 2026-10-11 22:50:06 +00:00
chore: Update community PR review skill (no-changelog) (#31380)
This commit is contained in:
Jon
@@ -0,0 +1,239 @@
|
||||
---
|
||||
description: >-
|
||||
Checks if a community pull request is ready for human review. Verifies CLA
|
||||
signature, PR title format, description completeness, test coverage, and
|
||||
cubic-dev-ai issues, then triages to the right Linear team or recommends a
|
||||
close. Use when given a PR number or branch name to review, or when the user
|
||||
says /community-pr-readiness-check, or asks to check if a PR is ready for
|
||||
review.
|
||||
allowed-tools: Bash(gh:*), Bash(git:*), Bash(node:*), Read, Glob, Grep, AskUserQuestion, mcp__linear-server__save_issue, mcp__linear-server__get_issue, mcp__linear-server__list_teams, mcp__linear-server__list_issue_statuses
|
||||
---
|
||||
|
||||
# Community PR Readiness Check
|
||||
|
||||
Given a PR number or branch name, determine whether it is ready for human review and take the right follow-up action.
|
||||
|
||||
## Decision tree
|
||||
|
||||
1. **Bot author** (`n8n-cat-bot` / `aikido-autofix`) → cleanup-only, no review. See "Internal automation PRs" below.
|
||||
2. **Auto-rejection screen matches** (typo-only / unsanctioned new node) → action path **D — close** with the matching template.
|
||||
3. **All checks pass** (`readyForReview === true`) → action path **B — triage to team**.
|
||||
4. **One or more checks fail** → action path **A** (if title is minor-fix only) then **C — post comment**.
|
||||
|
||||
## Step 1 — Resolve the PR
|
||||
|
||||
If given a branch name, find the PR number first:
|
||||
|
||||
```bash
|
||||
gh pr view <branch> --repo n8n-io/n8n --json number --jq .number
|
||||
```
|
||||
|
||||
## Step 2 — Fetch and pre-process
|
||||
|
||||
```bash
|
||||
gh pr view <number> --repo n8n-io/n8n \
|
||||
--json number,title,body,author,headRefName,headRefOid,files,isDraft,state,labels
|
||||
```
|
||||
|
||||
### Internal automation PRs (bot authors)
|
||||
|
||||
If `author.login` is one of n8n's internal bots — `n8n-cat-bot` / `app/n8n-cat-bot` or `aikido-autofix` / `app/aikido-autofix` — skip the PR entirely and perform the cleanup actions below. Do **not** emit any JSON output.
|
||||
|
||||
1. Relabel the PR (both bots): swap `community` → `n8n team`:
|
||||
```bash
|
||||
gh pr edit <number> --repo n8n-io/n8n --remove-label community --add-label "n8n team"
|
||||
```
|
||||
2. Update the linked Linear ticket (extract `GHC-XXXX` per step 5):
|
||||
- **`n8n-cat-bot`** — cancel: `mcp__linear-server__save_issue` with `state: "Canceled"`, no labels.
|
||||
- **`aikido-autofix`** — route to Dev Platform: `mcp__linear-server__save_issue` with `team: "Developer Platform"`, `state: "Triage"`, no labels.
|
||||
|
||||
When reviewing a batch, omit the skipped PR from the output. For a single PR, emit a one-line note (e.g. `Skipped & cleaned up #30591 (n8n-cat-bot): relabeled to n8n team, cancelled GHC-8398.`).
|
||||
|
||||
### Collision guard
|
||||
|
||||
If `triage:in-progress` is already on the PR, another reviewer is mid-triage — **bail out** to avoid double-processing. Emit a one-line note (e.g. `Skipped #30205: already has triage:in-progress`) and move on to the next PR. Do not run the checks, do not modify labels, do not touch Linear.
|
||||
|
||||
If the user explicitly asks to re-process a PR that's stuck on `triage:in-progress` (e.g. a previous run crashed), they can clear the label manually and re-invoke.
|
||||
|
||||
### Otherwise — mark in-progress
|
||||
|
||||
Strip any existing `triage:*` state label before adding `triage:in-progress`, so the single-state invariant holds even when re-reviewing a PR that was previously sent back with `triage:needs-info` or `triage:tests-needed`:
|
||||
|
||||
```bash
|
||||
gh pr edit <number> --repo n8n-io/n8n \
|
||||
--remove-label "triage:pending" \
|
||||
--remove-label "triage:needs-info" \
|
||||
--remove-label "triage:tests-needed" \
|
||||
--remove-label "triage:complete" \
|
||||
--add-label "triage:in-progress"
|
||||
```
|
||||
|
||||
Only one of those `triage:*` labels will actually be present; `--remove-label` errors when a label is missing, so run each removal as its own call (or batch and ignore errors) and then do the add. A PR carries exactly one `triage:<state>` label at a time; the skill replaces `triage:in-progress` with a terminal state before exit (see `reference/label-flow.md`).
|
||||
|
||||
### Also fetch (in parallel)
|
||||
|
||||
```bash
|
||||
# cubic-dev-ai PR review comments (for check E)
|
||||
gh api --paginate "repos/n8n-io/n8n/pulls/<number>/comments" \
|
||||
--jq '.[] | select(.user.login == "cubic-dev-ai[bot]") | {body: .body, path: .path}'
|
||||
|
||||
# n8n-assistant issue comments (for the Linear ticket reference)
|
||||
gh api --paginate "repos/n8n-io/n8n/issues/<number>/comments" \
|
||||
--jq '[.[] | select(.user.login == "n8n-assistant[bot]" or .user.login == "n8n-assistant") | .body] | join("\n")'
|
||||
```
|
||||
|
||||
## Step 2.5 — Auto-rejection screen
|
||||
|
||||
Per [`CONTRIBUTING.md`](../../../../../CONTRIBUTING.md), two PR patterns should be closed outright rather than reviewed:
|
||||
|
||||
- **Typo-only PR** — diff is entirely spelling/grammar fixes with no logic or tests.
|
||||
- **New-node PR** — adds a brand-new node, unless the n8n team has explicitly agreed to take it.
|
||||
|
||||
If either matches, set `checks.AutoReject` and skip directly to action **D**. Full rules and how to verify each pattern: see `reference/checks.md`.
|
||||
|
||||
## Step 3 — Run the five checks
|
||||
|
||||
Run when `AutoReject` is `null`. Full rules for each in `reference/checks.md`:
|
||||
|
||||
- **A. CLA** — `cla-signed` label present.
|
||||
- **B. Title** — matches the conventional-commit regex. Authoritative rules in [`.github/pull_request_title_conventions.md`](../../../../../.github/pull_request_title_conventions.md).
|
||||
- **C. Description** — every section heading and checklist item from [`.github/pull_request_template.md`](../../../../../.github/pull_request_template.md) is present in the PR body. The template is read at check time, so changes to it propagate automatically.
|
||||
- **D. Tests** — source logic changes have matching test files. Skip for `docs/ci/chore/build` PRs.
|
||||
- **E. cubic-dev-ai** — no unresolved comments (resolved = "Addressed in commit" marker).
|
||||
|
||||
## Step 4 — Identify the responsible team
|
||||
|
||||
Run `node .github/scripts/owners.mjs` against the changed file list and map the winning GitHub team to a Linear team. Full mapping table, sub-agent fallback procedure, and label rules: see `reference/teams.md`.
|
||||
|
||||
## Step 5 — Extract the Linear ticket
|
||||
|
||||
n8n-assistant leaves a comment on every community PR containing `This PR has been added to our internal tracker as "GHC-XXXX"`. Search the concatenated n8n-assistant comment body for `\bGHC-\d+\b`, take the first match.
|
||||
|
||||
If no n8n-assistant comment exists (older PRs that predate the automation), `linearTicket` is `null`.
|
||||
|
||||
## Step 6 — Output JSON
|
||||
|
||||
```json
|
||||
{
|
||||
"readyForReview": <true if all passing checks allow merge, false otherwise>,
|
||||
"messageForUser": "<Short message to the contributor listing what they need to address. 'N/A' if ready.>",
|
||||
"team": "<Linear team name (from reference/teams.md), or 'Engineering' as fallback>",
|
||||
"linearTicket": "<GHC-XXXX or null>",
|
||||
"checks": {
|
||||
"AutoReject": <"typo-only" | "new-node" | null>,
|
||||
"CLA": <bool>,
|
||||
"Title": <bool>,
|
||||
"Description": <bool>,
|
||||
"TestsNeeded": <bool>,
|
||||
"TestsIncluded": <bool>,
|
||||
"CubicIssues": <true if unresolved cubic issues exist, false otherwise>
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`readyForReview` is `true` only when: `AutoReject` is `null`; `CLA`, `Title`, and `Description` are all `true`; `CubicIssues` is `false`; and either `TestsNeeded` is `false` or `TestsIncluded` is `true`. If `AutoReject` is set, `readyForReview` is always `false`.
|
||||
|
||||
Emit the JSON first, then take the appropriate action path below.
|
||||
|
||||
## Step 7 — Action paths
|
||||
|
||||
Use `AskUserQuestion` for each prompt. Sub-agents called for analysis only should stop after step 6 and let the caller drive step 7.
|
||||
|
||||
### A — Minor title fix
|
||||
|
||||
A title issue is **minor** if it can be repaired by a deterministic transformation:
|
||||
|
||||
- Leading or trailing whitespace.
|
||||
- First letter of the summary in the wrong case.
|
||||
- Trailing period.
|
||||
- Mixed case `revert:` requiring lowercase (no change needed, just flag).
|
||||
|
||||
If the *only* failing check is `Title` (or `Title` + `CubicIssues`) and the issue is minor, propose the fix and ask `Apply proposed / Edit before applying / Skip`. Apply with:
|
||||
|
||||
```bash
|
||||
gh pr edit <number> --repo n8n-io/n8n --title "<new title>"
|
||||
```
|
||||
|
||||
Then re-evaluate `Title` (now passes) and continue to **B** or **C**. Non-minor title problems (wrong/missing type, no colon, hyphenated scope) need contributor input — skip A and go to **C**.
|
||||
|
||||
### B — Triage to team (`readyForReview === true`)
|
||||
|
||||
Ask: *"PR is ready for review. Assign Linear ticket `<linearTicket>` to team `<team>` and move to <destination state>?"* Options: `Yes, assign and triage` / `No, leave as-is`.
|
||||
|
||||
Destination state: `Review` for NODES, `Triage` for every other team. Label composition: see `reference/teams.md`.
|
||||
|
||||
On `Yes`:
|
||||
|
||||
```python
|
||||
# 1. Linear
|
||||
mcp__linear-server__save_issue(
|
||||
id=linearTicket,
|
||||
team=<team>,
|
||||
state=<destination>,
|
||||
labels=<computed labels>,
|
||||
)
|
||||
# 2. GitHub (only if Linear succeeded) — see reference/label-flow.md
|
||||
gh pr edit <number> --repo n8n-io/n8n \
|
||||
--remove-label "triage:in-progress" \
|
||||
--remove-label "status:pending-assignment" \
|
||||
--add-label "team:<slug>" \
|
||||
--add-label "status:team-assigned" \
|
||||
--add-label "triage:complete"
|
||||
```
|
||||
|
||||
If `linearTicket` is `null`, ask whether to create a new Linear ticket before triaging (older PRs predating n8n-assistant). Otherwise skip B and ask the user.
|
||||
|
||||
### C — Post contributor comment (`readyForReview === false`, no auto-reject)
|
||||
|
||||
Show `messageForUser` and ask `Post as-is / Edit before posting / Skip`. On post:
|
||||
|
||||
```bash
|
||||
gh pr comment <number> --repo n8n-io/n8n --body "<final message>"
|
||||
```
|
||||
|
||||
Then apply the right terminal triage label — exactly one, priority `triage:tests-needed` > `triage:needs-info`. See `reference/label-flow.md`. On `Skip`, leave the PR on `triage:in-progress` so the next loop picks it up.
|
||||
|
||||
Skip C entirely if A already handled the only failing check and the PR is now ready — run B instead.
|
||||
|
||||
### D — Close the PR
|
||||
|
||||
Used when the PR should be closed rather than reviewed. Three common triggers:
|
||||
|
||||
1. **Auto-rejection** (`AutoReject` set) — typo-only or unsanctioned new node.
|
||||
2. **Duplicate** — another open PR addresses the same change.
|
||||
3. **Out of scope / bundled** — multiple unrelated fixes that should be split, or scope n8n team has declined.
|
||||
|
||||
Ask `Close + comment / Edit before closing / Skip`. Templates below; pick one and adapt to the contributor and specifics.
|
||||
|
||||
**Typo-only:**
|
||||
> Thanks for taking the time to send this in! Per our [contributing guide](../blob/master/CONTRIBUTING.md#community-pr-guidelines) we don't accept typo-only PRs — they create review overhead without changing functionality, and our spell-checker rules cover most cases automatically. Closing this for now; please feel free to open a PR that pairs a typo fix with a related logic change. 🙏
|
||||
|
||||
**New node:**
|
||||
> Thanks for the contribution! n8n no longer accepts new nodes directly into the core monorepo unless the team has explicitly agreed to scope one in. Please publish this as a [community node](https://docs.n8n.io/integrations/creating-nodes/overview/) instead — that gives you full ownership and avoids the long review queue here. Closing this PR per our [contributing guide](../blob/master/CONTRIBUTING.md#community-pr-guidelines).
|
||||
|
||||
**Duplicate of another PR:**
|
||||
> Thanks for the contribution! This change is already being handled in #<other-pr>, which is further along in review. Closing this in favour of that PR to keep the queue tidy — please feel free to chime in over there if there's anything missing.
|
||||
|
||||
**Bundled / out of scope:**
|
||||
> Thanks for the contribution! Per our [contributing guide](../blob/master/CONTRIBUTING.md#community-pr-guidelines) we ask for one focused change per PR. This PR bundles <N> unrelated fixes — please reopen them as separate, focused PRs, each with the [template](../blob/master/.github/pull_request_template.md) filled in and a unit test that locks in the regression. Closing this one in the meantime. 🙏
|
||||
|
||||
Close action (same for every reason):
|
||||
|
||||
```bash
|
||||
gh pr comment <number> --repo n8n-io/n8n --body "<final message>"
|
||||
gh pr close <number> --repo n8n-io/n8n
|
||||
gh pr edit <number> --repo n8n-io/n8n \
|
||||
--remove-label "triage:in-progress" \
|
||||
--remove-label "status:pending-assignment" \
|
||||
--add-label "status:internal-closed" \
|
||||
--add-label "triage:complete"
|
||||
```
|
||||
|
||||
If `linearTicket` is set, also cancel it: `mcp__linear-server__save_issue(id=linearTicket, state="Canceled")`. If `gh pr close` reports the PR is already closed (contributor beat you to it), proceed with the comment, labels, and ticket cancellation anyway.
|
||||
|
||||
## Notes
|
||||
|
||||
- **Draft PRs** — report all findings but note the PR is a draft.
|
||||
- **Already merged or closed** — say so and skip the checks (don't apply triage labels).
|
||||
- **Re-reviewing a PR you've already commented on** — use the GitHub Timeline API to detect contributor activity since the last skill touch. See `reference/re-review.md`.
|
||||
- **Label state machine** — single `triage:<state>` label at any time; transitions documented in `reference/label-flow.md`.
|
||||
@@ -0,0 +1,140 @@
|
||||
# Community PR Readiness Check — checks
|
||||
|
||||
The full ruleset for the auto-rejection screen and the five readiness checks. Loaded when the skill is mid-review.
|
||||
|
||||
## Contents
|
||||
|
||||
- Step 2.5 — Auto-rejection screening
|
||||
- Typo-only PR
|
||||
- New-node PR
|
||||
- The five checks (run after auto-rejection screen clears)
|
||||
- A. CLA signed
|
||||
- B. PR title format
|
||||
- C. PR description completeness
|
||||
- D. Tests
|
||||
- E. cubic-dev-ai issues
|
||||
|
||||
---
|
||||
|
||||
## Step 2.5 — Auto-rejection screening
|
||||
|
||||
Before running the five checks, screen for PRs that the project policy in [`CONTRIBUTING.md`](../../../../../../CONTRIBUTING.md) (section "Community PR Guidelines") says should be rejected outright. If a PR matches one of these patterns, skip the five checks and recommend a polite close instead.
|
||||
|
||||
### Typo-only PR
|
||||
|
||||
The diff consists **entirely** of:
|
||||
|
||||
- Spelling fixes in code comments, error messages, or user-visible description strings.
|
||||
- No new tests, no logic changes, no behaviour changes, no config changes.
|
||||
|
||||
Per CONTRIBUTING.md: *"Typos are not sufficient justification for a PR and will be rejected."* Use `gh pr diff <number>` to verify the diff is purely text-edit. A PR that mixes a typo fix with a real logic fix is **not** typo-only — review it normally.
|
||||
|
||||
### New-node PR
|
||||
|
||||
The diff adds a new node entry: a new `*.node.ts` (and usually a sibling credentials file) under `packages/nodes-base/nodes/<NewNode>/` or `packages/@n8n/nodes-langchain/nodes/<NewNode>/`, with no corresponding node already existing in the codebase.
|
||||
|
||||
Per CONTRIBUTING.md: *"PRs that introduce new nodes will be auto-closed unless they are explicitly requested by the n8n team."* If the user calling the skill states the n8n team has agreed to take this node, proceed with the normal review; otherwise flag for close with a pointer to the [Community Nodes](https://docs.n8n.io/integrations/creating-nodes/overview/) flow.
|
||||
|
||||
### When a screen matches
|
||||
|
||||
Skip the five checks. Set `checks.AutoReject` in the JSON output to `"typo-only"` or `"new-node"` and proceed to action path 7D in SKILL.md (close the PR).
|
||||
|
||||
---
|
||||
|
||||
## The five checks
|
||||
|
||||
Run these in order when step 2.5 clears (`AutoReject` is `null`).
|
||||
|
||||
### A. CLA signed
|
||||
|
||||
Check the PR `labels` returned from `gh pr view`:
|
||||
|
||||
- `cla-signed` label present → ✅ signed
|
||||
- `cla-signed` label missing → ❌ not signed
|
||||
|
||||
### B. PR title format
|
||||
|
||||
The authoritative title rules live in [`.github/pull_request_title_conventions.md`](../../../../../../.github/pull_request_title_conventions.md). Read that file at the start of the check — the allowed `type` list and scope rules come from there, not from this skill.
|
||||
|
||||
The matching regex below is a cached extraction of those rules. If the conventions file disagrees with the regex (a new type, a different scope syntax), trust the file and flag the divergence in your output.
|
||||
|
||||
For all types except `revert`:
|
||||
|
||||
```
|
||||
^(feat|fix|perf|test|docs|refactor|build|ci|chore)(\([a-zA-Z0-9 ]+( Node)?\))?!?: [A-Z].+[^.]$
|
||||
```
|
||||
|
||||
For `revert` titles, the summary is the original commit header (which starts with a lowercase type), so capitalization is not enforced:
|
||||
|
||||
```
|
||||
^revert(\([a-zA-Z0-9 ]+( Node)?\))?!?: .+[^.]$
|
||||
```
|
||||
|
||||
Quick recap of what the regex enforces (full detail in the conventions file):
|
||||
|
||||
- Type must be one of: `feat fix perf test docs refactor build ci chore revert`.
|
||||
- Scope is optional, in parentheses; characters limited to `[a-zA-Z0-9 ]` — hyphens like `(nodes-base)` fail.
|
||||
- Breaking changes: `!` before the colon.
|
||||
- Summary: starts with capital letter (lowercase allowed for `revert:`), no trailing period.
|
||||
- No Linear ticket IDs in the title (e.g. `N8N-1234`).
|
||||
|
||||
### C. PR description completeness
|
||||
|
||||
The PR template at [`.github/pull_request_template.md`](../../../../../../.github/pull_request_template.md) is the **source of truth** for what a complete PR description looks like. Read it at the start of the check — sections and checklist items shouldn't be hardcoded here.
|
||||
|
||||
Procedure:
|
||||
|
||||
1. Read the template file and extract:
|
||||
- Every `## Heading` (the expected section list).
|
||||
- Every `- [ ]` checkbox item under the `## Review / Merge checklist` heading.
|
||||
2. For each `## Heading` from the template, check the PR body contains a heading with the same text. Treat headings case-insensitively but require the exact text — `## Related Issue` does **not** satisfy `## Related Linear tickets, Github issues, and Community forum posts`.
|
||||
3. For the `## Summary` heading specifically, require non-empty content below it (the HTML comment placeholder doesn't count as content).
|
||||
4. For the checklist, require every extracted checkbox item to appear in the PR body. Match by the item text (after stripping `- [ ]` / `- [x]`); the boxes can be checked or unchecked — community PRs commonly leave them unchecked, which is fine.
|
||||
5. For the "Related tickets" section: if the heading is present, accept any of these as content: a URL (`http`), a GitHub closing keyword (`closes #N`, `fixes #N`, `resolves #N`), or empty (only the HTML comment). Only flag if the heading is missing entirely.
|
||||
|
||||
When the check fails, report **which** section or checkbox item is missing — that's what the contributor message includes.
|
||||
|
||||
### D. Tests
|
||||
|
||||
Skip this check entirely if the PR type (from the title) is `docs`, `ci`, `chore`, or `build`.
|
||||
|
||||
Otherwise:
|
||||
|
||||
1. Identify source files changed: non-test files under `packages/` from the `files` list.
|
||||
2. If there are source file changes, read the diff via `gh pr diff`:
|
||||
```bash
|
||||
gh pr diff <number> --repo n8n-io/n8n
|
||||
```
|
||||
3. Use the diff to judge whether the changes introduce logic that warrants tests (new functions, bug fixes, behaviour changes, data transformations). Pure config changes, type-only changes, and trivial renames do not require tests.
|
||||
4. Look for matching test files (`*.test.ts`, `*.spec.ts`, files inside `__tests__/`) among the changed files.
|
||||
|
||||
**Fallback only if needed**: if the diff alone is insufficient (e.g. you need to read the full surrounding function body to assess whether a refactor preserved behaviour), check out a temporary worktree:
|
||||
|
||||
```bash
|
||||
git fetch origin pull/<number>/head:pr/<number>
|
||||
git worktree add /tmp/pr-<number>-review pr/<number>
|
||||
# … inspect files …
|
||||
git worktree remove /tmp/pr-<number>-review --force
|
||||
git branch -D pr/<number>
|
||||
```
|
||||
|
||||
Always clean up the worktree afterwards. In sub-agent contexts, `git fetch` and `git worktree add` are often denied by the sandbox — that's fine, the `gh pr diff` path handles almost every PR on its own.
|
||||
|
||||
Report:
|
||||
|
||||
- ✅ Tests present, or change does not require tests
|
||||
- ❌ Source logic changed but no test files found
|
||||
|
||||
### E. cubic-dev-ai issues
|
||||
|
||||
Fetch the PR review comments:
|
||||
|
||||
```bash
|
||||
gh api --paginate "repos/n8n-io/n8n/pulls/<number>/comments" \
|
||||
--jq '.[] | select(.user.login == "cubic-dev-ai[bot]") | {body: .body, path: .path}'
|
||||
```
|
||||
|
||||
`cubic-dev-ai[bot]` leaves a comment for every issue it finds. An issue is **resolved** if its comment body contains an "addressed in commit" marker — typically `✅ Addressed in [<sha>]` or `Addressed in <sha>`. Resolved issues count as if they were never raised.
|
||||
|
||||
- No comments, every comment explicitly states no issues were found, or every issue is marked addressed → ✅
|
||||
- One or more unresolved comments → ❌ report the unresolved count and priority breakdown (e.g. "3 unresolved issues: 1× P1, 1× P2, 1× P3")
|
||||
@@ -0,0 +1,92 @@
|
||||
# Community PR Readiness Check — triage label flow
|
||||
|
||||
A PR should carry **exactly one** `triage:<state>` label at any time. The skill drives the PR through this state machine.
|
||||
|
||||
## States
|
||||
|
||||
| State | Meaning |
|
||||
|------------------------|------------------------------------------------------------------------|
|
||||
| `triage:pending` | Auto-applied by n8n-assistant when the PR opens. Skill removes it. |
|
||||
| `triage:in-progress` | Skill is actively reviewing. Set in step 2, replaced before exit. |
|
||||
| `triage:complete` | PR has been triaged to a team (or closed). Terminal. |
|
||||
| `triage:needs-info` | Comment posted; contributor needs to address something non-test. |
|
||||
| `triage:tests-needed` | Comment posted; contributor needs to add tests (priority over info). |
|
||||
|
||||
## Transitions
|
||||
|
||||
### Skill entry (step 2)
|
||||
|
||||
Before transitioning, **check for an existing `triage:in-progress` label**. If present, another reviewer is mid-triage — bail out without touching labels, Linear, or comments. This prevents two parallel runs from double-processing the same PR.
|
||||
|
||||
Strip any existing `triage:*` state label before adding `triage:in-progress`, so the single-state invariant holds even when re-reviewing a PR that was previously sent back with `triage:needs-info` or `triage:tests-needed`:
|
||||
|
||||
```bash
|
||||
gh pr edit <number> --repo n8n-io/n8n \
|
||||
--remove-label "triage:pending" \
|
||||
--remove-label "triage:needs-info" \
|
||||
--remove-label "triage:tests-needed" \
|
||||
--remove-label "triage:complete" \
|
||||
--add-label "triage:in-progress"
|
||||
```
|
||||
|
||||
Only one of those `triage:*` labels will actually be present; `--remove-label` errors when a label is missing, so run each removal as its own call (or batch and ignore errors) and then do the add.
|
||||
|
||||
### Skill exit — branch by outcome
|
||||
|
||||
#### Triaged to a team (action path 7B)
|
||||
|
||||
```bash
|
||||
gh pr edit <number> --repo n8n-io/n8n \
|
||||
--remove-label "triage:in-progress" \
|
||||
--remove-label "status:pending-assignment" \
|
||||
--add-label "team:<slug>" \
|
||||
--add-label "status:team-assigned" \
|
||||
--add-label "triage:complete"
|
||||
```
|
||||
|
||||
The `team:<slug>` is taken from `reference/teams.md`. Apply only after the Linear `save_issue` call succeeds — if Linear fails, leave the PR on `triage:in-progress` for the next loop run. `status:pending-assignment` may not be present (e.g. older PRs that predate the convention) — if `--remove-label` errors, drop the flag and retry, or run it as a separate call.
|
||||
|
||||
#### Closed (action path 7D)
|
||||
|
||||
```bash
|
||||
gh pr edit <number> --repo n8n-io/n8n \
|
||||
--remove-label "triage:in-progress" \
|
||||
--remove-label "status:pending-assignment" \
|
||||
--add-label "status:internal-closed" \
|
||||
--add-label "triage:complete"
|
||||
```
|
||||
|
||||
Apply alongside `gh pr close <number>` and the close-comment template (see SKILL.md). If the PR is already closed (`gh pr close` reports "already closed"), still apply the labels and the Linear cancellation.
|
||||
|
||||
#### Comment posted, contributor must act (action path 7C)
|
||||
|
||||
Pick **one** terminal label using this priority:
|
||||
|
||||
| Condition | Label |
|
||||
|------------------------------------------------------------------------|------------------------|
|
||||
| `TestsNeeded === true` **and** `TestsIncluded === false` | `triage:tests-needed` |
|
||||
| Any other failing check (`CLA`, `Title`, `Description`, `CubicIssues`) | `triage:needs-info` |
|
||||
|
||||
When both conditions hold, `triage:tests-needed` wins — it's the more specific signal and the comment already covers everything else.
|
||||
|
||||
```bash
|
||||
gh pr edit <number> --repo n8n-io/n8n \
|
||||
--remove-label "triage:in-progress" \
|
||||
--add-label "triage:tests-needed" # or triage:needs-info
|
||||
```
|
||||
|
||||
#### Skill aborted before posting
|
||||
|
||||
If the user picks `Skip` at the "post this comment?" prompt, leave the PR on `triage:in-progress` — don't apply a terminal label without an accompanying contributor message. The next loop run will pick it up.
|
||||
|
||||
## status:* labels
|
||||
|
||||
These are independent of the triage state and signal where the PR sits in the n8n internal flow:
|
||||
|
||||
| Label | Applied when |
|
||||
|-----------------------------|----------------------------------------------------|
|
||||
| `status:team-assigned` | PR triaged to a Linear team (path 7B). |
|
||||
| `status:internal-closed` | PR closed via path 7D. |
|
||||
| `status:pending-assignment` | Default before the skill runs (set elsewhere). |
|
||||
|
||||
The skill writes `status:team-assigned` (path 7B) or `status:internal-closed` (path 7D) and clears `status:pending-assignment` at the same time, so a triaged or closed PR drops out of any `status:pending-assignment` queue. It does not touch other `status:*` labels — reconciliation between team-assigned/internal-closed happens elsewhere if needed.
|
||||
@@ -0,0 +1,55 @@
|
||||
# Community PR Readiness Check — re-reviewing a PR you've already commented on
|
||||
|
||||
When looping over the GHC queue, skip PRs you reviewed before unless the contributor has actually done something. This avoids burning agent runs on PRs that are still waiting on the contributor.
|
||||
|
||||
## What does "the contributor did something" mean?
|
||||
|
||||
Any of:
|
||||
|
||||
- New commit pushed (`committed` event)
|
||||
- Force-push / rebase (`head_ref_force_pushed`)
|
||||
- Comment added (`commented`)
|
||||
- Review submitted (`reviewed`)
|
||||
|
||||
Label changes, description edits by us, and skill-posted comments don't count.
|
||||
|
||||
## Use the GitHub Timeline API
|
||||
|
||||
The timeline endpoint returns every event on the PR with `actor.login` and a timestamp, so we can distinguish "contributor activity" from "skill activity" cleanly. This replaces an earlier heuristic that compared `committedDate` to the last skill comment — too narrow, since it missed contributor comments and force-pushes.
|
||||
|
||||
```bash
|
||||
SKILL_USER=$(gh api user --jq .login)
|
||||
gh api --paginate "repos/n8n-io/n8n/issues/<number>/timeline" \
|
||||
-H "Accept: application/vnd.github+json"
|
||||
```
|
||||
|
||||
## Procedure
|
||||
|
||||
1. Resolve the skill user's login (`$SKILL_USER` above). This is the GitHub handle the skill is running as.
|
||||
2. Fetch the full timeline.
|
||||
3. Find the **last skill-authored event** — any event where `actor.login == $SKILL_USER` (labels applied, comments posted, etc.). Take its timestamp as `lastSkillAt`. If no skill activity exists → first-time review, run the full check.
|
||||
4. Scan for **contributor activity after `lastSkillAt`** — events where:
|
||||
- `event` is one of `committed`, `head_ref_force_pushed`, `commented`, `reviewed`,
|
||||
- AND, for `commented` / `reviewed` / `head_ref_force_pushed`, `actor.login != $SKILL_USER` (skill comments shouldn't trigger re-review of themselves),
|
||||
- AND the event timestamp > `lastSkillAt`.
|
||||
|
||||
Timestamp field varies by event type:
|
||||
|
||||
| Event | Timestamp field |
|
||||
|---------------------------|--------------------------|
|
||||
| `committed` | `committer.date` |
|
||||
| `head_ref_force_pushed` | `created_at` |
|
||||
| `commented` | `created_at` |
|
||||
| `reviewed` | `submitted_at` |
|
||||
|
||||
5. Decide:
|
||||
- One or more qualifying events → run the full review (steps 2–7 in SKILL.md).
|
||||
- None → skip the PR. Report "no contributor activity since previous review" and move on.
|
||||
|
||||
For `committed` events, there's no `actor.login` — the actor is the commit author email. Since the skill never pushes commits, treat every `committed` event as contributor activity unconditionally.
|
||||
|
||||
## Edge case: description-only edits
|
||||
|
||||
Body edits don't generate timeline events, so a contributor who only updates the PR description (e.g. fills in a missing template section) won't be picked up. That's a GitHub API limitation; the skill can't see body edits without diffing against a cached snapshot.
|
||||
|
||||
If the user explicitly mentions a PR they edited, just re-review it — that's faster than working around the API.
|
||||
@@ -0,0 +1,104 @@
|
||||
# Community PR Readiness Check — teams and labels
|
||||
|
||||
Owner resolution, the GitHub-team → Linear-team → label mapping, and the Linear ticket label rules used when triaging.
|
||||
|
||||
## Contents
|
||||
|
||||
- Identifying the owning team
|
||||
- GitHub team → Linear team → GitHub label
|
||||
- Linear ticket label rules
|
||||
- Always-on label
|
||||
- Type label (based on PR title prefix)
|
||||
- Team-specific extra label
|
||||
- Worked examples
|
||||
|
||||
---
|
||||
|
||||
## Identifying the owning team
|
||||
|
||||
Use the canonical owners script at `.github/scripts/owners.mjs`. It parses `.github/OWNERS` with last-match-wins semantics and returns allocations sorted by file count with a `share` percentage. Using the script keeps this skill consistent with whatever CI uses.
|
||||
|
||||
1. Write the PR's changed file paths (from the `files` list) to a temp file, one per line:
|
||||
```bash
|
||||
printf '%s\n' <path1> <path2> ... > /tmp/pr-<number>-files.txt
|
||||
```
|
||||
2. Run the script:
|
||||
```bash
|
||||
node .github/scripts/owners.mjs /tmp/pr-<number>-files.txt
|
||||
```
|
||||
3. The script prints JSON of the form:
|
||||
```json
|
||||
{
|
||||
"totalFiles": 12,
|
||||
"allocations": [
|
||||
{ "team": "@n8n-io/ai", "fileCount": 10, "share": 83, "files": [...] },
|
||||
{ "team": "@n8n-io/catalysts", "fileCount": 2, "share": 17, "files": [...] }
|
||||
]
|
||||
}
|
||||
```
|
||||
Allocations are already sorted by `fileCount` descending — take the first entry as the winning team.
|
||||
4. Clean up: `rm /tmp/pr-<number>-files.txt`.
|
||||
5. Strip the `@n8n-io/` prefix from `allocations[0].team` — the GitHub team slug is `nodes`, `iam`, `ai`, etc. If `allocations` is empty (no file matched any rule, which is possible only if `.github/OWNERS` lost its catch-all), fall back to `catalysts`.
|
||||
6. Map the GitHub team slug to its Linear team name and PR label using the table below. The `team` field in the JSON output is the **Linear team name**. If the resolved GitHub team slug has no entry in the table, fall back to `Engineering`.
|
||||
|
||||
**Sub-agent fallback**: if `node` execution is denied by the sandbox, read `.github/OWNERS` directly and apply last-match-wins by hand. All the active rules fit on one screen.
|
||||
|
||||
## GitHub team → Linear team → GitHub label
|
||||
|
||||
| GitHub team (`@n8n-io/…`) | Linear team | GitHub team label |
|
||||
|---------------------------|-------------------------|--------------------|
|
||||
| `catalysts` | Catalysts | `team:cats` |
|
||||
| `adore` | Adore | `team:adore` |
|
||||
| `ai` | AI | `team:ai` |
|
||||
| `nodes` | NODES | `team:nodes` |
|
||||
| `design` | Design | `team:design` |
|
||||
| `iam` | Identity & Access | `team:identity` |
|
||||
| `ligo` | Lifecycle & Governance | `team:lifecycle` |
|
||||
| `instance-ai` | instanceAI | `team:instance-ai` |
|
||||
| `frontend` | Adore | `team:adore` |
|
||||
| `qa-dx` | Developer Platform | `team:qa-dx` |
|
||||
| `migrations-review` | Catalysts | `team:cats` |
|
||||
|
||||
The **GitHub team label** column is what gets applied to the PR after a successful Linear assignment (see `reference/label-flow.md`).
|
||||
|
||||
## Linear ticket label rules
|
||||
|
||||
When calling `mcp__linear-server__save_issue` to assign a ticket to a team, pass a `labels` array composed of three pieces:
|
||||
|
||||
### Always-on label
|
||||
|
||||
- `GitHub` — every community PR ticket carries this. In the Linear UI it renders as `source > GitHub`; the `source` prefix is a label-group parent, not part of the API name.
|
||||
|
||||
### Type label (from PR title prefix)
|
||||
|
||||
| PR title type | Linear label |
|
||||
|----------------------------------------------------------------------------------------|----------------|
|
||||
| `feat` | `feature` |
|
||||
| `fix` | `bug` |
|
||||
| anything else (`perf`, `refactor`, `docs`, `ci`, `chore`, `build`, `test`, `revert`) | `enhancement` |
|
||||
|
||||
Pass the **child** label name only — Linear silently drops unknown labels, so don't include `type > ` prefixes.
|
||||
|
||||
### Team-specific extra label
|
||||
|
||||
| Destination team | Extra label |
|
||||
|------------------|----------------|
|
||||
| Catalysts | `Community PR` |
|
||||
| NODES | `community-pr` |
|
||||
| other teams | _(no extra)_ |
|
||||
|
||||
### Worked examples
|
||||
|
||||
- `feat` PR going to Catalysts: `["GitHub", "feature", "Community PR"]`
|
||||
- `fix` PR going to NODES: `["GitHub", "bug", "community-pr"]`
|
||||
- `perf` PR going to AI: `["GitHub", "enhancement"]`
|
||||
- `chore` PR going to Lifecycle & Governance: `["GitHub", "enhancement"]`
|
||||
|
||||
## Destination state
|
||||
|
||||
| Destination team | Linear state |
|
||||
|------------------|--------------|
|
||||
| NODES | `Review` |
|
||||
| any other team | `Triage` |
|
||||
|
||||
NODES has a dedicated `Review` lane; every other team handles routing inside their own triage.
|
||||
@@ -1,150 +0,0 @@
|
||||
---
|
||||
description: >-
|
||||
Checks if a community pull request is ready for human review. Verifies CLA
|
||||
signature, PR title format, description completeness, test coverage, and
|
||||
cubic-dev-ai issues. Use when given a PR number or branch name to review,
|
||||
or when the user says /community-pr-review, /pr-review, or asks to check if
|
||||
a PR is ready for review.
|
||||
allowed-tools: Bash(gh:*), Bash(git:*), Read, Glob, Grep
|
||||
---
|
||||
|
||||
# Community PR Review
|
||||
|
||||
Given a PR number or branch name, determine whether it is ready for human review.
|
||||
|
||||
## Steps
|
||||
|
||||
### 1. Resolve the PR
|
||||
|
||||
If given a branch name, find the PR number first:
|
||||
```bash
|
||||
gh pr view <branch> --repo n8n-io/n8n --json number --jq .number
|
||||
```
|
||||
|
||||
### 2. Fetch PR data
|
||||
|
||||
```bash
|
||||
gh pr view <number> --repo n8n-io/n8n \
|
||||
--json number,title,body,author,headRefName,headRefOid,files,isDraft,state
|
||||
```
|
||||
|
||||
Fetch in parallel:
|
||||
|
||||
```bash
|
||||
# CLA commit status (primary signal) — statuses are newest-first; use the first returned entry
|
||||
gh api --paginate "repos/n8n-io/n8n/commits/<headRefOid>/statuses" \
|
||||
--jq '[.[] | select(.context == "license/cla") | {state, description}] | first'
|
||||
|
||||
# CLAassistant issue comment (fallback when no commit status) — use the last returned entry
|
||||
gh api --paginate "repos/n8n-io/n8n/issues/<number>/comments" \
|
||||
--jq '[.[] | select(.user.login == "CLAassistant") | .body] | last'
|
||||
|
||||
# cubic-dev-ai PR review comments (streamed so results concatenate cleanly across pages)
|
||||
gh api --paginate "repos/n8n-io/n8n/pulls/<number>/comments" \
|
||||
--jq '.[] | select(.user.login == "cubic-dev-ai[bot]") | {body: .body, path: .path}'
|
||||
```
|
||||
|
||||
### 3. Run the five checks
|
||||
|
||||
#### A. CLA signed
|
||||
|
||||
Check the `license/cla` commit status first; fall back to the CLAassistant comment if no status exists.
|
||||
|
||||
**Commit status** (`context == "license/cla"`):
|
||||
- `state: "success"` → ✅ signed
|
||||
- `state: "failure"` or `state: "error"` → ❌ not signed
|
||||
- `state: "pending"` → ⏳ pending
|
||||
- Not present → fall back to comment
|
||||
|
||||
**CLAassistant issue comment** (fallback):
|
||||
- Body contains `"All committers have signed the CLA."` → ✅ signed
|
||||
- Body contains `"not signed"` or a link to sign → ❌ not signed
|
||||
- No comment → ❌ treat as not signed
|
||||
|
||||
#### B. PR title format
|
||||
|
||||
For all types except `revert`, the title must match:
|
||||
```
|
||||
^(feat|fix|perf|test|docs|refactor|build|ci|chore)(\([a-zA-Z0-9 ]+( Node)?\))?!?: [A-Z].+[^.]$
|
||||
```
|
||||
|
||||
For `revert` titles, the summary is the original commit header (which starts with a lowercase type), so capitalization is not enforced:
|
||||
```
|
||||
^revert(\([a-zA-Z0-9 ]+( Node)?\))?!?: .+[^.]$
|
||||
```
|
||||
|
||||
- Type must be one of: `feat fix perf test docs refactor build ci chore revert`
|
||||
- Scope is optional, in parentheses e.g. `(editor)` or `(Slack Node)`
|
||||
- Breaking changes: `!` before the colon
|
||||
- Summary: starts with capital letter (lowercase allowed for `revert:`), no trailing period
|
||||
- No Linear ticket IDs in the title (e.g. `N8N-1234`)
|
||||
|
||||
#### C. PR description completeness
|
||||
|
||||
1. **Summary** (`## Summary`) — must have non-empty content below the heading (not just the HTML comment).
|
||||
2. **Related tickets** (`## Related Linear tickets, Github issues, and Community forum posts`) — acceptable content: a URL (`http`), a GitHub closing keyword (`closes #N`, `fixes #N`, `resolves #N`, etc.), or empty. Only flag if the section heading is missing entirely.
|
||||
3. **Checklist** (`## Review / Merge checklist`) — all four items must be present. Unchecked checkboxes are expected for community PRs; do **not** flag them as missing.
|
||||
|
||||
#### D. Tests
|
||||
|
||||
Skip this check if the PR type (from the title) is `docs`, `ci`, `chore`, or `build`.
|
||||
|
||||
Otherwise:
|
||||
1. Identify source files changed: non-test files under `packages/` from the `files` list.
|
||||
2. If there are source file changes, check out the PR in a temporary worktree:
|
||||
|
||||
```bash
|
||||
git fetch origin pull/<number>/head:pr/<number>
|
||||
git worktree add /tmp/pr-<number>-review pr/<number>
|
||||
```
|
||||
|
||||
3. Read the changed source files from the worktree to understand whether the changes introduce logic that warrants tests (new functions, bug fixes, behaviour changes, data transformations). Pure config changes, type-only changes, and trivial renames do not require tests.
|
||||
4. Look for matching test files (`*.test.ts`, `*.spec.ts`, files inside `__tests__/`) among the changed files.
|
||||
5. **Always clean up the worktree**, even if a previous check failed:
|
||||
|
||||
```bash
|
||||
git worktree remove /tmp/pr-<number>-review --force
|
||||
git branch -D pr/<number>
|
||||
```
|
||||
|
||||
Report:
|
||||
- ✅ Tests present, or change does not require tests
|
||||
- ❌ Source logic changed but no test files found
|
||||
|
||||
#### E. cubic-dev-ai issues
|
||||
|
||||
Review the PR review comments fetched in step 2. `cubic-dev-ai[bot]` leaves comments for every issue it finds.
|
||||
|
||||
- No comments from `cubic-dev-ai[bot]`, or every comment explicitly states no issues were found → ✅
|
||||
- Any other comment → ❌ report the total count and priority breakdown (e.g. "3 issues: 1× P1, 1× P2, 1× P3")
|
||||
|
||||
### 4. Output
|
||||
|
||||
Always output valid JSON in this exact shape:
|
||||
|
||||
```json
|
||||
{
|
||||
"readyForReview": <true if all passing checks allow merge, false otherwise>,
|
||||
"messageForUser": "<Human-readable summary of what needs to change, written as if posted directly to the PR contributor. 'N/A' if nothing is needed.>",
|
||||
"checks": {
|
||||
"CLA": <true if signed, false if not signed or pending>,
|
||||
"Title": <true if title matches convention, false otherwise>,
|
||||
"Description": <true if all three template sections are complete, false otherwise>,
|
||||
"TestsNeeded": <true if the code changes require tests, false if not applicable>,
|
||||
"TestsIncluded": <true if test files are present in the PR, false otherwise>,
|
||||
"CubicIssues": <true if cubic-dev-ai raised issues, false if no issues>
|
||||
}
|
||||
}
|
||||
```
|
||||
|
||||
`readyForReview` is `true` only when: `CLA`, `Title`, and `Description` are all `true`; `CubicIssues` is `false`; and either `TestsNeeded` is `false` or `TestsIncluded` is `true`.
|
||||
|
||||
`messageForUser` should be a short, friendly message directed at the contributor listing exactly what they need to address. If `readyForReview` is `true`, set it to `"N/A"`.
|
||||
|
||||
Output nothing other than the JSON block.
|
||||
|
||||
## Notes
|
||||
|
||||
- Draft PRs — report all findings but note the PR is a draft.
|
||||
- If the PR is already merged or closed, say so and skip the checks.
|
||||
- Always remove the worktree even if earlier checks failed.
|
||||
Reference in New Issue
Block a user