From aa6f30130576686547a5ad04186fc8a013bf8ace Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 16 Mar 2026 12:24:59 +0200 Subject: [PATCH] ci: add conventional commit PR title linting (#23096) Restore PR title validation that was removed in 828f33a when cdr-bot was expected to handle it. That bot has since been disabled. The new title job in contrib.yaml validates: - Conventional commit format (type(scope): description) - Type from the same set used by release notes generation - Scope validity derived from the changed files in the PR diff - All changed files fall under the declared scope Uses actions/github-script (no third-party marketplace actions). Also fixes feat(api) examples across docs (no api folder exists) and consolidates commit rules into CONTRIBUTING.md as the single source of truth. --- .claude/docs/ARCHITECTURE.md | 2 +- .claude/docs/PR_STYLE_GUIDE.md | 19 ++--- .claude/docs/WORKFLOWS.md | 8 +- .github/workflows/contrib.yaml | 103 ++++++++++++++++++++++++ docs/about/contributing/CONTRIBUTING.md | 11 ++- scripts/release.sh | 2 +- 6 files changed, 123 insertions(+), 22 deletions(-) diff --git a/.claude/docs/ARCHITECTURE.md b/.claude/docs/ARCHITECTURE.md index 097b0f0d8d..5d4807db97 100644 --- a/.claude/docs/ARCHITECTURE.md +++ b/.claude/docs/ARCHITECTURE.md @@ -113,7 +113,7 @@ Coder emphasizes clear error handling, with specific patterns required: All tests should run in parallel using `t.Parallel()` to ensure efficient testing and expose potential race conditions. The codebase is rigorously linted with golangci-lint to maintain consistent code quality. -Git contributions follow a standard format with commit messages structured as `type: `, where type is one of `feat`, `fix`, or `chore`. +Git contributions follow [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/). See [CONTRIBUTING.md](docs/about/contributing/CONTRIBUTING.md#commit-messages) for full rules. PR titles are linted in CI. ## Development Workflow diff --git a/.claude/docs/PR_STYLE_GUIDE.md b/.claude/docs/PR_STYLE_GUIDE.md index 76ae2e728c..6e106a1094 100644 --- a/.claude/docs/PR_STYLE_GUIDE.md +++ b/.claude/docs/PR_STYLE_GUIDE.md @@ -4,22 +4,13 @@ This guide documents the PR description style used in the Coder repository, base ## PR Title Format -Follow [Conventional Commits 1.0.0](https://www.conventionalcommits.org/en/v1.0.0/) format: +Format: `type(scope): description`. See [CONTRIBUTING.md](docs/about/contributing/CONTRIBUTING.md#commit-messages) for full rules. PR titles are linted in CI. -```text -type(scope): brief description -``` +- Types: `feat`, `fix`, `docs`, `style`, `refactor`, `perf`, `test`, `build`, `ci`, `chore`, `revert` +- Scopes must be a real path (directory or file stem) containing all changed files +- Omit scope if changes span multiple top-level directories -**Common types:** - -- `feat`: New features -- `fix`: Bug fixes -- `refactor`: Code refactoring without behavior change -- `perf`: Performance improvements -- `docs`: Documentation changes -- `chore`: Dependency updates, tooling changes - -**Examples:** +Examples: - `feat: add tracing to aibridge` - `fix: move contexts to appropriate locations` diff --git a/.claude/docs/WORKFLOWS.md b/.claude/docs/WORKFLOWS.md index c8e7338f20..4d2bab4898 100644 --- a/.claude/docs/WORKFLOWS.md +++ b/.claude/docs/WORKFLOWS.md @@ -136,9 +136,11 @@ Then make your changes and push normally. Don't use `git push --force` unless th ## Commit Style -- Follow [Conventional Commits 1.0.0](https://www.conventionalcommits.org/en/v1.0.0/) -- Format: `type(scope): message` -- Types: `feat`, `fix`, `docs`, `style`, `refactor`, `test`, `chore` +Format: `type(scope): message`. See [CONTRIBUTING.md](docs/about/contributing/CONTRIBUTING.md#commit-messages) for full rules. PR titles are linted in CI. + +- Types: `feat`, `fix`, `docs`, `style`, `refactor`, `perf`, `test`, `build`, `ci`, `chore`, `revert` +- Scopes must be a real path (directory or file stem) containing all changed files +- Omit scope if changes span multiple top-level directories - Keep message titles concise (~70 characters) - Use imperative, present tense in commit titles diff --git a/.github/workflows/contrib.yaml b/.github/workflows/contrib.yaml index 37366d697d..85578785c6 100644 --- a/.github/workflows/contrib.yaml +++ b/.github/workflows/contrib.yaml @@ -45,6 +45,109 @@ jobs: # Some users have signed a corporate CLA with Coder so are exempt from signing our community one. allowlist: "coryb,aaronlehmann,dependabot*,blink-so*,blinkagent*" + title: + runs-on: ubuntu-latest + if: ${{ github.event_name == 'pull_request_target' }} + steps: + - name: Validate PR title + uses: actions/github-script@ed597411d8f924073f98dfc5c65a23a2325f34cd # v8.0.0 + with: + script: | + const { pull_request } = context.payload; + const title = pull_request.title; + const repo = { owner: context.repo.owner, repo: context.repo.repo }; + + const allowedTypes = [ + "feat", "fix", "docs", "style", "refactor", + "perf", "test", "build", "ci", "chore", "revert", + ]; + const expectedFormat = `"type(scope): description" or "type: description"`; + const guidelinesLink = `See: https://github.com/coder/coder/blob/main/docs/about/contributing/CONTRIBUTING.md#commit-messages`; + const scopeHint = (type) => + `Use a broader scope or no scope (e.g., "${type}: ...") for cross-cutting changes.\n` + + guidelinesLink; + + console.log("Title: %s", title); + + // Parse conventional commit format: type(scope)!: description + const match = title.match(/^(\w+)(\(([^)]*)\))?(!)?\s*:\s*.+/); + if (!match) { + core.setFailed( + `PR title does not match conventional commit format.\n` + + `Expected: ${expectedFormat}\n` + + `Allowed types: ${allowedTypes.join(", ")}\n` + + guidelinesLink + ); + return; + } + + const type = match[1]; + const scope = match[3]; // undefined if no parentheses + + // Validate type. + if (!allowedTypes.includes(type)) { + core.setFailed( + `PR title has invalid type "${type}".\n` + + `Expected: ${expectedFormat}\n` + + `Allowed types: ${allowedTypes.join(", ")}\n` + + guidelinesLink + ); + return; + } + + // If no scope, we're done. + if (!scope) { + console.log("No scope provided, title is valid."); + return; + } + + console.log("Scope: %s", scope); + + // Fetch changed files. + const files = await github.paginate(github.rest.pulls.listFiles, { + ...repo, + pull_number: pull_request.number, + per_page: 100, + }); + const changedPaths = files.map(f => f.filename); + console.log("Changed files: %d", changedPaths.length); + + // Derive scope type from the changed files. The diff is the + // source of truth: if files exist under the scope, the path + // exists on the PR branch. No need for Contents API calls. + const isDir = changedPaths.some(f => f.startsWith(scope + "/")); + const isFile = changedPaths.some(f => f === scope); + const isStem = changedPaths.some(f => f.startsWith(scope + ".")); + + if (!isDir && !isFile && !isStem) { + core.setFailed( + `PR title scope "${scope}" does not match any files changed in this PR.\n` + + `Scopes must reference a path (directory or file stem) that contains changed files.\n` + + scopeHint(type) + ); + return; + } + + // Verify all changed files fall under the scope. + const outsideFiles = changedPaths.filter(f => { + if (isDir && f.startsWith(scope + "/")) return false; + if (f === scope) return false; + if (isStem && f.startsWith(scope + ".")) return false; + return true; + }); + + if (outsideFiles.length > 0) { + const listed = outsideFiles.map(f => " - " + f).join("\n"); + core.setFailed( + `PR title scope "${scope}" does not contain all changed files.\n` + + `Files outside scope:\n${listed}\n\n` + + scopeHint(type) + ); + return; + } + + console.log("PR title is valid."); + release-labels: runs-on: ubuntu-latest permissions: diff --git a/docs/about/contributing/CONTRIBUTING.md b/docs/about/contributing/CONTRIBUTING.md index 09934e6e42..a042f29fb9 100644 --- a/docs/about/contributing/CONTRIBUTING.md +++ b/docs/about/contributing/CONTRIBUTING.md @@ -247,8 +247,13 @@ characters long (no more than 72). Examples: -- Good: `feat(api): add feature X` -- Bad: `feat(api): added feature X` (past tense) +- Good: `feat(coderd): add feature X` +- Bad: `feat(coderd): added feature X` (past tense) + +Scopes must reference a real path in the repository (a directory or file stem) +and must contain all changed files. For example, use `coderd/database` if all +changes are within that directory. If changes span multiple top-level +directories, omit the scope. A good rule of thumb for writing good commit messages is to recite: [If applied, this commit will ...](https://reflectoring.io/meaningful-commit-messages/). @@ -263,7 +268,7 @@ to use the original commit title instead of the PR title. Breaking changes can be triggered in two ways: - Add `!` to the commit message title, e.g. - `feat(api)!: remove deprecated endpoint /test` + `feat(coderd)!: remove deprecated endpoint /test` - Add the [`release/breaking`](https://github.com/coder/coder/issues?q=sort%3Aupdated-desc+label%3Arelease%2Fbreaking) label to a PR that has, or will be, merged into `main`. diff --git a/scripts/release.sh b/scripts/release.sh index 8282863a62..6c47ab6f85 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -19,7 +19,7 @@ new patch version will be created. To mark a release as containing breaking changes, the commit title should either contain a known prefix with an exclamation mark ("feat!:", -"feat(api)!:") or the PR that was merged can be tagged with the +"feat(coderd)!:") or the PR that was merged can be tagged with the "release/breaking" label. GitHub labels that affect release notes: