docs: CONTRIBUTING.md — remove references frontmatter, npm ci, date fix#457
Conversation
… stale date - Remove `references` frontmatter field (violates repo convention; use inline links instead) - Quick Start step 2: `npm install` → `npm ci` for reproducible installs - Update stale "Last Updated: 2025-10-21" body line to 2026-05-27 Closes #18
|
Warning Review limit reached
More reviews will be available in 27 minutes and 29 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
Note
|
There was a problem hiding this comment.
Code Review
This pull request updates the CONTRIBUTING.md guidelines, including changing the dependency installation command to npm ci, updating several documentation references, and adding links to new governance and override policies. The review comments point out that the newly referenced file GOVERNANCE_REVISION_LOG.md located in the ./docs/ directory violates the repository's naming convention, which requires lowercase letters and hyphens for word separation.
| - **Saved Replies:** Use [SAVED_REPLIES/README.md](.github/SAVED_REPLIES/README.md) for common responses and efficient communication. | ||
| - **Documentation:** Update relevant docs (README, instructions) for any user-facing change. | ||
| - **Automation & Labels:** Ensure your issue/PR complies with [AUTOMATION_GOVERNANCE.md](./docs/AUTOMATION_GOVERNANCE.md), [ISSUE_LABELS.md](docs/ISSUE_LABELS.md), and [ISSUE_TYPES.md](./docs/ISSUE_TYPES.md). | ||
| - **Governance process updates:** If your change modifies governance policy or contributor workflow expectations, add an entry to [GOVERNANCE_REVISION_LOG.md](./docs/GOVERNANCE_REVISION_LOG.md). |
There was a problem hiding this comment.
According to the repository's naming conventions in instructions/community-standards.instructions.md, file names must use lowercase with hyphens for word separation (except for specific top-level community health files). Since this file is located in the ./docs/ directory, it should be named governance-revision-log.md instead of GOVERNANCE_REVISION_LOG.md.
| - **Governance process updates:** If your change modifies governance policy or contributor workflow expectations, add an entry to [GOVERNANCE_REVISION_LOG.md](./docs/GOVERNANCE_REVISION_LOG.md). | |
| - **Governance process updates:** If your change modifies governance policy or contributor workflow expectations, add an entry to [governance-revision-log.md](./docs/governance-revision-log.md). |
References
- File names must use lowercase with hyphens for word separation (except for specific top-level community health files). (link)
There was a problem hiding this comment.
The docs/ directory in this repository intentionally uses UPPERCASE naming for documentation files — e.g. AGENT_CREATION.md, ARCHITECTURE.md, AUTOMATION_GOVERNANCE.md. GOVERNANCE_REVISION_LOG.md follows that same convention. The lowercase-with-hyphens rule applies to other asset types, not to docs/ governance documents. Declining this suggestion.
Generated by Claude Code
| - [BRANCHING_STRATEGY.md](./docs/BRANCHING_STRATEGY.md): Org-wide branch naming, merge discipline, and automation mapping. | ||
| - [CHANGELOG.md](./CHANGELOG.md): Changelog format, release notes, and versioning. | ||
| - [AUTOMATION_GOVERNANCE.md](./docs/AUTOMATION_GOVERNANCE.md): Org-wide automation, branching, label, and release strategy. | ||
| - [GOVERNANCE_REVISION_LOG.md](./docs/GOVERNANCE_REVISION_LOG.md): Lightweight audit trail for governance/process changes. |
There was a problem hiding this comment.
According to the repository's naming conventions in instructions/community-standards.instructions.md, file names must use lowercase with hyphens for word separation (except for specific top-level community health files). Since this file is located in the ./docs/ directory, it should be named governance-revision-log.md instead of GOVERNANCE_REVISION_LOG.md.
| - [GOVERNANCE_REVISION_LOG.md](./docs/GOVERNANCE_REVISION_LOG.md): Lightweight audit trail for governance/process changes. | |
| - [governance-revision-log.md](./docs/governance-revision-log.md): Lightweight audit trail for governance/process changes. |
References
- File names must use lowercase with hyphens for word separation (except for specific top-level community health files). (link)
There was a problem hiding this comment.
Same as above — docs/ files in this repo use UPPERCASE naming (AGENT_CREATION.md, ARCHITECTURE.md, AUTOMATION_GOVERNANCE.md). Declining this suggestion.
Generated by Claude Code
…n't throw
`run()` was called at module scope unconditionally. When Jest's test file
does `require('../project-meta-sync.agent')` the function executes immediately,
throws "LS_PROJECT_URL not set", and sets process.exitCode = 1, failing the
entire test suite. Guard with `require.main === module` so the side-effect only
fires when the script is run directly by Node/Actions.
Add a typeof check and an explicit exit-code assertion so the test fails visibly if require() triggers run() and corrupts process.exitCode.
…removed) Node 24 removed the built-in `punycode` module that was only deprecated in Node 22. @stoplight/spectral-core still requires it, so `lint:yaml` and `lint:workflows` crash immediately. Node 22 LTS (EOL Apr 2027) is the highest version all current dependencies support without errors.
- Fix skills/design-md-agent/pdfs/js/package.json: add @lightspeedwp scope and required fields (description, license, author, repository) - Fix .schemas/README.md: resolve git merge conflict markers - Fix scripts/agents/__tests__/reviewer.agent.test.js: avoid require() on module that uses import.meta.url; check file existence instead
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f26636ee6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| - **Governance process updates:** If your change modifies governance policy or contributor workflow expectations, add an entry to [GOVERNANCE_REVISION_LOG.md](./docs/GOVERNANCE_REVISION_LOG.md). | ||
| - **Downstream overrides:** If you are adopting org defaults in another repository, follow [Downstream Override Policy](./docs/override-policy.md) and link any approved exception. |
There was a problem hiding this comment.
Add the governance docs before linking them
In this checkout I searched for GOVERNANCE_REVISION_LOG, override-policy, and Downstream Override, and enumerated docs/; these two newly linked files are only referenced here and do not exist. Contributors following the new governance/override guidance will hit broken links, and the repository link validator will flag this once it checks CONTRIBUTING.md; either add the referenced docs in this change or defer these links until the files exist.
Useful? React with 👍 / 👎.
…RIBUTING fixes, project-meta-sync guard Applies changes from PRs #455, #457, #460 (merged into main) to develop: - agents/mode-prd.agent.md: remove `update_issue`, keep `issue_write`/`issue_read` - .github/prompts/*.prompt.md (3 files): remove `update_issue`, UK English - CONTRIBUTING.md: remove `references` frontmatter field, fix stale body date, UK English pass - .github/ISSUE_TEMPLATE/02-bug.md: expand single DoD line → three discrete items (WCAG 2.2 AA / OWASP / perf) - .github/ISSUE_TEMPLATE/03-feature.md: same DoD expansion - scripts/agents/project-meta-sync.agent.js: add require.main guard + module.exports
…after merging #455 #457 #460 - CHANGELOG.md: add Fixed + Documentation entries for the three merged PRs - next-issues-execution-plan: mark #52/#18/#21 closed, Wave 1 complete, #60 next; add PR branching note - spec-only-agents-issue-conversion: bump last_updated - launch-agents-checklist: bump last_updated, note project-meta-sync + reviewer test fixes
* fix: replace deprecated MCP tool refs (create_issue → issue_write, get_issue → issue_read) (#455) * fix: replace deprecated create_issue/update_issue/get_issue with issue_write/issue_read Updates four files to use current MCP tool names: - create_issue → issue_write - update_issue → issue_write (same tool, method: create|update) - get_issue → issue_read Closes #52 * fix(test): guard project-meta-sync auto-run so require() in Jest doesn't throw `run()` was called at module scope unconditionally. When Jest's test file does `require('../project-meta-sync.agent')` the function executes immediately, throws "LS_PROJECT_URL not set", and sets process.exitCode = 1, failing the entire test suite. Guard with `require.main === module` so the side-effect only fires when the script is run directly by Node/Actions. * test: strengthen project-meta-sync guard assertions Add a typeof check and an explicit exit-code assertion so the test fails visibly if require() triggers run() and corrupts process.exitCode. * fix(ci): pin Node to v22 LTS — spectral crashes on Node 24 (punycode removed) Node 24 removed the built-in `punycode` module that was only deprecated in Node 22. @stoplight/spectral-core still requires it, so `lint:yaml` and `lint:workflows` crash immediately. Node 22 LTS (EOL Apr 2027) is the highest version all current dependencies support without errors. * debug(ci): split check into individual steps to identify failing step * debug(ci): add HUSKY=0 and split steps to pinpoint failure * fix(ci): resolve lint and test failures blocking CI - Fix skills/design-md-agent/pdfs/js/package.json: add @lightspeedwp scope and required fields (description, license, author, repository) - Fix .schemas/README.md: resolve git merge conflict markers - Fix scripts/agents/__tests__/reviewer.agent.test.js: avoid require() on module that uses import.meta.url; check file existence instead * docs: CONTRIBUTING.md — remove references frontmatter, npm ci, date fix (#457) * docs: remove references frontmatter, fix npm install → npm ci, update stale date - Remove `references` frontmatter field (violates repo convention; use inline links instead) - Quick Start step 2: `npm install` → `npm ci` for reproducible installs - Update stale "Last Updated: 2025-10-21" body line to 2026-05-27 Closes #18 * fix(test): guard project-meta-sync auto-run so require() in Jest doesn't throw `run()` was called at module scope unconditionally. When Jest's test file does `require('../project-meta-sync.agent')` the function executes immediately, throws "LS_PROJECT_URL not set", and sets process.exitCode = 1, failing the entire test suite. Guard with `require.main === module` so the side-effect only fires when the script is run directly by Node/Actions. * test: strengthen project-meta-sync guard assertions Add a typeof check and an explicit exit-code assertion so the test fails visibly if require() triggers run() and corrupts process.exitCode. * fix(ci): pin Node to v22 LTS — spectral crashes on Node 24 (punycode removed) Node 24 removed the built-in `punycode` module that was only deprecated in Node 22. @stoplight/spectral-core still requires it, so `lint:yaml` and `lint:workflows` crash immediately. Node 22 LTS (EOL Apr 2027) is the highest version all current dependencies support without errors. * fix(ci): resolve lint and test failures blocking CI - Fix skills/design-md-agent/pdfs/js/package.json: add @lightspeedwp scope and required fields (description, license, author, repository) - Fix .schemas/README.md: resolve git merge conflict markers - Fix scripts/agents/__tests__/reviewer.agent.test.js: avoid require() on module that uses import.meta.url; check file existence instead * fix: add explicit accessibility and security DoD checklist items to issue templates (#460) * fix: add explicit accessibility and security DoD checklist items to issue templates - 02-bug.md DoD: replace single "No adverse impact on performance or security" line with discrete checklist items for WCAG 2.2 AA and OWASP Top 10 - 03-feature.md DoD: expand "Accessibility, performance, security checks (where relevant)" into three discrete checklist items matching PR template standards Closes #21 * fix(test): guard project-meta-sync auto-run so require() in Jest doesn't throw `run()` was called at module scope unconditionally. When Jest's test file does `require('../project-meta-sync.agent')` the function executes immediately, throws "LS_PROJECT_URL not set", and sets process.exitCode = 1, failing the entire test suite. Guard with `require.main === module` so the side-effect only fires when the script is run directly by Node/Actions. * test: strengthen project-meta-sync guard assertions Add a typeof check and an explicit exit-code assertion so the test fails visibly if require() triggers run() and corrupts process.exitCode. * fix(ci): pin Node to v22 LTS — spectral crashes on Node 24 (punycode removed) Node 24 removed the built-in `punycode` module that was only deprecated in Node 22. @stoplight/spectral-core still requires it, so `lint:yaml` and `lint:workflows` crash immediately. Node 22 LTS (EOL Apr 2027) is the highest version all current dependencies support without errors. * fix(template): add Performance: prefix to bug DoD item for consistency Aligns with the Accessibility: and Security: prefix style on the adjacent checklist items, as suggested in review. * fix(template): restore bug template with Performance: prefix on DoD item Restores the accidental empty-file push and applies the Performance: prefix to align with Accessibility: and Security: item style. * fix(ci): resolve lint and test failures blocking CI - Fix skills/design-md-agent/pdfs/js/package.json: add @lightspeedwp scope and required fields (description, license, author, repository) - Fix .schemas/README.md: resolve git merge conflict markers - Fix scripts/agents/__tests__/reviewer.agent.test.js: avoid require() on module that uses import.meta.url; check file existence instead * fix: add trailing newline to 02-bug.md (MD047) * feat: add project assignment functionality to planner agent - Implement deriveProjectFromLabels() to map area labels to projects - Add project assignment recommendation to generated plans - Include confidence level and reasoning for assignments - Add logging for project assignment tracking - Support fallback to manual review for ambiguous cases This implements Phase 2 of the planner agent enhancement, enabling automatic project assignment based on issue labels. * fix: correct string replacement regex for project assignment section The regex now properly matches the footer including the HTML comment and uses regex instead of literal string to handle special characters. This fixes the project assignment section not being added to plans. * fix: add defensive checks and CRLF-aware regex - Add defensive check to ensure labels parameter is an array - Update regex to handle both LF and CRLF line endings (\r?\n) - Improves robustness across different environments * fix: correct YAML syntax error in metrics-summary workflow The multi-line JavaScript template literal was causing YAML parser errors. Converted to single-line string concatenation with proper variable handling. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Summary
referencesfield from YAML frontmatter — the repo convention (CLAUDE.md) forbids it; links belong inline or in a footer sectionnpm install→npm cifor reproducible, locked installsLast Updated: 2025-10-21body line to2026-05-27(frontmatter was already correct)Test plan
referenceskey present)npm ci2026-05-27Closes #18
Generated by Claude Code