fix: pr-agent - correct branch-validation prefix lists, complete the pr-creation-agent merge - #3403
Conversation
|
Warning Review limit reachedNext included review available in 21 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Repository: lightspeedwp/.github/.coderabbit.yml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (10)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Tick the box to add this pull request to the merge queue (same as
|
Merge Protections🔴 2 of 2 protections blocking · waiting on ⛓️ dependency
🔴 ⛓️ Depends-On RequirementsWaiting for
This rule is failing.Requirement based on the presence of
🔴 🚦 Auto-queueThis rule is failing.When all merge protections are satisfied and these conditions match, this pull request will be queued automatically.
|
deabb2b to
b9829b1
Compare
PR Template RoutingBranch Type: This PR was automatically routed based on the branch naming strategy. |
b9829b1 to
6c64c93
Compare
📋 Changelog Quality ValidationSummary
Status❌ Validation FAILED - This PR introduces 1 new changelog failure(s). Please fix them before merging. How to FixReproduce the exact PR check locally (from the repository root): cd .github/validation/changelog
node bin/validate.js --changelog-path ../../../CHANGELOG.md --trigger pr_submission --pr-number 3403 --branch fix/pr-agent-branch-name-validation --output textOnly new failures introduced by this PR block merging; pre-existing Unreleased failures do not block. See Changelog Quality Audit for the rule reference. |
Contract accuracy - contracts/pr-agent-invocation.md: the "must not" list forbade even reading .github/PULL_REQUEST_TEMPLATE/config.yml, contradicting FR-010's requirement to route through it. Narrowed to writes only -- reading it for routing is required, not prohibited. Not a LOCKED file change: this is our own contract doc, not the PR template. Branch-name consistency - Update every stale feature/pr-agent-consolidation-portability reference to aiops/pr-agent-consolidation-portability across spec.md and plan.md (4 more occurrences than CodeRabbit's single flagged line, found via full sweep) - Mark the Complexity Tracking violation resolved, not just renamed Verification accuracy - quickstart.md: check SKILL.md file *content* for the literal "template-skill" string via grep, not just file existence via ls Story status accuracy - spec.md: User Story 1 was incorrectly marked COMPLETE on this branch, which contains none of the actual restructuring/deletion work (that's PR #3401/#3403) -- corrected to IN PROGRESS, moved the completion claim to where it will actually be true Base-branch resolution policy - FR-006: was ambiguous -- "by branch type" but the only described mechanism (repository default branch) doesn't vary by type at all. Now explicit: hotfix/release -> production-role branch, everything else -> integration-role branch, falling back to the repo default only when neither role applies - data-model.md: updated base/resolvedBaseBranch fields to match Documentation accuracy - AGENT.md: Implementation Reference pointed at scripts/ paths that don't exist on this branch yet (that restructuring is PR #3401) -- corrected to this branch's actual current flat file layout Verification - markdownlint clean on all 6 touched files - agents/pr-agent full suite: 12/12 suites, 238/238 tests (unchanged)
Task generation - tasks.md: 37 tasks across Setup/Foundational/4 user-story phases, Story 1's 14 tasks marked done (delivered across PR #3400/#3401/#3403), Stories 2-4's remaining 23 tasks marked not started -- verified against actual skill code, not assumed Requirements-quality checklist - checklists/story2-behaviour.md: 22 items reviewing Story 2's FR-005 through FR-019 for completeness/clarity/consistency/measurability/ coverage, 100% traceability Governance-doc alignment fixes (cross-checked against the LightSpeed PR & Code Review Workflow doc, the GitHub Governance Rollout Strategy, and the open-pr skill) - FR-013a (new): the CI-enforced restrictedTypes rule (type:feature/ bug/performance/security/release/hotfix forbid meta:no-changelog regardless of stack position) -- found only by triggering the real CI check on PR #3403, documented nowhere else in this repo. Also added to data-model.md's Changelog Entry entity. - FR-014: embedded the exact ## Stack block field format (Position/ Issue-Epic/Depends on/Followed by/Review scope) instead of relying only on an external reference - FR-008: added concrete file patterns for the review-budget exclusion list (package-lock.json, *.lock, *.snapshot, dist/**, build/**, *.min.js, *.po/*.pot) Verification - markdownlint clean on all touched/created files
Contract accuracy - contracts/pr-agent-invocation.md: the "must not" list forbade even reading .github/PULL_REQUEST_TEMPLATE/config.yml, contradicting FR-010's requirement to route through it. Narrowed to writes only -- reading it for routing is required, not prohibited. Not a LOCKED file change: this is our own contract doc, not the PR template. Branch-name consistency - Update every stale feature/pr-agent-consolidation-portability reference to aiops/pr-agent-consolidation-portability across spec.md and plan.md (4 more occurrences than CodeRabbit's single flagged line, found via full sweep) - Mark the Complexity Tracking violation resolved, not just renamed Verification accuracy - quickstart.md: check SKILL.md file *content* for the literal "template-skill" string via grep, not just file existence via ls Story status accuracy - spec.md: User Story 1 was incorrectly marked COMPLETE on this branch, which contains none of the actual restructuring/deletion work (that's PR #3401/#3403) -- corrected to IN PROGRESS, moved the completion claim to where it will actually be true Base-branch resolution policy - FR-006: was ambiguous -- "by branch type" but the only described mechanism (repository default branch) doesn't vary by type at all. Now explicit: hotfix/release -> production-role branch, everything else -> integration-role branch, falling back to the repo default only when neither role applies - data-model.md: updated base/resolvedBaseBranch fields to match Documentation accuracy - AGENT.md: Implementation Reference pointed at scripts/ paths that don't exist on this branch yet (that restructuring is PR #3401) -- corrected to this branch's actual current flat file layout Verification - markdownlint clean on all 6 touched files - agents/pr-agent full suite: 12/12 suites, 238/238 tests (unchanged)
Task generation - tasks.md: 37 tasks across Setup/Foundational/4 user-story phases, Story 1's 14 tasks marked done (delivered across PR #3400/#3401/#3403), Stories 2-4's remaining 23 tasks marked not started -- verified against actual skill code, not assumed Requirements-quality checklist - checklists/story2-behaviour.md: 22 items reviewing Story 2's FR-005 through FR-019 for completeness/clarity/consistency/measurability/ coverage, 100% traceability Governance-doc alignment fixes (cross-checked against the LightSpeed PR & Code Review Workflow doc, the GitHub Governance Rollout Strategy, and the open-pr skill) - FR-013a (new): the CI-enforced restrictedTypes rule (type:feature/ bug/performance/security/release/hotfix forbid meta:no-changelog regardless of stack position) -- found only by triggering the real CI check on PR #3403, documented nowhere else in this repo. Also added to data-model.md's Changelog Entry entity. - FR-014: embedded the exact ## Stack block field format (Position/ Issue-Epic/Depends on/Followed by/Review scope) instead of relying only on an external reference - FR-008: added concrete file patterns for the review-budget exclusion list (package-lock.json, *.lock, *.snapshot, dist/**, build/**, *.min.js, *.po/*.pot) Verification - markdownlint clean on all touched/created files
61d16d3 to
fee3eda
Compare
Contract accuracy - contracts/pr-agent-invocation.md: the "must not" list forbade even reading .github/PULL_REQUEST_TEMPLATE/config.yml, contradicting FR-010's requirement to route through it. Narrowed to writes only -- reading it for routing is required, not prohibited. Not a LOCKED file change: this is our own contract doc, not the PR template. Branch-name consistency - Update every stale feature/pr-agent-consolidation-portability reference to aiops/pr-agent-consolidation-portability across spec.md and plan.md (4 more occurrences than CodeRabbit's single flagged line, found via full sweep) - Mark the Complexity Tracking violation resolved, not just renamed Verification accuracy - quickstart.md: check SKILL.md file *content* for the literal "template-skill" string via grep, not just file existence via ls Story status accuracy - spec.md: User Story 1 was incorrectly marked COMPLETE on this branch, which contains none of the actual restructuring/deletion work (that's PR #3401/#3403) -- corrected to IN PROGRESS, moved the completion claim to where it will actually be true Base-branch resolution policy - FR-006: was ambiguous -- "by branch type" but the only described mechanism (repository default branch) doesn't vary by type at all. Now explicit: hotfix/release -> production-role branch, everything else -> integration-role branch, falling back to the repo default only when neither role applies - data-model.md: updated base/resolvedBaseBranch fields to match Documentation accuracy - AGENT.md: Implementation Reference pointed at scripts/ paths that don't exist on this branch yet (that restructuring is PR #3401) -- corrected to this branch's actual current flat file layout Verification - markdownlint clean on all 6 touched files - agents/pr-agent full suite: 12/12 suites, 238/238 tests (unchanged)
Task generation - tasks.md: 37 tasks across Setup/Foundational/4 user-story phases, Story 1's 14 tasks marked done (delivered across PR #3400/#3401/#3403), Stories 2-4's remaining 23 tasks marked not started -- verified against actual skill code, not assumed Requirements-quality checklist - checklists/story2-behaviour.md: 22 items reviewing Story 2's FR-005 through FR-019 for completeness/clarity/consistency/measurability/ coverage, 100% traceability Governance-doc alignment fixes (cross-checked against the LightSpeed PR & Code Review Workflow doc, the GitHub Governance Rollout Strategy, and the open-pr skill) - FR-013a (new): the CI-enforced restrictedTypes rule (type:feature/ bug/performance/security/release/hotfix forbid meta:no-changelog regardless of stack position) -- found only by triggering the real CI check on PR #3403, documented nowhere else in this repo. Also added to data-model.md's Changelog Entry entity. - FR-014: embedded the exact ## Stack block field format (Position/ Issue-Epic/Depends on/Followed by/Review scope) instead of relying only on an external reference - FR-008: added concrete file patterns for the review-budget exclusion list (package-lock.json, *.lock, *.snapshot, dist/**, build/**, *.min.js, *.po/*.pot) Verification - markdownlint clean on all touched/created files
fee3eda to
41a03cd
Compare
Contract accuracy - contracts/pr-agent-invocation.md: the "must not" list forbade even reading .github/PULL_REQUEST_TEMPLATE/config.yml, contradicting FR-010's requirement to route through it. Narrowed to writes only -- reading it for routing is required, not prohibited. Not a LOCKED file change: this is our own contract doc, not the PR template. Branch-name consistency - Update every stale feature/pr-agent-consolidation-portability reference to aiops/pr-agent-consolidation-portability across spec.md and plan.md (4 more occurrences than CodeRabbit's single flagged line, found via full sweep) - Mark the Complexity Tracking violation resolved, not just renamed Verification accuracy - quickstart.md: check SKILL.md file *content* for the literal "template-skill" string via grep, not just file existence via ls Story status accuracy - spec.md: User Story 1 was incorrectly marked COMPLETE on this branch, which contains none of the actual restructuring/deletion work (that's PR #3401/#3403) -- corrected to IN PROGRESS, moved the completion claim to where it will actually be true Base-branch resolution policy - FR-006: was ambiguous -- "by branch type" but the only described mechanism (repository default branch) doesn't vary by type at all. Now explicit: hotfix/release -> production-role branch, everything else -> integration-role branch, falling back to the repo default only when neither role applies - data-model.md: updated base/resolvedBaseBranch fields to match Documentation accuracy - AGENT.md: Implementation Reference pointed at scripts/ paths that don't exist on this branch yet (that restructuring is PR #3401) -- corrected to this branch's actual current flat file layout Verification - markdownlint clean on all 6 touched files - agents/pr-agent full suite: 12/12 suites, 238/238 tests (unchanged)
41a03cd to
71d700c
Compare
Task generation - tasks.md: 37 tasks across Setup/Foundational/4 user-story phases, Story 1's 14 tasks marked done (delivered across PR #3400/#3401/#3403), Stories 2-4's remaining 23 tasks marked not started -- verified against actual skill code, not assumed Requirements-quality checklist - checklists/story2-behaviour.md: 22 items reviewing Story 2's FR-005 through FR-019 for completeness/clarity/consistency/measurability/ coverage, 100% traceability Governance-doc alignment fixes (cross-checked against the LightSpeed PR & Code Review Workflow doc, the GitHub Governance Rollout Strategy, and the open-pr skill) - FR-013a (new): the CI-enforced restrictedTypes rule (type:feature/ bug/performance/security/release/hotfix forbid meta:no-changelog regardless of stack position) -- found only by triggering the real CI check on PR #3403, documented nowhere else in this repo. Also added to data-model.md's Changelog Entry entity. - FR-014: embedded the exact ## Stack block field format (Position/ Issue-Epic/Depends on/Followed by/Review scope) instead of relying only on an external reference - FR-008: added concrete file patterns for the review-budget exclusion list (package-lock.json, *.lock, *.snapshot, dist/**, build/**, *.min.js, *.po/*.pot) Verification - markdownlint clean on all touched/created files
Branch validation fix - FORBIDDEN_PREFIXES: claude/bot/automated -> claude/copilot/openai (was contradicting docs/BRANCHING_STRATEGY.md Section 4.1) - ALLOWED_TYPES: add task, doc, aiops, automation, epic -- the 5 canonical types missing from the previous list - Add test coverage for both lists (neither had any before) Lint verification (deferred from Step 4) - Fix the labels unused-var error in orchestrate-pr-creation.test.js - Confirm the flat config actually catches a deliberately introduced violation, not just passing by chance Verification - Full suite: 13/13 test suites, 247/247 tests passing - Lint: clean
Verification before deletion - Confirmed all 5 files had a recorded disposition: pr-creation.agent.md folded into AGENT.md; eslint.config.js and its test adopted byte-for-byte identical into pr-agent; package.json/package-lock.json superseded by pr-agent's own corrected versions - Confirmed pr-agent's test suite and lint pass unaffected by the removal (13/13 suites, 247/247 tests, lint clean)
- README.md: skills table, structure diagram, dev commands, and a governance section referencing docs/BRANCHING_*.md, PR_*.md, LABEL*.md, ISSUE*.md by wildcard rather than duplicating their rules - CHANGELOG.md: Keep a Changelog format, matching this repo's existing agent convention -- 1.1.0 documents the full pr-creation-agent merge and skills restructuring, 1.0.0 as the prior baseline This completes User Story 1's merge and restructure work.
Required because type:bug is on this repo's CI-enforced restrictedTypes list -- meta:no-changelog is not permitted for it, only meta:needs-changelog with a real entry. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contract accuracy - contracts/pr-agent-invocation.md: the "must not" list forbade even reading .github/PULL_REQUEST_TEMPLATE/config.yml, contradicting FR-010's requirement to route through it. Narrowed to writes only -- reading it for routing is required, not prohibited. Not a LOCKED file change: this is our own contract doc, not the PR template. Branch-name consistency - Update every stale feature/pr-agent-consolidation-portability reference to aiops/pr-agent-consolidation-portability across spec.md and plan.md (4 more occurrences than CodeRabbit's single flagged line, found via full sweep) - Mark the Complexity Tracking violation resolved, not just renamed Verification accuracy - quickstart.md: check SKILL.md file *content* for the literal "template-skill" string via grep, not just file existence via ls Story status accuracy - spec.md: User Story 1 was incorrectly marked COMPLETE on this branch, which contains none of the actual restructuring/deletion work (that's PR #3401/#3403) -- corrected to IN PROGRESS, moved the completion claim to where it will actually be true Base-branch resolution policy - FR-006: was ambiguous -- "by branch type" but the only described mechanism (repository default branch) doesn't vary by type at all. Now explicit: hotfix/release -> production-role branch, everything else -> integration-role branch, falling back to the repo default only when neither role applies - data-model.md: updated base/resolvedBaseBranch fields to match Documentation accuracy - AGENT.md: Implementation Reference pointed at scripts/ paths that don't exist on this branch yet (that restructuring is PR #3401) -- corrected to this branch's actual current flat file layout Verification - markdownlint clean on all 6 touched files - agents/pr-agent full suite: 12/12 suites, 238/238 tests (unchanged)
Task generation - tasks.md: 37 tasks across Setup/Foundational/4 user-story phases, Story 1's 14 tasks marked done (delivered across PR #3400/#3401/#3403), Stories 2-4's remaining 23 tasks marked not started -- verified against actual skill code, not assumed Requirements-quality checklist - checklists/story2-behaviour.md: 22 items reviewing Story 2's FR-005 through FR-019 for completeness/clarity/consistency/measurability/ coverage, 100% traceability Governance-doc alignment fixes (cross-checked against the LightSpeed PR & Code Review Workflow doc, the GitHub Governance Rollout Strategy, and the open-pr skill) - FR-013a (new): the CI-enforced restrictedTypes rule (type:feature/ bug/performance/security/release/hotfix forbid meta:no-changelog regardless of stack position) -- found only by triggering the real CI check on PR #3403, documented nowhere else in this repo. Also added to data-model.md's Changelog Entry entity. - FR-014: embedded the exact ## Stack block field format (Position/ Issue-Epic/Depends on/Followed by/Review scope) instead of relying only on an external reference - FR-008: added concrete file patterns for the review-budget exclusion list (package-lock.json, *.lock, *.snapshot, dist/**, build/**, *.min.js, *.po/*.pot) Verification - markdownlint clean on all touched/created files
55425e8 to
6eddea4
Compare
Linked issues
Part of LS-4214
Context
validate-branch-name.js's forbidden/allowed lists contradicted this repository's owndocs/BRANCHING_STRATEGY.md, with zero test coverage on either listagents/pr-agent'svalidate-branch-nameskill onlyReproduction
validateBranchNamewith aclaude/,copilot/, oropenai/branch name 2) observe onlyclaude/was rejected 3) call it withtask/,doc/,aiops/,automation/, orepic/4) observe all five were incorrectly rejected as unknown typesdocs/BRANCHING_STRATEGY.mdSection 3/4.1 exactly; actual lists predated that document and were never reconciled against itRoot Cause
FORBIDDEN_PREFIXESwas hardcoded to["claude", "bot", "automated"]andALLOWED_TYPESwas missing 5 canonical types, both sourced independently rather than from the canonical document — the exact failure modedocs/BRANCHING_STRATEGY.md's own governance section warns against.Fix Summary
FORBIDDEN_PREFIXES:claude/bot/automated→claude/copilot/openai, matching Section 4.1 exactlyALLOWED_TYPES: addedtask,doc,aiops,automation,epic— the 5 canonical types missing from the previous listlabelsunused-var lint violation inorchestrate-pr-creation.test.js(uncaught until PR 1 gave this agent a working lint config) and confirmed the config genuinely catches violations via a deliberately-introduced fixture, reverted after confirmingagents/pr-creation-agent/— its 5 files were each individually confirmed folded intopr-agent, superseded, or discarded-with-reason across PRs 1-2; re-verified here thatpr-agent's suite and lint are unaffected by the removalREADME.mdandCHANGELOG.mdforpr-agent, completing User Story 1 of spec 015This is PR 3 of a 3-PR stack, based on PR 2 (
refactor/pr-agent-skills-restructure), notdevelop. Together, PRs 1-3 complete User Story 1 (the merge and restructure) of spec 015 — later work (Stories 2-4: PR-creation behaviour, portability, further polish) is separate, not started here.Verification
pr-agent's full suite (13/13 suites, 247/247 tests) and lint pass after the deletion, not just before itbot/andautomated/are still correctly rejected (as unknown types now, not as forbidden prefixes) rather than silently becoming validRisk & Rollback
pr-creation-agent's files had a confirmed, verified dispositionpr-creation-agent/would need restoring from git history if reverted after mergeChangelog
type:bugis on this repository's CI-enforcedrestrictedTypeslist, someta:no-changelogis not permitted here even though this is internal agent tooling. Added a rootCHANGELOG.mdentry under Fixed,meta:needs-changelogapplied.agents/pr-agent/CHANGELOG.md(also added by this PR) documents the change at the agent level as well.Added
Changed
Fixed
Removed
Checklist (Global DoD / PR)
README.md/CHANGELOG.mdadded at the agent level; rootCHANGELOG.mdentry added,meta:needs-changelogat the repo levelStack
validate-branch-name.jsprefix-list fix and its new tests, the deferred lint fix, thepr-creation-agentdeletion, andREADME.md/CHANGELOG.mdPart of LS-4214