ci: Add title checker action

[PierreQuinton]

A rule that is not enforced is not a rule.

This checks

• that the title is at most 72 chars long
• that it is a conventional commit (from label list with format cc: *)
• it can have optional (*) where * has associated label of the form package: *
• it can have optional ! with associated label breaking-change
• then follows : and a capital letter followed by the rest of the description.

It cleans up labels of those types, then adds all the associated labels to the PR and is run on edits of the PR. We could improve messages but this is not needed yet.

[ValerianRey]

@claude can you explain the regex in detail?

[claude]

Claude finished @ValerianRey's task in 29s —— View job

---

Regex explanation

The regex is on line 21 of .github/workflows/title-checker.yml:

/^([a-z-]+)(?:\(([^)]+)\))?(!?): ([A-Z].+)$/

Here is a breakdown of each part:

Part	What it matches	Captured?
^	Start of string	—
([a-z-]+)	Type: one or more lowercase letters or hyphens (e.g. feat, fix, ci, breaking-change)	Group 1 → type
(?:\(([^)]+)\))?	Scope (optional): literal (, then one or more chars that are not ), then literal )	Group 2 → scope (the (?:...) outer wrapper is non-capturing; the inner (...) captures the scope value)
(!?)	Breaking change (optional): zero or one !	Group 3 → breaking
:	Separator: literal colon + space	—
([A-Z].+)	Description: must start with a capital letter, followed by one or more of any character	Group 4 → description
$	End of string	—

In plain English, the format enforced is:

type(scope)!: Description

• type — required, e.g. feat, fix, ci
• (scope) — optional, e.g. (auth), (torchJD)
• ! — optional, signals a breaking change
• : — required separator
• Description — required, must begin with a capital letter

Valid title examples:

feat: Add new aggregation method
fix(upgrad): Correct gradient computation
ci!: Drop Python 3.9 support

One note: [^)]+ uses a negated character class — the ^ inside [...] means "any character except ) ". This ensures the scope cannot contain a closing parenthesis, but can otherwise contain spaces, dots, etc.

[ValerianRey]

I love the idea btw, this is really nice!

[ValerianRey]

@claude can you explain this part of the regex in more detail?

(?:(([^)]+)))?

In particular, I don't understand why there is a column, and why only the parentheses are optional and not the whole thing.

[claude]

Claude finished @ValerianRey's task in 42s —— View job

---

Regex explanation: (?:\(([^)]+)\))?

Let me dissect this piece by piece, from outermost to innermost:

(?:   \(   ([^)]+)   \)   )?
 │     │       │      │    │
 │     │       │      │    └─ ? makes the entire non-capturing group OPTIONAL
 │     │       │      └────── literal ) in the title (backslash-escaped)
 │     │       └───────────── capturing group: the scope value itself
 │     └───────────────────── literal ( in the title (backslash-escaped)
 └─────────────────────────── start of non-capturing group

Why the ?: (the "colon")?

In regex, parentheses (...) serve double duty: they group things AND capture the match into a numbered group. The ?: prefix opts out of capturing — so (?:...) is a "non-capturing group". It's needed here purely to group \(([^)]+)\) so that the trailing ? can make the whole group optional as a unit.

Without ?: it would be (\(([^)]+)\))?, which would work the same way mechanically, but it would shift all subsequent group numbers by 1 (since the outer group would become group 2, pushing ([^)]+) to group 3, (!?) to group 4, etc.). Using ?: keeps the capture groups clean.

Why only the parentheses are optional?

The whole thing IS optional — the ? at the very end applies to the entire (?:...) group. What it means is: the scope including its surrounding ( and ) characters is either fully present or fully absent. You can't have feat: Description (no scope, ✓) or feat(upgrad): Description (with scope, ✓), but not feat(): Description (empty scope, ✗ — [^)]+ requires at least one character).

The key source of confusion is that there are two kinds of parentheses in play:

• Regex parentheses (?:...) and (...) — these are regex syntax for grouping
• Literal parentheses \( and \) — these are backslash-escaped because they represent actual ( and ) characters that must appear in the PR title

[ValerianRey]

@claude please review

[claude]

Claude finished @ValerianRey's task in 1m 13s —— View job

---

Code Review

Overall: Solid idea, clean implementation. A few real bugs and one minor nit worth fixing before merging.

---

Bug 1: Title length limit is wrong

Line 33 — the limit is 72 but as @ValerianRey noted, GitHub appends (#NNN) when squash-merging a PR. At 3-digit PR numbers that's 7 characters; at 4-digit numbers it's 8. So the actual available space for the title is 72 - 8 = 64.

# current
if (title.length > 72) {
  core.setFailed(`PR title is too long (${title.length}/72 characters).`);

# should be
if (title.length > 64) {
  core.setFailed(`PR title is too long (${title.length}/64 characters).`);

Fix this →

---

Bug 2: Labels are never removed when the title is edited

Lines 61–68 — addLabels only appends. If a PR title changes from feat: Foo to fix: Bar, the cc: feat label stays forever. Stale labels mislead contributors and CI tooling.

Two options:

• Replace all labels each run: fetch the PR's current labels, compute which ones this workflow manages (those matching cc: *, package: *, breaking-change), remove them, then add the new ones.
• Use setLabels if you're OK with overwriting all labels (heavy-handed since labels can also be manually set).

The first option is safer. Roughly:

// get current labels on the PR
const prLabels = await github.rest.issues.listLabelsOnIssue({
  owner: context.repo.owner,
  repo: context.repo.repo,
  issue_number: pr_number,
});
const managedPrefixes = ['cc: ', 'package: ', 'breaking-change'];
const labelsToRemove = prLabels.data
  .map(l => l.name)
  .filter(n => managedPrefixes.some(p => n.startsWith(p) || n === 'breaking-change'));
for (const label of labelsToRemove) {
  await github.rest.issues.removeLabel({ owner, repo, issue_number, name: label });
}

Fix this →

---

Bug 3: breaking-change label existence is not validated

Lines 57–59 — the type and scope labels are validated against the repo's label list before being added, but breaking-change is not. If that label doesn't exist in the repo, the addLabels API call will throw a 422 error with a cryptic message. Add the same guard:

if (breaking === '!') {
  if (!labelNames.includes('breaking-change')) {
    core.setFailed('No "breaking-change" label found in repo.');
    return;
  }
  labelsToAdd.push('breaking-change');
}

Fix this →

---

Bug 4: Label list pagination

Lines 37–41 — listLabelsForRepo returns at most 30 labels by default. If the repo ever has more than 30 labels, valid labels beyond that page will not be found and the check will incorrectly fail. Add per_page: 100:

const repoLabels = await github.rest.issues.listLabelsForRepo({
  owner: context.repo.owner,
  repo: context.repo.repo,
  per_page: 100,
});

(For future-proofing, proper pagination with paginate would be ideal, but per_page: 100 is almost certainly sufficient in practice.)

Fix this →

---

Nit: contents: read permission is unnecessary

Line 11 — this workflow never checks out code or reads repository contents. Removing it reduces the token's scope slightly (principle of least privilege).

---

Non-issue (just noting)

The if (labelsToAdd.length > 0) guard on line 61 is always true at that point since typeLabel is unconditionally pushed beforehand. It's harmless, but could be removed for clarity.

---

Summary of items by priority:

#	Severity	Description
1	Must fix	Length limit 72 → 64
2	Must fix	Stale labels on title edit
3	Should fix	breaking-change label not validated
4	Should fix	Label pagination (per_page: 100)
5	Nit	Remove contents: read permission

[PierreQuinton]

@claude Can you review again, independently of the previous review?

[claude]

Claude finished @PierreQuinton's task in 2m 8s —— View job

---

Code Review (Independent)

• Read and analyze the workflow file
• Post review findings

---

Overall: Clean, focused workflow. I found two real bugs, one likely-harmless logic error in filter construction, and a couple of style nits.

---

Bug 1: Missing # in displayTitle — line 31

const displayTitle = `${title} (${pr_number})`;

GitHub squash-merges format commit titles as title (#NNN), not title (NNN). The # is missing, so the computed displayTitle is 1 character shorter than the real commit title. A title that is exactly 1 character too long will pass the check when it shouldn't.

// Fix:
const displayTitle = `${title} (#${pr_number})`;

Fix this →

---

Bug 2: labelsToRemove filter logic is wrong — line 82

.filter(n => managedPrefixes.some(p => n.startsWith(p) || n === 'breaking-change'));

The || is inside the some() callback, so n === 'breaking-change' is evaluated as a sub-condition of each prefix iteration, not as a standalone condition. This happens to work right now because managedPrefixes is non-empty (so the callback runs at least once, and true || anything short-circuits). But the intent is wrong and any future reader will be confused. More concretely: if managedPrefixes were ever made empty, breaking-change labels would silently stop being removed.

// Fix:
.filter(n => managedPrefixes.some(p => n.startsWith(p)) || n === 'breaking-change');

Fix this →

---

Minor: listLabelsOnIssue has no per_page — lines 74–78

const prLabels = await github.rest.issues.listLabelsOnIssue({
  owner,
  repo,
  issue_number: pr_number,
});

Default page size is 30. If a PR ever has more than 30 labels, some managed labels won't appear in prLabels.data and won't be cleaned up on title edits. Very unlikely in practice, but consistent with the per_page: 100 already applied to listLabelsForRepo.

Fix this →

---

Nit: description is destructured but never used — line 29

const [_, type, scope, breaking, description] = match;

description is never referenced. Either drop it or replace with _2 / __ to signal intentional discard.

---

Nit: Inconsistent owner/repo style — lines 92–94

All API calls use the destructured owner and repo from line 18, except addLabels:

await github.rest.issues.addLabels({
  owner: context.repo.owner,  // inconsistent
  repo: context.repo.repo,    // inconsistent

Should use owner, repo like the rest of the calls.

---

Summary:

#	Severity	Description
1	Should fix	Missing # in displayTitle → length check is off by 1
2	Should fix	labelsToRemove filter || placement is logically wrong
3	Minor	listLabelsOnIssue missing per_page: 100
4	Nit	Unused description variable
5	Nit	Inconsistent context.repo.owner vs owner in addLabels

[PierreQuinton]

@claude can you review one final time?

[claude]

Claude finished @PierreQuinton's task in 1m 55s —— View job

---

Final Code Review

• Read current state of workflow file
• Analyze for bugs, logic issues, and style problems
• Post review findings

---

Overall: All issues from the previous two reviews have been fixed. I found one real bug that was not previously mentioned, one minor inconsistency, and one harmless regex quirk.

---

Bug: pull_request event cannot label PRs from forks — line 3

on:
  pull_request:
    types: [opened, edited, reopened]

The pull_request event runs with restricted permissions for PRs opened from forks — even with permissions: pull-requests: write declared, GitHub drops write access for fork PRs as a security measure. In practice, when an external contributor opens a PR, the addLabels and removeLabel calls will fail with a 403.

The standard fix is to use pull_request_target instead. This runs in the context of the base repository (with full permissions) while still firing on PR events from forks. It is safe here because the script only uses github.rest API calls, not actions/checkout — so there is no risk of executing untrusted fork code:

on:
  pull_request_target:
    types: [opened, edited, reopened]

Fix this →

---

Minor: listLabelsOnIssue still missing per_page — lines 74–78

const prLabels = await github.rest.issues.listLabelsOnIssue({
  owner,
  repo,
  issue_number: pr_number,
});

listLabelsForRepo correctly uses per_page: 100, but listLabelsOnIssue does not. If a PR ever has more than 30 labels, managed labels beyond the first page won't be cleaned up on title edits. Very unlikely, but inconsistent.

Fix this →

---

Nit: Regex requires descriptions of at least 2 characters

The description group is ([A-Z].+) — [A-Z] matches one capital letter, then .+ requires one or more additional characters. So a one-character description like feat: X would fail the regex, while feat: Xy would pass. This is almost certainly intentional (descriptions should be meaningful), but worth being aware of.

---

Previously fixed (confirming all clear)

Item	Status
# added to displayTitle	✅ Fixed (line 31)
maxLength = 72 with dynamic displayTitle	✅ Correct — dynamically accounts for actual PR number length
breaking-change label validated before use	✅ Fixed (lines 66–72)
per_page: 100 on listLabelsForRepo	✅ Fixed (line 44)
Stale labels removed before adding new ones	✅ Fixed (lines 74–90)
`
contents: read permission removed	✅ Fixed
addLabels uses owner, repo consistently	✅ Fixed (lines 92–97)
Unused description → __	✅ Fixed (line 29)

---

Summary of remaining items:

#	Severity	Description
1	Should fix	Use pull_request_target so labeling works on fork PRs
2	Minor	Add per_page: 100 to listLabelsOnIssue
3	Nit	Descriptions must be ≥ 2 characters ([A-Z].+)

[PierreQuinton]

@claude can you do another PR? keep in mind that if you keep seeing stuff, I will keep asking you this. This will make @ValerianRey sad because we used all his tokens.

[claude]

Claude finished @PierreQuinton's task in 2m 3s —— View job

---

Code Review

• Read and analyze the workflow file
• Post review findings

---

Overall: All previously flagged issues have been fixed. The workflow is in good shape. I found one real bug, one worth-considering improvement, and two nits.

---

Bug: removeLabel can throw 404, failing the workflow — lines 84–91

for (const label of labelsToRemove) {
  await github.rest.issues.removeLabel({
    owner,
    repo,
    issue_number: pr_number,
    name: label
  });
}

If a managed label is removed from the PR externally (e.g., manually, or by a concurrent workflow run triggered by a rapid open + edit) between the listLabelsOnIssue fetch and this removeLabel call, the API returns a 404 which becomes an unhandled exception. The workflow fails even though the desired end state is already correct.

Fix: swallow 404s in the loop:

for (const label of labelsToRemove) {
  try {
    await github.rest.issues.removeLabel({
      owner,
      repo,
      issue_number: pr_number,
      name: label
    });
  } catch (e) {
    if (e.status !== 404) throw e;
  }
}

Fix this →

---

Minor: No concurrency control

If two workflow runs trigger in rapid succession (e.g., a PR is opened and immediately edited), they can interleave: both fetch labels, both remove, both add. The second removeLabel call may hit the 404 mentioned above (partially addressed by the fix above), but more subtly, the two addLabels calls are redundant and wasteful. Adding a concurrency group makes only the latest run matter:

jobs:
  check-title:
    runs-on: ubuntu-latest
    concurrency:
      group: title-checker-${{ github.event.pull_request.number }}
      cancel-in-progress: true

---

Nit: Scope regex allows spaces

([^)]+) matches any characters except ), including spaces. So feat(my module): Description is a syntactically valid title. If scopes are meant to be slug-style identifiers (matching package: * labels which presumably don't have spaces), consider ([^) ]+) or ([\w-]+) to prevent false positives that fail at the label-lookup stage anyway — but at least the error message would be more confusing if someone writes feat(my module).

This is mostly fine as-is since the label validation catches it, but the error message would be No label "package: my module" found in repo rather than something clearer.

---

Nit: Description requires ≥ 2 characters

[A-Z].+ — the .+ requires at least one character after the capital letter, so feat: X fails the regex while feat: Xy passes. Single-character descriptions are probably not meaningful anyway, but if this is intentional it's worth a comment in the regex.

---

Summary:

#	Severity	Description
1	Should fix	removeLabel throws 404 on concurrent/external label removal
2	Minor	No concurrency group; parallel runs can interfere
3	Nit	Scope regex allows spaces
4	Nit	[A-Z].+ silently rejects single-character descriptions

---

Note: I cannot directly implement these fixes because GitHub App permissions don't allow modifying .github/workflows/ files.

[PierreQuinton]

I wouldn't do the proposed changes, I think we are good here, @ValerianRey I let you merge so you can try it yourself if you want.

[ValerianRey]

Yeah you've fixed most important issues, so let's merge and we can always fix problems later if any.