fix: enforce release commit types - #130
Conversation
WalkthroughAdded enforcement of semantic versioning commit types (feat, fix, perf, revert, docs) when modifying release-critical files (package*.json, npm/global.json, .devcontainer/codex*, .codex/**). Implemented via a new commitlint plugin rule that inspects staged files and conditionally enforces the release-type requirement. Documented in code quality guidelines. Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant Git as Git
participant Commitlint as Commitlint
participant Plugin as Release-Type Plugin
Dev->>Git: git commit (with staged files)
Git->>Commitlint: trigger hook
Commitlint->>Plugin: check codex-release-type rule
alt Sensitive files detected
Plugin->>Git: getStagedFiles()
Git-->>Plugin: staged file list
Plugin->>Plugin: check if type ∈ {feat, fix, perf, revert, docs}
alt Valid release type
Plugin-->>Commitlint: ✓ pass
Commitlint-->>Git: allow commit
else Invalid type
Plugin-->>Commitlint: ✗ fail (release type required)
Commitlint-->>Dev: reject commit
end
else No sensitive files
Plugin-->>Commitlint: ✓ skip rule
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
commitlint.config.js (1)
21-34: LGTM with optional message enhancement.The logic correctly enforces release types when sensitive files are touched. The early return optimization is good.
Consider making the error message slightly more actionable:
return [ isReleaseType, - `Changes in ${touched.join(', ')} require a release-triggering type (feat|fix|perf|revert|docs) to keep semantic-release automated.`, + `Changes to ${touched.join(', ')} require a release-triggering commit type. Use: feat, fix, perf, revert, or docs (not chore, refactor, etc.) to keep semantic-release automated.`, ];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CLAUDE.md(1 hunks)README.md(1 hunks)commitlint.config.js(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: keito4/config PR: 0
File: .cursor/rules/commit.cursorrules:0-0
Timestamp: 2025-06-25T06:37:26.200Z
Learning: Commit messages must follow the Conventional Commits format: <type>(<scope>): <subject>, where type can be feat, fix, docs, refactor, etc.
📚 Learning: 2025-06-25T06:37:26.200Z
Learnt from: CR
Repo: keito4/config PR: 0
File: .cursor/rules/commit.cursorrules:0-0
Timestamp: 2025-06-25T06:37:26.200Z
Learning: Commit messages must follow the Conventional Commits format: <type>(<scope>): <subject>, where type can be feat, fix, docs, refactor, etc.
Applied to files:
README.md
🔇 Additional comments (2)
commitlint.config.js (2)
1-2: LGTM!The import is appropriate for executing git commands to retrieve staged files.
36-52: Verify that the codex-release-type rule blocks commits to release-sensitive files with non-release commit types.The plugin structure and implementation are sound. The
releaseTypeRulecorrectly identifies staged files and enforces release-type commit messages (feat|fix|perf|revert|docs) when changes touch sensitive files likepackage.jsonor.codex/directories.However, the original test approach cannot verify this:
git commit --dry-runskips hook execution entirely, so commitlint never runs. To properly test:# Test that non-release types are blocked for sensitive files git add package.json git commit -m "chore: update package.json" # Expected: commitlint blocks with error message about release-triggering types # Test that release types are allowed git commit --amend -m "fix: update package.json" # Expected: commitlint passesEnsure this rule works as expected in your local environment before merging.
| - **Test-Driven Development (TDD)**: Red → Green → Refactor methodology with 70%+ line coverage requirement | ||
| - **Static Quality Gates**: Automated linting, formatting, security analysis, and license checking | ||
| - **Git Workflow**: Conventional commits, branch naming conventions, and pull request requirements | ||
| - **Release Types Required for Tooling Changes**: Commits that touch `.codex/**`, `.devcontainer/codex*`, `package*.json`, or `npm/global.json` must use release-triggering types (`feat` / `fix` / `perf` / `revert` / `docs`). commitlint blocks `chore`など非リリース型のメッセージを防止し、semantic-release の自動リリースと整合させます。 |
There was a problem hiding this comment.
🧩 Analysis chain
Verify pattern consistency and consider language clarity.
The documented pattern .devcontainer/codex* is a shell glob, while commitlint.config.js uses specific regex patterns (/^\.devcontainer\/Dockerfile$/ and /^\.devcontainer\/codex-config\.json$/). Additionally, .devcontainer/Dockerfile is enforced in the code but not mentioned here, creating documentation drift.
The mixed Japanese/English text may also reduce readability for international contributors.
Consider aligning the patterns precisely between docs and code:
🏁 Script executed:
#!/bin/bash
# Verify which .devcontainer files are actually enforced
rg -n "devcontainer" commitlint.config.jsLength of output: 79
🏁 Script executed:
#!/bin/bash
# Check CLAUDE.md line 12 and surrounding context
sed -n '10,15p' CLAUDE.mdLength of output: 533
🏁 Script executed:
#!/bin/bash
# Verify full commitlint.config.js content for devcontainer patterns
rg -B2 -A2 "devcontainer" commitlint.config.jsLength of output: 145
🏁 Script executed:
#!/bin/bash
# Check if Dockerfile or codex-config.json appear elsewhere in commitlint config
rg -n "Dockerfile|codex-config" commitlint.config.jsLength of output: 79
Update CLAUDE.md line 12 to accurately document enforced patterns and improve language clarity.
Documentation drift exists. CLAUDE.md lists .devcontainer/codex* and package*.json, but commitlint enforces specific files: .devcontainer/Dockerfile, .devcontainer/codex-config.json, and package-lock.json. Additionally, .devcontainer/Dockerfile is entirely missing from the documentation.
Replace line 12 with precise patterns that match actual enforcement:
.codex/**(correct).devcontainer/Dockerfileand.devcontainer/codex-config.json(instead of glob.devcontainer/codex*)package-lock.json(instead ofpackage*.json)npm/global.json(correct)
Remove the mixed Japanese/English phrase for clarity.
🤖 Prompt for AI Agents
In CLAUDE.md around line 12, the documented file patterns are inaccurate and
include a mixed Japanese/English phrase; replace the current line with a clear,
precise list matching enforcement: use ".codex/**", ".devcontainer/Dockerfile",
".devcontainer/codex-config.json", "package-lock.json", and "npm/global.json"
and remove the mixed-language text so the line reads concisely and accurately
about required release-triggering commit types for those files.
| const releaseTypeAllowList = new Set(['feat', 'fix', 'perf', 'revert', 'docs']); | ||
| const releaseSensitivePatterns = [ | ||
| /^package\.json$/, | ||
| /^package-lock\.json$/, | ||
| /^npm\/global\.json$/, | ||
| /^\.devcontainer\/Dockerfile$/, | ||
| /^\.devcontainer\/codex-config\.json$/, | ||
| /^\.codex\//, | ||
| ]; |
There was a problem hiding this comment.
Pattern mismatch between implementation and documentation.
The patterns don't align with the documented glob .devcontainer/codex*:
- Line 8 matches
.devcontainer/Dockerfilewhich is unrelated to "codex" naming - Line 9 matches only
.devcontainer/codex-config.json, missing other potentialcodex*files
This creates two problems:
.devcontainer/Dockerfilechanges trigger the rule but aren't documented as release-critical- Files like
.devcontainer/codex-setup.shor.devcontainer/codex.jsonwould NOT trigger the rule despite matching the documented pattern
Apply this diff to match the documented behavior:
const releaseSensitivePatterns = [
/^package\.json$/,
/^package-lock\.json$/,
/^npm\/global\.json$/,
- /^\.devcontainer\/Dockerfile$/,
- /^\.devcontainer\/codex-config\.json$/,
+ /^\.devcontainer\/codex/,
/^\.codex\//,
];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const releaseTypeAllowList = new Set(['feat', 'fix', 'perf', 'revert', 'docs']); | |
| const releaseSensitivePatterns = [ | |
| /^package\.json$/, | |
| /^package-lock\.json$/, | |
| /^npm\/global\.json$/, | |
| /^\.devcontainer\/Dockerfile$/, | |
| /^\.devcontainer\/codex-config\.json$/, | |
| /^\.codex\//, | |
| ]; | |
| const releaseTypeAllowList = new Set(['feat', 'fix', 'perf', 'revert', 'docs']); | |
| const releaseSensitivePatterns = [ | |
| /^package\.json$/, | |
| /^package-lock\.json$/, | |
| /^npm\/global\.json$/, | |
| /^\.devcontainer\/codex/, | |
| /^\.codex\//, | |
| ]; |
| const getStagedFiles = () => { | ||
| try { | ||
| return execSync('git diff --cached --name-only', { encoding: 'utf8' }).split('\n').filter(Boolean); | ||
| } catch { | ||
| return []; | ||
| } | ||
| }; |
There was a problem hiding this comment.
Consider logging errors or documenting the silent failure behavior.
The try-catch silently returns an empty array on any error, which means:
- Rule enforcement is skipped if git commands fail
- Developers won't know why enforcement isn't working
- Could create a false sense of security
In CI environments or when using git commit --no-verify, this might not behave as expected.
Consider at minimum logging errors to help debug issues:
const getStagedFiles = () => {
try {
return execSync('git diff --cached --name-only', { encoding: 'utf8' }).split('\n').filter(Boolean);
- } catch {
+ } catch (error) {
+ // Log for debugging: console.warn('Unable to retrieve staged files:', error.message);
return [];
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const getStagedFiles = () => { | |
| try { | |
| return execSync('git diff --cached --name-only', { encoding: 'utf8' }).split('\n').filter(Boolean); | |
| } catch { | |
| return []; | |
| } | |
| }; | |
| const getStagedFiles = () => { | |
| try { | |
| return execSync('git diff --cached --name-only', { encoding: 'utf8' }).split('\n').filter(Boolean); | |
| } catch (error) { | |
| // Log for debugging: console.warn('Unable to retrieve staged files:', error.message); | |
| return []; | |
| } | |
| }; |
🤖 Prompt for AI Agents
In commitlint.config.js around lines 13 to 19, the try-catch around execSync
silently swallows all errors and returns an empty array; update the catch to
capture the error and log a clear diagnostic (e.g., console.error or a logger)
including the git command and error message, then return the empty array as the
fallback so behavior is documented in logs; ensure the logged message is concise
and includes context (file/function name and that staged files couldn't be
retrieved) to aid CI and local debugging.
| - The script performs `npm-check-updates`, `npm install`, rebuilds the `dist/` artifacts, and re-synchronizes global CLI versions via `npm view <package> version`. | ||
| - Packages that currently require newer Node.js releases (`semantic-release`, `@semantic-release/github`) are excluded by default. Override the exclusion list with `UPDATE_LIBS_REJECT="pkg1,pkg2" npm run update:libs` when you are ready to bump them. | ||
| - `.github/workflows/update-libraries.yml` executes the same script weekly and opens a PR whenever it produces changes, ensuring Codex/Claude Code tooling stays current without manual effort. | ||
| - Commits that touch release-critical files (`package*.json`, `npm/global.json`, `.devcontainer/codex*`, `.codex/**`) **must** use a release-triggering Conventional Commit type (`feat`, `fix`, `perf`, `revert`, or `docs`). Commitlint enforces this so semantic-release can publish automatically when tooling versions change. |
There was a problem hiding this comment.
Pattern documentation drift with implementation.
This line documents .devcontainer/codex* as release-critical, but commitlint.config.js actually enforces .devcontainer/Dockerfile and .devcontainer/codex-config.json specifically. This creates the same inconsistency noted in CLAUDE.md.
After resolving the pattern mismatch in commitlint.config.js, update this line to reflect the actual enforced patterns. If the intent is to match any .devcontainer/codex* file, update the regex; if only specific files should be enforced, document them explicitly here.
|
🎉 This PR is included in version 1.1.3 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary
chorecommits)feat|fix|perf|revert|docs) whenever.codex/**,.devcontainer/codex*,package*.json, ornpm/global.jsonchangeTesting
Summary by CodeRabbit
Documentation
Chores