diff --git a/agents/pr-agent/AGENT.md b/agents/pr-agent/AGENT.md index 9584077b2a..fa0412de23 100644 --- a/agents/pr-agent/AGENT.md +++ b/agents/pr-agent/AGENT.md @@ -52,8 +52,8 @@ Create and update GitHub pull requests correctly and consistently: deriving PR c ## Implementation Reference - **Folder:** `agents/pr-agent/` -- **Entry points:** each skill's own `skills//.js` -- **Tests:** `__tests__/` (unit), `__tests__/integration/` (cross-skill) +- **Entry points:** each skill's own `skills//scripts/.js` +- **Tests:** `skills//scripts/__tests__/.test.js` (unit), `__tests__/integration/` (cross-skill) --- diff --git a/agents/pr-agent/__tests__/integration/label-application-scenarios.test.js b/agents/pr-agent/__tests__/integration/label-application-scenarios.test.js index 50af503291..39988b8502 100644 --- a/agents/pr-agent/__tests__/integration/label-application-scenarios.test.js +++ b/agents/pr-agent/__tests__/integration/label-application-scenarios.test.js @@ -2,7 +2,7 @@ // Test complex label scenarios import { describe, test, expect, beforeEach } from "@jest/globals"; -import { validateAndApplyLabels } from "../../skills/validate-and-apply-labels/validate-and-apply-labels.js"; +import { validateAndApplyLabels } from "../../skills/validate-and-apply-labels/scripts/validate-and-apply-labels.js"; import { MockGitHub, createMockConfig } from "./setup.js"; describe("Category B: Label Application Scenarios", () => { @@ -14,8 +14,8 @@ describe("Category B: Label Application Scenarios", () => { config = createMockConfig(); }); - test("Test B1: Single Label Application → type:feature only", async () => { - const labels = ["type:feature"]; + test("Test B1: Label Application → type and changelog decision", async () => { + const labels = ["type:feature", "meta:no-changelog"]; const result = await validateAndApplyLabels({ labels, @@ -24,12 +24,12 @@ describe("Category B: Label Application Scenarios", () => { }); expect(result.valid).toBe(true); - expect(result.appliedLabels).toEqual(["type:feature"]); - expect(result.appliedLabels.length).toBe(1); + expect(result.appliedLabels).toEqual(labels); + expect(result.appliedLabels.length).toBe(2); }); test("Test B2: Multiple Labels → type:feature + area:agents", async () => { - const labels = ["type:feature", "area:agents"]; + const labels = ["type:feature", "area:agents", "meta:no-changelog"]; const result = await validateAndApplyLabels({ labels, @@ -39,7 +39,7 @@ describe("Category B: Label Application Scenarios", () => { expect(result.valid).toBe(true); expect(result.appliedLabels).toEqual(labels); - expect(result.appliedLabels.length).toBe(2); + expect(result.appliedLabels.length).toBe(3); }); test("Test B3: Label Conflicts → Resolved per labeling strategy", async () => { @@ -86,7 +86,7 @@ describe("Category B: Label Application Scenarios", () => { test("Test B6: Conditional Labels → Applied based on branch type", async () => { // Branch type determines which labels should be applied const branchType = "fix"; - const conditionalLabels = ["type:bug"]; + const conditionalLabels = ["type:bug", "meta:no-changelog"]; const result = await validateAndApplyLabels({ labels: conditionalLabels, @@ -101,7 +101,12 @@ describe("Category B: Label Application Scenarios", () => { }); test("Test B7: Label Priority → Higher priority labels applied first", async () => { - const labels = ["area:agents", "type:feature", "priority:critical"]; + const labels = [ + "area:agents", + "type:feature", + "priority:critical", + "meta:no-changelog", + ]; const result = await validateAndApplyLabels({ labels, @@ -115,7 +120,12 @@ describe("Category B: Label Application Scenarios", () => { }); test("Test B8: Label Deduplication → Duplicate labels removed", async () => { - const labels = ["type:feature", "type:feature", "area:agents"]; + const labels = [ + "type:feature", + "type:feature", + "area:agents", + "meta:no-changelog", + ]; const result = await validateAndApplyLabels({ labels, @@ -124,8 +134,12 @@ describe("Category B: Label Application Scenarios", () => { }); expect(result.valid).toBe(true); - expect(result.appliedLabels).toEqual(["type:feature", "area:agents"]); - expect(result.appliedLabels.length).toBe(2); + expect(result.appliedLabels).toEqual([ + "type:feature", + "area:agents", + "meta:no-changelog", + ]); + expect(result.appliedLabels.length).toBe(3); expect(result.deduplicatedCount).toBe(1); }); }); diff --git a/agents/pr-agent/__tests__/integration/performance-edge-cases.test.js b/agents/pr-agent/__tests__/integration/performance-edge-cases.test.js index c754eb4633..b80994609b 100644 --- a/agents/pr-agent/__tests__/integration/performance-edge-cases.test.js +++ b/agents/pr-agent/__tests__/integration/performance-edge-cases.test.js @@ -2,10 +2,10 @@ // Test performance and unusual scenarios import { describe, test, expect, beforeEach } from "@jest/globals"; -import { validateBranchName } from "../../skills/validate-branch-name/validate-branch-name.js"; -import { routePrTemplate } from "../../skills/route-pr-template/route-pr-template.js"; -import { validateAndApplyLabels } from "../../skills/validate-and-apply-labels/validate-and-apply-labels.js"; -import { orchestratePrCreation } from "../../skills/orchestrate-pr-creation/orchestrate-pr-creation.js"; +import { validateBranchName } from "../../skills/validate-branch-name/scripts/validate-branch-name.js"; +import { routePrTemplate } from "../../skills/route-pr-template/scripts/route-pr-template.js"; +import { validateAndApplyLabels } from "../../skills/validate-and-apply-labels/scripts/validate-and-apply-labels.js"; +import { orchestratePrCreation } from "../../skills/orchestrate-pr-creation/scripts/orchestrate-pr-creation.js"; import { MockGitHub, createMockConfig } from "./setup.js"; describe("Category F: Performance & Edge Cases", () => { diff --git a/agents/pr-agent/__tests__/integration/real-github-workflows.test.js b/agents/pr-agent/__tests__/integration/real-github-workflows.test.js index 817ca4b322..a757318f71 100644 --- a/agents/pr-agent/__tests__/integration/real-github-workflows.test.js +++ b/agents/pr-agent/__tests__/integration/real-github-workflows.test.js @@ -2,10 +2,10 @@ // Test complete end-to-end workflows import { describe, test, expect, beforeEach } from "@jest/globals"; -import { validateBranchName } from "../../skills/validate-branch-name/validate-branch-name.js"; -import { routePrTemplate } from "../../skills/route-pr-template/route-pr-template.js"; -import { validateAndApplyLabels } from "../../skills/validate-and-apply-labels/validate-and-apply-labels.js"; -import { orchestratePrCreation } from "../../skills/orchestrate-pr-creation/orchestrate-pr-creation.js"; +import { validateBranchName } from "../../skills/validate-branch-name/scripts/validate-branch-name.js"; +import { routePrTemplate } from "../../skills/route-pr-template/scripts/route-pr-template.js"; +import { validateAndApplyLabels } from "../../skills/validate-and-apply-labels/scripts/validate-and-apply-labels.js"; +import { orchestratePrCreation } from "../../skills/orchestrate-pr-creation/scripts/orchestrate-pr-creation.js"; import { MockGitHub, createMockConfig } from "./setup.js"; describe("Category E: Real GitHub Workflows", () => { @@ -19,7 +19,7 @@ describe("Category E: Real GitHub Workflows", () => { test("Test E1: Feature Branch Complete Workflow → All 4 skills succeed", async () => { const branchName = "feat/new-dashboard"; - const labels = ["type:feature"]; + const labels = ["type:feature", "meta:no-changelog"]; // Validate branch const branchValidation = await validateBranchName({ branchName, config }); @@ -59,7 +59,7 @@ describe("Category E: Real GitHub Workflows", () => { test("Test E2: Bug Fix Workflow → Branch validation → bug template → labels → PR", async () => { const branchName = "fix/invalid-validation"; - const labels = ["type:bug", "priority:critical"]; + const labels = ["type:bug", "priority:critical", "meta:needs-changelog"]; const branchValidation = await validateBranchName({ branchName, config }); expect(branchValidation.valid).toBe(true); @@ -78,7 +78,7 @@ describe("Category E: Real GitHub Workflows", () => { test("Test E3: Documentation Update → docs/ → docs template → minimal labels", async () => { const branchName = "docs/branching-guide"; - const labels = ["type:docs"]; + const labels = ["type:docs", "meta:no-changelog"]; const branchValidation = await validateBranchName({ branchName, config }); expect(branchValidation.valid).toBe(true); @@ -92,7 +92,7 @@ describe("Category E: Real GitHub Workflows", () => { mockGitHub: mockGitHub.issues, }); expect(labelValidation.valid).toBe(true); - expect(labelValidation.appliedLabels.length).toBe(1); + expect(labelValidation.appliedLabels.length).toBe(2); }); test("Test E4: Chore/Dependency Update → chore/ → chore template → meta labels", async () => { diff --git a/agents/pr-agent/__tests__/integration/sequential-skill-execution.test.js b/agents/pr-agent/__tests__/integration/sequential-skill-execution.test.js index f96a2d6092..a1d4727032 100644 --- a/agents/pr-agent/__tests__/integration/sequential-skill-execution.test.js +++ b/agents/pr-agent/__tests__/integration/sequential-skill-execution.test.js @@ -2,10 +2,10 @@ // Test skills in order as they execute in real workflows import { describe, test, expect, beforeEach } from "@jest/globals"; -import { validateBranchName } from "../../skills/validate-branch-name/validate-branch-name.js"; -import { routePrTemplate } from "../../skills/route-pr-template/route-pr-template.js"; -import { validateAndApplyLabels } from "../../skills/validate-and-apply-labels/validate-and-apply-labels.js"; -import { orchestratePrCreation } from "../../skills/orchestrate-pr-creation/orchestrate-pr-creation.js"; +import { validateBranchName } from "../../skills/validate-branch-name/scripts/validate-branch-name.js"; +import { routePrTemplate } from "../../skills/route-pr-template/scripts/route-pr-template.js"; +import { validateAndApplyLabels } from "../../skills/validate-and-apply-labels/scripts/validate-and-apply-labels.js"; +import { orchestratePrCreation } from "../../skills/orchestrate-pr-creation/scripts/orchestrate-pr-creation.js"; import { MockGitHub, createMockConfig } from "./setup.js"; describe("Category A: Sequential Skill Execution", () => { @@ -37,7 +37,7 @@ describe("Category A: Sequential Skill Execution", () => { expect(templateRoute.routed).toBe(true); // Step 3: Validate labels - const labels = ["type:feature"]; + const labels = ["type:feature", "meta:no-changelog"]; const labelValidation = await validateAndApplyLabels({ labels, config, @@ -117,7 +117,7 @@ describe("Category A: Sequential Skill Execution", () => { }); test("Test A6: Mixed Label Scenarios → Multiple labels applied correctly", async () => { - const labels = ["type:feature", "area:agents"]; + const labels = ["type:feature", "area:agents", "meta:no-changelog"]; const result = await validateAndApplyLabels({ labels, @@ -146,7 +146,7 @@ describe("Category A: Sequential Skill Execution", () => { test("Test A8: Complete Feature Workflow → feat/ branch full pipeline", async () => { const branchName = "feat/new-feature"; - const labels = ["type:feature"]; + const labels = ["type:feature", "meta:no-changelog"]; // Full workflow validation const branchValidation = await validateBranchName({ diff --git a/agents/pr-agent/__tests__/integration/template-routing-scenarios.test.js b/agents/pr-agent/__tests__/integration/template-routing-scenarios.test.js index c71bb9966d..eb526afb10 100644 --- a/agents/pr-agent/__tests__/integration/template-routing-scenarios.test.js +++ b/agents/pr-agent/__tests__/integration/template-routing-scenarios.test.js @@ -2,7 +2,7 @@ // Test PR template selection for all branch types import { describe, test, expect, beforeEach } from "@jest/globals"; -import { routePrTemplate } from "../../skills/route-pr-template/route-pr-template.js"; +import { routePrTemplate } from "../../skills/route-pr-template/scripts/route-pr-template.js"; import { createMockConfig } from "./setup.js"; describe("Category C: Template Routing Scenarios", () => { diff --git a/agents/pr-agent/skills/handle-pr-errors/SKILL.md b/agents/pr-agent/skills/handle-pr-errors/SKILL.md index 41729838d8..3beded5319 100644 --- a/agents/pr-agent/skills/handle-pr-errors/SKILL.md +++ b/agents/pr-agent/skills/handle-pr-errors/SKILL.md @@ -1,8 +1,66 @@ --- -name: template-skill -description: Replace with description of the skill and when Claude should use it. +name: handle-pr-errors +description: Catches an error from any stage of the PR workflow, categorizes it, assigns a severity, and suggests recovery options and next steps. Use whenever any other pr-agent skill returns or throws an error, to turn it into an actionable recovery response instead of a bare failure. --- -# Insert instructions below +# handle-pr-errors -*Have questions? Ping us on GitHub! 🐙 Made with 💚 by LightSpeedWP* +Handles errors and provides recovery mechanisms for the PR creation workflow. + +## When to use this skill + +Whenever any other skill in this agent (`validate-branch-name`, `route-pr-template`, `orchestrate-pr-creation`, `validate-and-apply-labels`, `submit-pr`) returns or throws an error — to categorise it and suggest a concrete recovery path rather than surfacing a bare failure. + +## Input + +```js +{ + error: object, // { type, message, stage } + context: object, // optional: context from the failed operation + history: array, // optional: prior attempts, for retry logic +} +``` + +## Behaviour + +- Categorises the error (input validation, branch name, template, GitHub API, authentication, rate limit, unknown). +- Assigns a severity (LOW/MEDIUM/HIGH/CRITICAL). +- Determines whether the error is retryable, capping retries at 3 attempts. +- Provides recovery options and ordered next steps. + +## Output + +Handled errors include their classification and recovery plan: + +```js +{ + handled: true, + errorCategory: string, + severity: string, + originalError: string, + recoveryOptions: string[], + recommendedAction: string, + nextSteps: string[], + retryable: boolean, + retryCount: number, + maxRetries: 3, +} +``` + +Invalid input and failures caught while handling an error return the same failure shape: + +```js +{ + handled: false, + error: string, + recoveryOptions: [], +} +``` + +## Usage + +```js +import { handlePrErrors } from './scripts/handle-pr-errors.js'; + +const result = await handlePrErrors({ error: { message: 'Rate limit exceeded' } }); +``` diff --git a/agents/pr-agent/__tests__/submit-pr-and-error-handling.test.js b/agents/pr-agent/skills/handle-pr-errors/scripts/__tests__/handle-pr-errors.test.js similarity index 50% rename from agents/pr-agent/__tests__/submit-pr-and-error-handling.test.js rename to agents/pr-agent/skills/handle-pr-errors/scripts/__tests__/handle-pr-errors.test.js index ca64268387..dd68ca9444 100644 --- a/agents/pr-agent/__tests__/submit-pr-and-error-handling.test.js +++ b/agents/pr-agent/skills/handle-pr-errors/scripts/__tests__/handle-pr-errors.test.js @@ -1,241 +1,4 @@ -import { submitPr } from "../skills/submit-pr/submit-pr.js"; -import { handlePrErrors } from "../skills/handle-pr-errors/handle-pr-errors.js"; - -describe("submitPr (Skill 5)", () => { - const validPr = { - title: "feat: User Authentication — Implementation", - body: "This PR implements user authentication system with OAuth2 support.", - head: "feat/user-auth", - base: "develop", - labels: ["type:feature", "area:security"], - draft: false, - }; - - const validContext = { - owner: "lightspeedwp", - repo: ".github", - token: "gh_test_token", - }; - - describe("Input Validation", () => { - test("should return error for missing PR object", async () => { - const result = await submitPr({ githubContext: validContext }); - - expect(result.valid).toBe(false); - expect(result.error).toContain("PR object"); - expect(result.submitted).toBe(false); - }); - - test("should return error for missing PR fields", async () => { - const incompletePr = { - title: "Test PR", - // missing body, head, base, labels - }; - - const result = await submitPr({ - pr: incompletePr, - githubContext: validContext, - }); - - expect(result.valid).toBe(false); - expect(result.error).toContain("missing required fields"); - expect(result.missingFields).toBeDefined(); - expect(result.missingFields.length).toBeGreaterThan(0); - }); - - test("should validate PR before submission", async () => { - const invalidPr = { - title: "", - body: "Short", - head: "feat/test", - base: "develop", - labels: [], - }; - - const result = await submitPr({ - pr: invalidPr, - githubContext: validContext, - }); - - expect(result.valid).toBe(false); - expect(result.submitted).toBe(false); - expect(result.validationErrors).toBeDefined(); - }); - }); - - describe("Dry-Run Mode", () => { - test("should validate without submitting in dry-run", async () => { - const result = await submitPr({ - pr: validPr, - githubContext: validContext, - dryRun: true, - }); - - expect(result.valid).toBe(true); - expect(result.dryRun).toBe(true); - expect(result.submitted).toBe(false); - expect(result.prUrl).toBeNull(); - expect(result.message).toContain("not submitted"); - }); - - test("should show PR preview in dry-run", async () => { - const result = await submitPr({ - pr: validPr, - githubContext: validContext, - dryRun: true, - }); - - expect(result.valid).toBe(true); - expect(result.prPreview).toBeDefined(); - expect(result.prPreview.title).toBe(validPr.title); - expect(result.prPreview.head).toBe(validPr.head); - expect(result.prPreview.base).toBe(validPr.base); - }); - }); - - describe("PR Validation", () => { - test("should validate title presence", async () => { - const prNoTitle = { ...validPr, title: "" }; - const result = await submitPr({ pr: prNoTitle, dryRun: true }); - - expect(result.valid).toBe(false); - expect(result.validationErrors.some((e) => e.includes("title"))).toBe( - true, - ); - }); - - test("should validate body minimum length", async () => { - const prShortBody = { ...validPr, body: "Too" }; - const result = await submitPr({ pr: prShortBody, dryRun: true }); - - expect(result.valid).toBe(false); - expect(result.validationErrors.some((e) => e.includes("too short"))).toBe( - true, - ); - }); - - test("should validate branch fields", async () => { - const prNoBranches = { ...validPr, head: "", base: "" }; - const result = await submitPr({ pr: prNoBranches, dryRun: true }); - - expect(result.valid).toBe(false); - }); - - test("should validate labels array", async () => { - const prInvalidLabels = { ...validPr, labels: "invalid" }; - const result = await submitPr({ pr: prInvalidLabels, dryRun: true }); - - expect(result.valid).toBe(false); - }); - - test("should warn about missing labels", async () => { - const prNoLabels = { ...validPr, labels: [] }; - const result = await submitPr({ - pr: prNoLabels, - dryRun: true, - }); - - expect(result.valid).toBe(true); - expect(result.warnings.some((w) => w.includes("No labels"))).toBe(true); - }); - - test("should reject bare labels without prefix", async () => { - const prBareLabels = { ...validPr, labels: ["feature", "bug"] }; - const result = await submitPr({ - pr: prBareLabels, - dryRun: true, - }); - - expect(result.valid).toBe(false); - expect(result.validationErrors).toBeDefined(); - expect( - result.validationErrors.some((e) => e.includes("Invalid label format")), - ).toBe(true); - }); - }); - - describe("GitHub Context Validation", () => { - test("should require owner in GitHub context", async () => { - const result = await submitPr({ - pr: validPr, - githubContext: { repo: ".github" }, - }); - - expect(result.valid).toBe(false); - expect(result.submitted).toBe(false); - }); - - test("should require repo in GitHub context", async () => { - const result = await submitPr({ - pr: validPr, - githubContext: { owner: "lightspeedwp" }, - }); - - expect(result.valid).toBe(false); - expect(result.submitted).toBe(false); - }); - - test("should use provided GitHub context", async () => { - const result = await submitPr({ - pr: validPr, - githubContext: validContext, - }); - - expect(result.valid).toBe(true); - expect(result.submitted).toBe(true); - expect(result.prUrl).toContain("lightspeedwp"); - expect(result.prUrl).toContain(".github"); - }); - }); - - describe("PR Submission", () => { - test("should return PR URL on successful submission", async () => { - const result = await submitPr({ - pr: validPr, - githubContext: validContext, - }); - - expect(result.valid).toBe(true); - expect(result.submitted).toBe(true); - expect(result.prUrl).toBeDefined(); - expect(result.prUrl).toContain("github.com"); - expect(result.prUrl).toContain("pull"); - }); - - test("should return PR number on successful submission", async () => { - const result = await submitPr({ - pr: validPr, - githubContext: validContext, - }); - - expect(result.valid).toBe(true); - expect(result.prNumber).toBeDefined(); - expect(typeof result.prNumber).toBe("number"); - expect(result.prNumber).toBeGreaterThan(0); - }); - - test("should return PR ID on successful submission", async () => { - const result = await submitPr({ - pr: validPr, - githubContext: validContext, - }); - - expect(result.valid).toBe(true); - expect(result.prId).toBeDefined(); - expect(typeof result.prId).toBe("string"); - }); - - test("should preserve labels in submission", async () => { - const result = await submitPr({ - pr: validPr, - githubContext: validContext, - }); - - expect(result.valid).toBe(true); - expect(result.labels).toEqual(validPr.labels); - }); - }); -}); +import { handlePrErrors } from "../handle-pr-errors.js"; describe("handlePrErrors (Skill 6)", () => { describe("Input Validation", () => { diff --git a/agents/pr-agent/skills/handle-pr-errors/handle-pr-errors.js b/agents/pr-agent/skills/handle-pr-errors/scripts/handle-pr-errors.js similarity index 100% rename from agents/pr-agent/skills/handle-pr-errors/handle-pr-errors.js rename to agents/pr-agent/skills/handle-pr-errors/scripts/handle-pr-errors.js diff --git a/agents/pr-agent/skills/orchestrate-pr-creation/SKILL.md b/agents/pr-agent/skills/orchestrate-pr-creation/SKILL.md index 842c5288d0..1bc09eda0e 100644 --- a/agents/pr-agent/skills/orchestrate-pr-creation/SKILL.md +++ b/agents/pr-agent/skills/orchestrate-pr-creation/SKILL.md @@ -1,8 +1,59 @@ --- -name: template-skill -description: Replace with description of the skill and when Claude should use it. +name: orchestrate-pr-creation +description: Assembles and validates the complete PR data object (title, body, head, base, labels) derived from the branch's own commits and diff, before it is submitted. Use after branch name validation and template routing, once the PR's content is ready to be assembled. --- -# Insert instructions below +# orchestrate-pr-creation -_Docs signed by 🤖 Copilot for LightSpeedWP – always fresh!_ +Orchestrates PR creation by assembling PR data and validating it. + +## When to use this skill + +After `validate-branch-name` and `route-pr-template`, once the PR's title, body, head, base and labels are ready to be assembled into a single PR object for `submit-pr`. + +## Input + +```js +{ + pr: object, // { owner, repo, title, body, head, base, labels } + aiFeedback: array, // optional: AI review feedback + triggerWorkflow: boolean, // optional + createFeedbackResponse: boolean, // optional + parseFrontmatter: boolean, // optional +} +``` + +## Behaviour + +- Derives PR content from the branch's own commits/diff, not assumed prior context. +- Validates the assembled PR object before it is handed to `submit-pr`. + +## Output + +Failures return a single error: + +```js +{ success: false, error: string } +``` + +Successes return the assembled PR and orchestration metadata: + +```js +{ + success: true, + pr: object, + frontmatter: object | null, + feedbackResponseCreated: boolean, + workflowRequested: boolean, +} +``` + +## Usage + +```js +import { orchestratePrCreation } from "./scripts/orchestrate-pr-creation.js"; + +const result = await orchestratePrCreation({ + pr: { owner, repo, title, body, head, base, labels }, +}); +``` diff --git a/agents/pr-agent/__tests__/orchestrate-pr-creation.test.js b/agents/pr-agent/skills/orchestrate-pr-creation/scripts/__tests__/orchestrate-pr-creation.test.js similarity index 97% rename from agents/pr-agent/__tests__/orchestrate-pr-creation.test.js rename to agents/pr-agent/skills/orchestrate-pr-creation/scripts/__tests__/orchestrate-pr-creation.test.js index 8cc5c95063..b17188e393 100644 --- a/agents/pr-agent/__tests__/orchestrate-pr-creation.test.js +++ b/agents/pr-agent/skills/orchestrate-pr-creation/scripts/__tests__/orchestrate-pr-creation.test.js @@ -1,4 +1,4 @@ -import { orchestratePrCreation } from "../skills/orchestrate-pr-creation/orchestrate-pr-creation.js"; +import { orchestratePrCreation } from "../orchestrate-pr-creation.js"; describe("orchestratePrCreation", () => { const validPr = { diff --git a/agents/pr-agent/skills/orchestrate-pr-creation/orchestrate-pr-creation.js b/agents/pr-agent/skills/orchestrate-pr-creation/scripts/orchestrate-pr-creation.js similarity index 100% rename from agents/pr-agent/skills/orchestrate-pr-creation/orchestrate-pr-creation.js rename to agents/pr-agent/skills/orchestrate-pr-creation/scripts/orchestrate-pr-creation.js diff --git a/agents/pr-agent/skills/route-pr-template/SKILL.md b/agents/pr-agent/skills/route-pr-template/SKILL.md index 41729838d8..f8c3631457 100644 --- a/agents/pr-agent/skills/route-pr-template/SKILL.md +++ b/agents/pr-agent/skills/route-pr-template/SKILL.md @@ -1,8 +1,61 @@ --- -name: template-skill -description: Replace with description of the skill and when Claude should use it. +name: route-pr-template +description: Selects the correct PR template for a branch based on its type using the static branch-type routing map or an explicit user override. Use once the branch has been validated and before assembling the PR body, to know which template structure to follow. --- -# Insert instructions below +# route-pr-template -*Have questions? Ping us on GitHub! 🐙 Made with 💚 by LightSpeedWP* +Routes a pull request to the correct template based on branch type. + +## When to use this skill + +After `validate-branch-name` confirms the branch name, and before `orchestrate-pr-creation` assembles the PR body — to determine which template structure the PR body must follow. + +## Input + +```js +{ + branchName: string, // full branch name, e.g. "feat/new-feature" + branchType: string, // alternative: just the type, e.g. "feat" + userSelectedTemplate: string, // optional: explicit user override +} +``` + +## Behaviour + +- Matches the branch type against the static `BRANCH_TYPE_ROUTING` map. +- Uses `userSelectedTemplate` when provided. + +## Output + +Matched routes and user overrides return: + +```js +{ + routed: true, + template: string, + reason: string, + fallback: false, + userOverride?: true, +} +``` + +Invalid input and unknown branch types use the fallback template and include a warning: + +```js +{ + routed: false, + template: "pr_feature.md", + reason: "invalid-input" | "unknown-branch-type", + fallback: true, + warning: string, +} +``` + +## Usage + +```js +import { routePrTemplate } from './scripts/route-pr-template.js'; + +const result = await routePrTemplate({ branchName: 'feat/new-feature' }); +``` diff --git a/agents/pr-agent/__tests__/route-pr-template.test.js b/agents/pr-agent/skills/route-pr-template/scripts/__tests__/route-pr-template.test.js similarity index 99% rename from agents/pr-agent/__tests__/route-pr-template.test.js rename to agents/pr-agent/skills/route-pr-template/scripts/__tests__/route-pr-template.test.js index 3355ac3982..bcb8a27c54 100644 --- a/agents/pr-agent/__tests__/route-pr-template.test.js +++ b/agents/pr-agent/skills/route-pr-template/scripts/__tests__/route-pr-template.test.js @@ -1,4 +1,4 @@ -import { routePrTemplate } from "../skills/route-pr-template/route-pr-template.js"; +import { routePrTemplate } from "../route-pr-template.js"; describe("routePrTemplate", () => { test("returns user override when provided", async () => { diff --git a/agents/pr-agent/skills/route-pr-template/route-pr-template.js b/agents/pr-agent/skills/route-pr-template/scripts/route-pr-template.js similarity index 100% rename from agents/pr-agent/skills/route-pr-template/route-pr-template.js rename to agents/pr-agent/skills/route-pr-template/scripts/route-pr-template.js diff --git a/agents/pr-agent/skills/submit-pr/SKILL.md b/agents/pr-agent/skills/submit-pr/SKILL.md index 842c5288d0..20abe4a2fe 100644 --- a/agents/pr-agent/skills/submit-pr/SKILL.md +++ b/agents/pr-agent/skills/submit-pr/SKILL.md @@ -1,8 +1,100 @@ --- -name: template-skill -description: Replace with description of the skill and when Claude should use it. +name: submit-pr +description: Submits the orchestrated PR object to GitHub, or validates it without creating anything in dry-run mode. Use as the final step, once orchestrate-pr-creation and validate-and-apply-labels have produced a complete, valid PR object. --- -# Insert instructions below +# submit-pr -_Docs signed by 🤖 Copilot for LightSpeedWP – always fresh!_ +Submits a PR to GitHub using the orchestrated PR object. + +## When to use this skill + +As the final step, once `orchestrate-pr-creation` and `validate-and-apply-labels` have produced a complete PR object (title, body, head, base, labels). + +## Input + +```js +{ + pr: object, // { title, body, head, base, labels } + githubContext: object, // { owner, repo, token } + dryRun: boolean, // optional, default false — validate only, don't create +} +``` + +## Behaviour + +- Validates the PR object (title, body length, branch fields, label format) before submitting. +- In dry-run mode, returns a preview without creating anything on GitHub. +- Otherwise creates the PR and returns its URL, number, and ID. + +## Output + +PR validation failures return: + +```js +{ + valid: false, + error: "PR validation failed", + validationErrors: string[], + warnings: string[], + submitted: false, + prUrl: null, +} +``` + +A successful dry run returns a preview without submission identifiers: + +```js +{ + valid: true, + dryRun: true, + submitted: false, + prUrl: null, + prPreview: { + title: string, + body: string, + head: string, + base: string, + labels: string[], + }, + warnings: string[], + message: string, +} +``` + +A successful submission returns the GitHub identifiers and applied labels: + +```js +{ + valid: true, + submitted: true, + prUrl: string, + prNumber: number, + prId: string, + message: string, + labels: string[], + warnings: string[], +} +``` + +Invalid input, GitHub submission failures, and caught errors return an error variant. Missing-field input errors add +`missingFields`; GitHub submission errors add `details`: + +```js +{ + valid: false, + error: string, + submitted: false, + prUrl: null, + missingFields?: string[], + details?: { missingFields: string[] }, +} +``` + +## Usage + +```js +import { submitPr } from './scripts/submit-pr.js'; + +const result = await submitPr({ pr, githubContext, dryRun: true }); +``` diff --git a/agents/pr-agent/skills/submit-pr/scripts/__tests__/submit-pr.test.js b/agents/pr-agent/skills/submit-pr/scripts/__tests__/submit-pr.test.js new file mode 100644 index 0000000000..e0a2f131f8 --- /dev/null +++ b/agents/pr-agent/skills/submit-pr/scripts/__tests__/submit-pr.test.js @@ -0,0 +1,237 @@ +import { submitPr } from "../submit-pr.js"; + +describe("submitPr (Skill 5)", () => { + const validPr = { + title: "feat: User Authentication — Implementation", + body: "This PR implements user authentication system with OAuth2 support.", + head: "feat/user-auth", + base: "develop", + labels: ["type:feature", "area:security"], + draft: false, + }; + + const validContext = { + owner: "lightspeedwp", + repo: ".github", + token: "gh_test_token", + }; + + describe("Input Validation", () => { + test("should return error for missing PR object", async () => { + const result = await submitPr({ githubContext: validContext }); + + expect(result.valid).toBe(false); + expect(result.error).toContain("PR object"); + expect(result.submitted).toBe(false); + }); + + test("should return error for missing PR fields", async () => { + const incompletePr = { + title: "Test PR", + // missing body, head, base, labels + }; + + const result = await submitPr({ + pr: incompletePr, + githubContext: validContext, + }); + + expect(result.valid).toBe(false); + expect(result.error).toContain("missing required fields"); + expect(result.missingFields).toBeDefined(); + expect(result.missingFields.length).toBeGreaterThan(0); + }); + + test("should validate PR before submission", async () => { + const invalidPr = { + title: "", + body: "Short", + head: "feat/test", + base: "develop", + labels: [], + }; + + const result = await submitPr({ + pr: invalidPr, + githubContext: validContext, + }); + + expect(result.valid).toBe(false); + expect(result.submitted).toBe(false); + expect(result.validationErrors).toBeDefined(); + }); + }); + + describe("Dry-Run Mode", () => { + test("should validate without submitting in dry-run", async () => { + const result = await submitPr({ + pr: validPr, + githubContext: validContext, + dryRun: true, + }); + + expect(result.valid).toBe(true); + expect(result.dryRun).toBe(true); + expect(result.submitted).toBe(false); + expect(result.prUrl).toBeNull(); + expect(result.message).toContain("not submitted"); + }); + + test("should show PR preview in dry-run", async () => { + const result = await submitPr({ + pr: validPr, + githubContext: validContext, + dryRun: true, + }); + + expect(result.valid).toBe(true); + expect(result.prPreview).toBeDefined(); + expect(result.prPreview.title).toBe(validPr.title); + expect(result.prPreview.head).toBe(validPr.head); + expect(result.prPreview.base).toBe(validPr.base); + }); + }); + + describe("PR Validation", () => { + test("should validate title presence", async () => { + const prNoTitle = { ...validPr, title: "" }; + const result = await submitPr({ pr: prNoTitle, dryRun: true }); + + expect(result.valid).toBe(false); + expect(result.validationErrors.some((e) => e.includes("title"))).toBe( + true, + ); + }); + + test("should validate body minimum length", async () => { + const prShortBody = { ...validPr, body: "Too" }; + const result = await submitPr({ pr: prShortBody, dryRun: true }); + + expect(result.valid).toBe(false); + expect(result.validationErrors.some((e) => e.includes("too short"))).toBe( + true, + ); + }); + + test("should validate branch fields", async () => { + const prNoBranches = { ...validPr, head: "", base: "" }; + const result = await submitPr({ pr: prNoBranches, dryRun: true }); + + expect(result.valid).toBe(false); + }); + + test("should validate labels array", async () => { + const prInvalidLabels = { ...validPr, labels: "invalid" }; + const result = await submitPr({ pr: prInvalidLabels, dryRun: true }); + + expect(result.valid).toBe(false); + }); + + test("should warn about missing labels", async () => { + const prNoLabels = { ...validPr, labels: [] }; + const result = await submitPr({ + pr: prNoLabels, + dryRun: true, + }); + + expect(result.valid).toBe(true); + expect(result.warnings.some((w) => w.includes("No labels"))).toBe(true); + }); + + test("should reject bare labels without prefix", async () => { + const prBareLabels = { ...validPr, labels: ["feature", "bug"] }; + const result = await submitPr({ + pr: prBareLabels, + dryRun: true, + }); + + expect(result.valid).toBe(false); + expect(result.validationErrors).toBeDefined(); + expect( + result.validationErrors.some((e) => e.includes("Invalid label format")), + ).toBe(true); + }); + }); + + describe("GitHub Context Validation", () => { + test("should require owner in GitHub context", async () => { + const result = await submitPr({ + pr: validPr, + githubContext: { repo: ".github" }, + }); + + expect(result.valid).toBe(false); + expect(result.submitted).toBe(false); + }); + + test("should require repo in GitHub context", async () => { + const result = await submitPr({ + pr: validPr, + githubContext: { owner: "lightspeedwp" }, + }); + + expect(result.valid).toBe(false); + expect(result.submitted).toBe(false); + }); + + test("should use provided GitHub context", async () => { + const result = await submitPr({ + pr: validPr, + githubContext: validContext, + }); + + expect(result.valid).toBe(true); + expect(result.submitted).toBe(true); + expect(result.prUrl).toContain("lightspeedwp"); + expect(result.prUrl).toContain(".github"); + }); + }); + + describe("PR Submission", () => { + test("should return PR URL on successful submission", async () => { + const result = await submitPr({ + pr: validPr, + githubContext: validContext, + }); + + expect(result.valid).toBe(true); + expect(result.submitted).toBe(true); + expect(result.prUrl).toBeDefined(); + expect(result.prUrl).toContain("github.com"); + expect(result.prUrl).toContain("pull"); + }); + + test("should return PR number on successful submission", async () => { + const result = await submitPr({ + pr: validPr, + githubContext: validContext, + }); + + expect(result.valid).toBe(true); + expect(result.prNumber).toBeDefined(); + expect(typeof result.prNumber).toBe("number"); + expect(result.prNumber).toBeGreaterThan(0); + }); + + test("should return PR ID on successful submission", async () => { + const result = await submitPr({ + pr: validPr, + githubContext: validContext, + }); + + expect(result.valid).toBe(true); + expect(result.prId).toBeDefined(); + expect(typeof result.prId).toBe("string"); + }); + + test("should preserve labels in submission", async () => { + const result = await submitPr({ + pr: validPr, + githubContext: validContext, + }); + + expect(result.valid).toBe(true); + expect(result.labels).toEqual(validPr.labels); + }); + }); +}); diff --git a/agents/pr-agent/skills/submit-pr/submit-pr.js b/agents/pr-agent/skills/submit-pr/scripts/submit-pr.js similarity index 100% rename from agents/pr-agent/skills/submit-pr/submit-pr.js rename to agents/pr-agent/skills/submit-pr/scripts/submit-pr.js diff --git a/agents/pr-agent/skills/validate-and-apply-labels/SKILL.md b/agents/pr-agent/skills/validate-and-apply-labels/SKILL.md index 831219ee38..88b3ad5548 100644 --- a/agents/pr-agent/skills/validate-and-apply-labels/SKILL.md +++ b/agents/pr-agent/skills/validate-and-apply-labels/SKILL.md @@ -1,9 +1,91 @@ --- -name: template-skill -description: Replace with description of the skill and when Claude should use it. +name: validate-and-apply-labels +description: Validates labels against the repository's real canonical label set and maps branch type to its required labels, including exactly one changelog-decision label. Use before submission, to make sure every label on a PR is valid and nothing required (like the changelog-decision label) is missing. --- -# Insert instructions below +# validate-and-apply-labels -_This page brought to you by the 🦄 Magic Automation Unicorns of LightSpeedWP._ -[Automation Docs](https://github.com/lightspeedwp/.github/tree/main/instructions) +Validates GitHub labels against the canonical set and maps branch type to labels. + +## When to use this skill + +Before `submit-pr`, to confirm every label attached to the PR is real (exists in this repository's canonical set) and that branch-type-required labels — including exactly one changelog-decision label — are present. + +## Input + +```js +{ + labels: string[], // e.g. ["type:feature", "area:agents"] + branchType: string, // optional: for conditional labels + templateFile: string, // optional: PR template file + config: object, // optional + mockGitHub: object, // optional: for testing +} +``` + +## Behaviour + +- Rejects a bare label without its required family prefix (e.g. `bug` instead of `type:bug`). +- Rejects a label not present in the repository's real canonical label set — never invents one. +- Ensures exactly one changelog-decision label is present, never zero or two. + +## Output + +Branch-type mapping returns the applied labels and any mapping validation errors: + +```js +{ + valid: boolean, + appliedLabels: string[], + branchType: string, + templateFile: string, + validationErrors: string[], + warnings: string[], + deduplicatedCount: number, + metadata: { + typeLabels: string[], + contextLabels: string[], + totalLabels: number, + }, +} +``` + +Invalid branch-type mapping input omits `templateFile`, deduplication data, and metadata: + +```js +{ + valid: false, + error: string, + appliedLabels: [], + branchType: unknown, + validationErrors: [], + warnings: [], +} +``` + +Direct label validation returns label-specific errors and may identify invalid or conflicting labels: + +```js +{ + valid: boolean, + appliedLabels: string[], + errors: string[] | undefined, + deduplicatedCount: number, + validationErrors: string[], + warnings: string[], + invalidLabels?: unknown[], + conflicts?: Array<{ family: string, labels: string[] }>, +} +``` + +## Usage + +```js +import { validateAndApplyLabels } from './scripts/validate-and-apply-labels.js'; + +const result = await validateAndApplyLabels({ + labels: ['meta:no-changelog'], + branchType: 'feat', + templateFile: 'pr_feature.md', +}); +``` diff --git a/agents/pr-agent/__tests__/validate-and-apply-labels.test.js b/agents/pr-agent/skills/validate-and-apply-labels/scripts/__tests__/validate-and-apply-labels.test.js similarity index 85% rename from agents/pr-agent/__tests__/validate-and-apply-labels.test.js rename to agents/pr-agent/skills/validate-and-apply-labels/scripts/__tests__/validate-and-apply-labels.test.js index 1d446e05ce..1682169e3f 100644 --- a/agents/pr-agent/__tests__/validate-and-apply-labels.test.js +++ b/agents/pr-agent/skills/validate-and-apply-labels/scripts/__tests__/validate-and-apply-labels.test.js @@ -1,5 +1,5 @@ import { jest } from "@jest/globals"; -import { validateAndApplyLabels } from "../skills/validate-and-apply-labels/validate-and-apply-labels.js"; +import { validateAndApplyLabels } from "../validate-and-apply-labels.js"; describe("validateAndApplyLabels", () => { beforeEach(() => { @@ -48,11 +48,52 @@ describe("validateAndApplyLabels", () => { }); }); + describe("Direct Label Validation", () => { + test("should accept canonical labels with one changelog decision", async () => { + const result = await validateAndApplyLabels({ + labels: ["type:feature", "meta:no-changelog"], + }); + + expect(result.valid).toBe(true); + expect(result.appliedLabels).toEqual([ + "type:feature", + "meta:no-changelog", + ]); + }); + + test("should reject a format-matching non-canonical label", async () => { + const result = await validateAndApplyLabels({ + labels: ["type:feature", "custom:value", "meta:no-changelog"], + }); + + expect(result.valid).toBe(false); + expect(result.errors).toContain("non-canonical-label"); + expect(result.invalidLabels).toContain("custom:value"); + }); + + test("should reject labels without a changelog decision", async () => { + const result = await validateAndApplyLabels({ labels: ["type:feature"] }); + + expect(result.valid).toBe(false); + expect(result.errors).toContain("missing-changelog-decision-label"); + }); + + test("should reject multiple changelog decisions", async () => { + const result = await validateAndApplyLabels({ + labels: ["type:feature", "meta:needs-changelog", "meta:no-changelog"], + }); + + expect(result.valid).toBe(false); + expect(result.errors).toContain("multiple-changelog-decision-labels"); + }); + }); + describe("Branch Type Label Mapping", () => { test("should map feat to type:feature", async () => { const result = await validateAndApplyLabels({ branchType: "feat", templateFile: "pr_feature.md", + labels: ["meta:no-changelog"], }); expect(result.valid).toBe(true); @@ -63,6 +104,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "fix", templateFile: "pr_bug.md", + labels: ["meta:no-changelog"], }); expect(result.valid).toBe(true); @@ -73,6 +115,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "docs", templateFile: "pr_docs.md", + labels: ["meta:no-changelog"], }); expect(result.valid).toBe(true); @@ -83,6 +126,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "hotfix", templateFile: "pr_hotfix.md", + labels: ["meta:no-changelog"], }); expect(result.valid).toBe(true); @@ -94,6 +138,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "security", templateFile: "pr_bug.md", + labels: ["meta:no-changelog"], }); expect(result.valid).toBe(true); @@ -104,6 +149,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "ci", templateFile: "pr_ci.md", + labels: ["meta:no-changelog"], }); expect(result.valid).toBe(true); @@ -114,6 +160,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "test", templateFile: "pr_chore.md", + labels: ["meta:no-changelog"], }); expect(result.valid).toBe(true); @@ -124,6 +171,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "a11y", templateFile: "pr_feature.md", + labels: ["meta:no-changelog"], }); expect(result.valid).toBe(true); @@ -169,6 +217,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType, templateFile: "pr_feature.md", + labels: ["meta:no-changelog"], }); expect(result.valid).toBe(true); @@ -182,6 +231,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "feat", templateFile: "pr_feature.md", + labels: ["meta:no-changelog"], templateMetadata: { missingSections: ["Changelog", "Checklist"], complete: false, @@ -196,6 +246,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "feat", templateFile: "pr_feature.md", + labels: ["meta:no-changelog"], templateMetadata: { missingSections: [], complete: true, @@ -210,6 +261,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "feat", templateFile: "pr_feature.md", + labels: ["meta:no-changelog"], templateMetadata: null, }); @@ -222,6 +274,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "feat", templateFile: "pr_feature.md", + labels: ["meta:no-changelog"], templateMetadata: { missingSections: ["Changelog"], complete: false, @@ -239,6 +292,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "feat", templateFile: "pr_feature.md", + labels: ["meta:no-changelog"], }); expect(result.valid).toBe(true); @@ -281,6 +335,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "feat", templateFile: "pr_feature.md", + labels: ["meta:no-changelog"], config: { branchTypeLabels: { feat: ["type:feature", "type:feature"] }, }, @@ -299,6 +354,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "feat", templateFile: "pr_feature.md", + labels: ["meta:no-changelog"], config: { branchTypeLabels: { feat: ["type:feature", "area:docs"], @@ -308,6 +364,7 @@ describe("validateAndApplyLabels", () => { "type:bug", "area:docs", "meta:ready-for-review", + "meta:no-changelog", ], }, }); @@ -318,10 +375,11 @@ describe("validateAndApplyLabels", () => { }); test("should use custom canonical labels when provided", async () => { - const customLabels = ["custom:label1", "custom:label2"]; + const customLabels = ["custom:label1", "custom:label2", "meta:no-changelog"]; const result = await validateAndApplyLabels({ branchType: "feat", templateFile: "pr_feature.md", + labels: ["meta:no-changelog"], config: { branchTypeLabels: { feat: ["custom:label1"], @@ -340,9 +398,10 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "feat", templateFile: "pr_feature.md", + labels: ["meta:no-changelog"], config: { branchTypeLabels: { feat: ["type:feature", "wp:plugin"] }, - canonicalLabels: ["type:feature", "wp:plugin", "wp:theme"], + canonicalLabels: ["type:feature", "wp:plugin", "wp:theme", "meta:no-changelog"], }, }); @@ -388,6 +447,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "feat", templateFile: "pr_feature.md", + labels: ["meta:no-changelog"], templateMetadata: { missingSections: [], complete: true, @@ -404,6 +464,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "feat", templateFile: "pr_feature.md", + labels: ["meta:no-changelog"], templateMetadata: { missingSections: [], complete: true, @@ -425,6 +486,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "security", templateFile: "pr_bug.md", + labels: ["meta:no-changelog"], templateMetadata: { missingSections: ["Changelog"], complete: false, @@ -443,6 +505,7 @@ describe("validateAndApplyLabels", () => { "type:bug", "type:security", "meta:needs-more-info", + "meta:no-changelog", ], }, }); @@ -459,6 +522,7 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "feat", templateFile: "pr_feature.md", + labels: ["meta:no-changelog"], templateMetadata: {}, }); @@ -499,11 +563,12 @@ describe("validateAndApplyLabels", () => { const result = await validateAndApplyLabels({ branchType: "feat", templateFile: "pr_feature.md", + labels: ["meta:no-changelog"], config: { branchTypeLabels: { feat: ["type:feature", "area:docs/api"], }, - canonicalLabels: ["type:feature", "area:docs/api"], + canonicalLabels: ["type:feature", "area:docs/api", "meta:no-changelog"], }, }); diff --git a/agents/pr-agent/skills/validate-and-apply-labels/validate-and-apply-labels.js b/agents/pr-agent/skills/validate-and-apply-labels/scripts/validate-and-apply-labels.js similarity index 82% rename from agents/pr-agent/skills/validate-and-apply-labels/validate-and-apply-labels.js rename to agents/pr-agent/skills/validate-and-apply-labels/scripts/validate-and-apply-labels.js index c8c6dcc50a..230a5e415a 100644 --- a/agents/pr-agent/skills/validate-and-apply-labels/validate-and-apply-labels.js +++ b/agents/pr-agent/skills/validate-and-apply-labels/scripts/validate-and-apply-labels.js @@ -35,8 +35,12 @@ const CANONICAL_LABELS = { "area:a11y": 2, "meta:needs-more-info": 3, "meta:ready-for-review": 3, + "meta:needs-changelog": 3, + "meta:no-changelog": 3, }; +const CHANGELOG_DECISION_LABELS = ["meta:needs-changelog", "meta:no-changelog"]; + // Default branch type to label mapping (30+ branch types) const BRANCH_TYPE_LABELS = { feat: ["type:feature"], @@ -146,10 +150,16 @@ export async function validateAndApplyLabels(input) { mappedLabels.push("meta:ready-for-review"); } + // Combine caller-supplied labels (e.g. the changelog-decision label) with + // the branch-type-mapped labels before validating -- a branch-mapped call + // must still carry exactly one changelog-decision label (SKILL.md's + // documented contract), which the mapping alone never provides. + const combinedLabels = [...mappedLabels, ...labels]; + // Deduplicate labels const seenLabels = new Set(); const deduplicatedLabels = []; - for (const label of mappedLabels) { + for (const label of combinedLabels) { if (!seenLabels.has(label)) { deduplicatedLabels.push(label); seenLabels.add(label); @@ -188,6 +198,21 @@ export async function validateAndApplyLabels(input) { } } + // A branch-mapped call must carry exactly one changelog-decision label, + // same as a direct label-validation call. + const changelogDecisionLabels = deduplicatedLabels.filter((label) => + CHANGELOG_DECISION_LABELS.includes(label), + ); + if (changelogDecisionLabels.length === 0) { + validationErrors.push( + "Exactly one changelog-decision label (meta:needs-changelog or meta:no-changelog) is required", + ); + } else if (changelogDecisionLabels.length > 1) { + validationErrors.push( + "Only one changelog-decision label is allowed, found multiple", + ); + } + const isValid = validationErrors.length === 0 && mappedLabels.length > 0; // Separate context labels from type labels @@ -203,7 +228,7 @@ export async function validateAndApplyLabels(input) { templateFile, validationErrors, warnings, - deduplicatedCount: mappedLabels.length - deduplicatedLabels.length, + deduplicatedCount: combinedLabels.length - deduplicatedLabels.length, metadata: { typeLabels, contextLabels, @@ -212,18 +237,6 @@ export async function validateAndApplyLabels(input) { }; } - // If no labels provided, that's valid (no labels required) - if (!labels || labels.length === 0) { - return { - valid: true, - appliedLabels: [], - errors: [], - deduplicatedCount: 0, - validationErrors: [], - warnings: [], - }; - } - // Validate each label const validLabels = []; const invalidLabels = []; @@ -245,15 +258,8 @@ export async function validateAndApplyLabels(input) { continue; } - // Check if label is canonical or has valid prefix format - let isValid = false; - if (CANONICAL_LABELS[label]) { - isValid = true; - } else if (label.match(/^[a-z]+:[a-z0-9-]+$/)) { - isValid = true; - } - - if (!isValid) { + // A valid prefix is not enough: the label must be canonical. + if (!CANONICAL_LABELS[label]) { errors.push("non-canonical-label"); invalidLabels.push(label); continue; @@ -274,6 +280,15 @@ export async function validateAndApplyLabels(input) { } } + const changelogDecisionLabels = validLabels.filter((label) => + CHANGELOG_DECISION_LABELS.includes(label), + ); + if (changelogDecisionLabels.length === 0) { + errors.push("missing-changelog-decision-label"); + } else if (changelogDecisionLabels.length > 1) { + errors.push("multiple-changelog-decision-labels"); + } + // Sort labels by priority (lower priority number = higher priority) validLabels.sort((a, b) => { const priorityA = CANONICAL_LABELS[a] || 99; diff --git a/agents/pr-agent/skills/validate-branch-name/SKILL.md b/agents/pr-agent/skills/validate-branch-name/SKILL.md index 831219ee38..c85ab03907 100644 --- a/agents/pr-agent/skills/validate-branch-name/SKILL.md +++ b/agents/pr-agent/skills/validate-branch-name/SKILL.md @@ -1,9 +1,41 @@ --- -name: template-skill -description: Replace with description of the skill and when Claude should use it. +name: validate-branch-name +description: Validates that a branch name follows the {type}/{scope}-{short-title} convention, rejects forbidden prefixes (claude/, copilot/, openai/), and checks the type against the repository's canonical list. Use before creating or updating a PR, whenever a branch name needs checking against this repository's branching convention. --- -# Insert instructions below +# validate-branch-name -_This page brought to you by the 🦄 Magic Automation Unicorns of LightSpeedWP._ -[Automation Docs](https://github.com/lightspeedwp/.github/tree/main/instructions) +Validates a branch name follows `{type}/{scope}-{short-title}` format. + +## When to use this skill + +Before any PR is created or updated, and whenever a branch name needs to be checked against this repository's branching convention (e.g. `docs/BRANCHING_STRATEGY.md`). + +## Input + +```js +{ + branchName: string, // e.g. "feat/pr-agent-consolidation-portability" + config: object, // optional: override allowed types through config.allowed_types only +} +``` + +## Behaviour + +- Rejects branch names using a forbidden prefix (`claude/`, `copilot/`, `openai/`). +- Rejects a branch type not in the repository's approved-type list. +- Rejects a name that doesn't match `{type}/{scope}-{short-title}`. + +## Output + +```js +{ valid: boolean, errors: string[] } +``` + +## Usage + +```js +import { validateBranchName } from "./scripts/validate-branch-name.js"; + +const result = await validateBranchName({ branchName: "feat/my-feature" }); +``` diff --git a/agents/pr-agent/__tests__/validate-branch-name.test.js b/agents/pr-agent/skills/validate-branch-name/scripts/__tests__/validate-branch-name.test.js similarity index 99% rename from agents/pr-agent/__tests__/validate-branch-name.test.js rename to agents/pr-agent/skills/validate-branch-name/scripts/__tests__/validate-branch-name.test.js index 7f478621a6..9304f8677e 100644 --- a/agents/pr-agent/__tests__/validate-branch-name.test.js +++ b/agents/pr-agent/skills/validate-branch-name/scripts/__tests__/validate-branch-name.test.js @@ -1,4 +1,4 @@ -import { validateBranchName } from "../skills/validate-branch-name/validate-branch-name.js"; +import { validateBranchName } from "../validate-branch-name.js"; describe("Skill: validate-branch-name", () => { // ===== VALID BRANCH NAMES ===== diff --git a/agents/pr-agent/skills/validate-branch-name/validate-branch-name.js b/agents/pr-agent/skills/validate-branch-name/scripts/validate-branch-name.js similarity index 100% rename from agents/pr-agent/skills/validate-branch-name/validate-branch-name.js rename to agents/pr-agent/skills/validate-branch-name/scripts/validate-branch-name.js