ci(pr-title): fail PRs carrying changes their title doesn't cover - #10029
ci(pr-title): fail PRs carrying changes their title doesn't cover#10029bengl wants to merge 1 commit into
Conversation
Overall package sizeSelf size: 8.42 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.3.3 | 125.43 kB | 445.14 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
|
✅ All CI checks and tests passed. 🎉 All green!🧪 All tests passed 🎯 Code Coverage (details) 🔗 Commit SHA: adf151d | Docs | View more details | Give us feedback! |
BenchmarksBenchmark execution time: 2026-08-27 18:04:20 Comparing candidate commit adf151d in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2302 metrics, 8 unstable metrics.
|
AGENTS.md asks that everything in a PR serve its title, but nothing enforced it, so drive-by refactors, stray formatting, unrequested file moves, and development leftovers kept landing alongside scoped changes. Add scripts/pr-scope.js and a step in the existing pull-request-title workflow that fails on four deterministic proxies for that creep: - a commit whose conventional type or scope is outside the title's, with test, docs, ci, bench, and chore treated as supporting any change - a modified file whose hunks differ only in whitespace, unless the title is a style or chore change - a pure rename with no content change under a title that isn't about moving files - an added file matching a development-leftover pattern (root-level reports and logs, agent-tool state directories, scratch and tmp files, .orig/.rej leftovers, crash dumps) The workflow step is a one-line call into reportScopeViolations, so the rules, the API calls, and the annotations all live in a linted, tested module rather than in YAML. The checks read evidence the author already supplied rather than judging intent, so they can't see an unrelated fix buried in an otherwise relevant hunk. The scope-exception label skips them when a violation is deliberate.
bf9bb8e to
adf151d
Compare
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: adf151da16
ℹ️ 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".
| # can't see semantic creep inside an otherwise relevant hunk. Apply the scope-exception label when a | ||
| # violation is deliberate, and say why in the PR description. |
There was a problem hiding this comment.
The documented escape hatch does not work when the label is applied by itself: this workflow only subscribes to opened, edited, reopened, and synchronize, so adding scope-exception after a failed run never executes this step again. Conversely, removing the label after a successful run leaves the scope check green. Include the labeled and unlabeled activity types so the required check reflects the current label state.
Useful? React with 👍 / 👎.
| if (added.length === 0 || added.length !== removed.length) return false | ||
| return added.every((line, index) => line === removed[index]) |
There was a problem hiding this comment.
When a block is relocated within the same file without reordering its lines, the patch's removed and added sequences are identical after normalization, so this comparison returns true. A dedicated refactor(core): move helper near caller PR is consequently reported as a formatting-only violation because refactor is not in FORMATTING_TITLE_TYPES; compare hunk locations or otherwise exclude delete/add relocations before classifying the whole file as whitespace-only.
Useful? React with 👍 / 👎.
| // Development leftovers. Anything matching these is a scratch artifact no PR title covers. | ||
| const SCRATCH_PATTERNS = [ | ||
| /^[^/]+\.(?:html|log)$/, // Reports and logs dropped at the repository root | ||
| /(?:^|\/)\.(?:claude|cursor|aider|pi-subagents|agents-local)\//, // Agent-tool state directories |
There was a problem hiding this comment.
This pattern treats every newly added path under .claude or .cursor as a development leftover, but this repository already intentionally tracks shared configuration in .cursor/commands, .cursor/skills, and .claude/skills. A PR adding another shared command or skill symlink is therefore unconditionally failed as scratch-artifact; narrow the match to actual local-state files and directories rather than rejecting these entire repository-owned configuration trees.
Useful? React with 👍 / 👎.
| continue | ||
| } | ||
|
|
||
| if (title.scope && commit.scope && commit.scope !== title.scope) { |
There was a problem hiding this comment.
The repository already treats comma-delimited Conventional Commit scopes as multiple scopes, and its history contains titles such as feat(mysql,mysql2). With that title and granular feat(mysql) and feat(mysql2) commits, this exact string comparison flags both commits even though each scope is explicitly covered by the title; reuse the existing scope-splitting convention, normalize both lists, and only reject commit scopes outside the title's set.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #10029 +/- ##
========================================
Coverage 98.58% 98.58%
========================================
Files 990 990
Lines 149247 149247
Branches 13046 12712 -334
========================================
Hits 147130 147130
Misses 2117 2117 Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
1 participant
What does this PR do?
Adds a
Validate PR scopestep to the existingPull Request Titleworkflow, backed by a newscripts/pr-scope.jsmodule, that fails a PR carrying changes its title doesn't cover. Fourdeterministic rules:
mixed-commit-typetest,docs,ci,bench,chorecommits support any changemixed-commit-scopeformatting-only-filestyle,choretitlesrename-only-filestyle,chore,refactor,build,ci,testtitlesscratch-artifactscratch-*/tmp.*,.orig/.rej, crash dump)Applying the
scope-exceptionlabel skips the whole step, for the cases where a violation is deliberate.Motivation
AGENTS.md(in #10028) asks that everything in a PR serve its title, but nothing enforced it, so drive-byrefactors, stray reformatting, unrequested file moves and development leftovers kept riding along with
scoped changes — each individually defensible, collectively inflating review surface and coupling
unrelated risk into one revert unit.
Every rule keys off evidence the author already supplied (commit types, file status, whitespace-only
hunks) rather than judging intent, so false positives are limited to cases the label covers. The
deliberate gap: an unrelated one-line fix inside a file the PR legitimately touches, committed under the
right type, passes all five rules. Catching that needs a semantic reviewer, not this check.
Additional Notes
conventional-commitjob, so it reuses that required check rather thanadding one to
all-green.PR_TITLE_PATTERNand the module's parser are pinned to each other by a test.pull_request_targetand checks out the base revision, so this step never executesPR-authored code — which also means the rules here only take effect for PRs opened after this merges,
including this one.
scope-exceptionlabel to be created in repo settings before the escape hatch works;until then the step can't be bypassed.
npm run test:scripts(scripts/pr-scope.spec.mjs, 15 cases; existingscripts/pr-title.spec.mjsstill passes). The new spec also compiles the inline workflow script to catch syntax errors locally.
Scope disciplinesection ofAGENTS.mdonce docs(agents): require PR contents to serve the PR title #10028 lands(kept out of here to avoid a conflicting edit to the same section).
rename-only-filerelies on the files API reportingchanges: 0for a pure rename, verified againstrefactor(appsec): slim down appsec index file #9670, which contains both shapes:
appsec/blocked_templates.js->appsec/blocking/templates.jsisreported as
status: renamed, changes: 0with nopatch, whileappsec/blocking.js->appsec/blocking/index.jsisstatus: renamed, changes: 32with a patch. The rule fires on the formerand ignores the latter. When a move falls below GitHub's rename-similarity threshold it is reported as an
add plus a delete instead, and the rule stays silent — it fails open, never falsely.