Handle revert commits

README.md

@@ -23,6 +23,7 @@ Focuses on Git history hygiene, developer metadata constraints, and layout stand
 *   **Identity Integrity:** Validates author and committer name formats and strictly blocks generic GitHub `noreply.github.com` email addresses.
 *   **Linked GitHub Account:** Verifies that the commit author email address is registered and verified on a GitHub account, linking the commit to a valid GitHub username. Can be downgraded to a non-blocking warning or disabled entirely, see `require_linked_github_account` below.
 *   **Autosquash Compliance:** Automatically bypasses style constraints for development-phase `fixup!` and `squash!` syntax blocks.
+*   **Revert Compliance (`allow_revert`):** Accepts the subject line `git revert` generates — `Revert "<original subject>"`, its nested `Revert "Revert "<original subject>""` form, and the prefixed `<package>: Revert "..."` variant. The quoted text is copied verbatim from the reverted commit, so the prefix and lowercase rules are not applied to it, and the `Revert "..."` wrapper is excluded from the subject length limits. A revert also puts `PKG_VERSION` and `PKG_RELEASE` back to the values that preceded the reverted commit, so those restored values are accepted instead of a bump (see **PKG_RELEASE Validation** below).
 *   **Subject String Hygiene:** Enforces `<package name or prefix>: ` prefix headers, checks lowercase starting strings post-prefix, and rejects trailing periods.
 *   **Length Constraints:** Implements dual-layered (soft and hard) line width boundaries for both subject lines and description body text blocks.
 *   **Signed-off-by Check:** Ensures a consistent, properly structured `Signed-off-by:` declaration is present and matches the original author metadata.
@@ -42,7 +43,7 @@ Inspects file modification trees targeting OpenWrt build recipes:
 *   **Conffiles Tracker:** Mandates the definition of the `Package/.../conffiles` tracking macro whenever configuration file installations (`INSTALL_CONF`) are triggered.
 *   **Line Ending Sanitization:** Inspects modifications for Windows-style Carriage Returns (CRLF) to guarantee exclusive UNIX (LF) formatting compliance.
 *   **Trailing Newline Check:** Verifies that newly created or modified files end with a trailing newline character, catching the common `\ No newline at end of file` issue in diffs (customizable level: warning/error/disabled).
-*   **PKG_RELEASE Validation:** Enforces correct release values on package changes: new packages must initialize `PKG_RELEASE` to `1`, version updates must reset `PKG_RELEASE` to `1`, and modifications to package files must be accompanied by a version/release change (customizable level: warning/error/disabled).
+*   **PKG_RELEASE Validation:** Enforces correct release values on package changes: new packages must initialize `PKG_RELEASE` to `1`, version updates must reset `PKG_RELEASE` to `1`, and modifications to package files must be accompanied by a version/release change (customizable level: warning/error/disabled). Packages that only revert commits touched are exempt from the reset/initialize rules, because a revert restores the version and release of an already released state. They must still carry a release bump if the revert changes package content without touching the version or release, otherwise users would never receive it.
 *   **UCI Config Validation:** Ensures that any configuration files destined to be installed into `/etc/config/` conform to the standard OpenWrt UCI format (consisting of only `package`, `config`, `option`, `list` statements, comments, and empty lines).
 *   **PKG_NAME Reuse Prevention:** Ensures `PKG_NAME` is not reused inside `call`, `define`, and `eval` Makefile lines, requiring the literal package name instead to keep recipes readable and searchable (default true).
 
@@ -122,6 +123,7 @@ Some configuration keys offer advanced options:
 *   `check_pkg_name_reuse`: Set to `true` (default) to detect and reject reuse of the `PKG_NAME` variable in `call`, `define`, and `eval` lines, or `false` to disable.
 *   `show_force_push_tip`: Set to `true` (default) to append a helpful tip regarding how to correct validation errors using force-pushing. Set to `false` to disable.
 *   `check_openwrt_spelling`: Set to `true` (default) to validate the correct capitalization of "OpenWrt" in commit subjects and descriptions. Set to `false` to disable.
+*   `allow_revert`: Set to `true` (default) to accept the subject format produced by `git revert` (`Revert "<original subject>"`, nested reverts, and the prefixed `<package>: Revert "..."` variant) and the `PKG_VERSION`/`PKG_RELEASE` values a revert restores. Set to `false` to hold revert commits to the regular subject and release bump rules.
 *   `enable_stale_bot`: Set to `true` to enable the stale PR bot cleanup for this repository. Defaults to `false` (opt-in).
 *   `enable_labeler_yml`: Set to `true` to enable dynamic pull request labeling based on matching files in the `.github/labeler.yml` configuration file. Defaults to `false` (opt-in).
 *   `enable_issue_labeller`: Set to `true` to enable automated issue form validation and labelling (replaces the GitHub Actions `issue-labeller.yml` workflow). Defaults to `false` (opt-in).
@@ -136,6 +138,7 @@ Here is a comprehensive example containing all available toggle options:
   "check_signoff": true,
   "check_signature": true,
   "allow_autosquash": true,
+  "allow_revert": true,
   "enable_comments": true,
   "show_force_push_tip": true,
   "max_subject_len_soft": 60,

cloudflare-worker/src/config.js

@@ -6,6 +6,7 @@ export const DEFAULT_CONFIG = {
   check_signoff: true,
   check_signature: true,
   allow_autosquash: true,
+  allow_revert: true,
   enable_comments: true,
   show_force_push_tip: true,
   max_subject_len_soft: 60,

cloudflare-worker/src/validators.js

@@ -80,6 +80,32 @@ export function isVirtuallyIdentical(subject, body, pkgName) {
   return !hasMeaningfulWord;
 }
 
+// `git revert` builds the subject from the reverted commit verbatim as
+// `Revert "<original subject>"`, reverting a revert nests the wrapper another
+// level, and OpenWrt also uses a prefixed `<pkg>: Revert "<original>"` variant.
+// None of these can satisfy the regular subject rules - the package prefix sits
+// inside the quotes, `Revert` is capitalized, and the wrapper eats into the
+// length budget - and the author cannot rewrite the quoted part without losing
+// the reference to the commit being reverted.
+// Returns { prefix, original, depth } for a revert subject, null otherwise.
+export function parseRevertSubject(subject) {
+  if (typeof subject !== 'string') return null;
+
+  // `(?:<name>: )*` also covers `tools/cmake: ` and chained `toolchain: binutils: ` prefixes.
+  const outer = subject.trim().match(/^((?:[a-zA-Z0-9_-]+(?:\/[a-zA-Z0-9_-]+)?: )*)[Rr]evert "(.+)"$/);
+  if (!outer) return null;
+
+  let original = outer[2];
+  let depth = 1;
+  let nested;
+  while ((nested = original.match(/^[Rr]evert "(.+)"$/))) {
+    original = nested[1];
+    depth++;
+  }
+
+  return { prefix: outer[1], original, depth };
+}
+
 async function getSshKeyFingerprint(sigText) {
   try {
     let cleanSig = sigText.replace(/-----[a-zA-Z0-9\s]+-----/g, '');
@@ -191,42 +217,57 @@ export async function validateFormalities(fullCommit, CONFIG) {
     subject = subject.replace(/^(fixup!|squash!)\s+/, '');
   }
 
+  const revert = CONFIG.allow_revert === false ? null : parseRevertSubject(subject);
+
   if (!isAutosquash) {
     if (/^\s/.test(lines[0])) subjectErrors.push("Commit subject must not start with whitespace");
-
-    // Special case for tools/* prefix (e.g., tools/cmake: backport bootstrap fix)
-    // These use a subdirectory naming convention like tools/cmake, tools/bison, etc.
-    const toolsPrefixMatch = subject.match(/^(tools\/[a-zA-Z0-9_-]+): /);
-    if (toolsPrefixMatch) {
-      const afterPrefix = subject.replace(/^(tools\/[a-zA-Z0-9_-]+): \s*/, '');
-      if (afterPrefix.length > 0 && afterPrefix[0] === afterPrefix[0].toUpperCase() && /[a-zA-Z]/.test(afterPrefix[0])) {
-        subjectErrors.push("Commit subject must start with a lower-case word after the prefix");
-      }
-      if (subject.endsWith('.')) {
-        subjectErrors.push("Commit subject must not end with a period");
-      }
-    } else if (!/^[a-zA-Z0-9_-]+: /.test(subject)) {
-      subjectErrors.push("Commit subject must start with `<package name or prefix>: `");
-    } else {
-      const afterPrefix = subject.replace(/^[a-zA-Z0-9_-]+: \s*/, '');
-      if (afterPrefix.length > 0 && afterPrefix[0] === afterPrefix[0].toUpperCase() && /[a-zA-Z]/.test(afterPrefix[0])) {
-        subjectErrors.push("Commit subject must start with a lower-case word after the prefix");
-      }
-      if (subject.endsWith('.')) {
-        subjectErrors.push("Commit subject must not end with a period");
+
+    // The quoted part of a revert subject is copied from the reverted commit,
+    // so the prefix/lower-case/period rules apply to the original subject and
+    // not to the wrapper `git revert` generated around it.
+    if (!revert) {
+      // Special case for tools/* prefix (e.g., tools/cmake: backport bootstrap fix)
+      // These use a subdirectory naming convention like tools/cmake, tools/bison, etc.
+      const toolsPrefixMatch = subject.match(/^(tools\/[a-zA-Z0-9_-]+): /);
+      if (toolsPrefixMatch) {
+        const afterPrefix = subject.replace(/^(tools\/[a-zA-Z0-9_-]+): \s*/, '');
+        if (afterPrefix.length > 0 && afterPrefix[0] === afterPrefix[0].toUpperCase() && /[a-zA-Z]/.test(afterPrefix[0])) {
+          subjectErrors.push("Commit subject must start with a lower-case word after the prefix");
+        }
+        if (subject.endsWith('.')) {
+          subjectErrors.push("Commit subject must not end with a period");
+        }
+      } else if (!/^[a-zA-Z0-9_-]+: /.test(subject)) {
+        subjectErrors.push("Commit subject must start with `<package name or prefix>: `");
+      } else {
+        const afterPrefix = subject.replace(/^[a-zA-Z0-9_-]+: \s*/, '');
+        if (afterPrefix.length > 0 && afterPrefix[0] === afterPrefix[0].toUpperCase() && /[a-zA-Z]/.test(afterPrefix[0])) {
+          subjectErrors.push("Commit subject must start with a lower-case word after the prefix");
+        }
+        if (subject.endsWith('.')) {
+          subjectErrors.push("Commit subject must not end with a period");
+        }
       }
     }
   }
 
-  const subjectLen = lines[0].length;
+  // Measure a revert against the subject underneath the `Revert "..."` wrapper:
+  // the wrapper is generated from the reverted commit, so the author cannot
+  // shorten it without breaking the reference.
+  const subjectLen = revert ? (revert.prefix + revert.original).length : lines[0].length;
+  const lenSuffix = revert ? ' chars, excluding the `Revert "..."` wrapper' : ' chars';
   if (subjectLen > CONFIG.max_subject_len_hard) {
-    subjectErrors.push(`Subject line exceeds hard limit (${subjectLen}/${CONFIG.max_subject_len_hard} chars)`);
+    subjectErrors.push(`Subject line exceeds hard limit (${subjectLen}/${CONFIG.max_subject_len_hard}${lenSuffix})`);
   } else if (subjectLen > CONFIG.max_subject_len_soft) {
-    warnings.push(`Subject line exceeds soft limit (${subjectLen}/${CONFIG.max_subject_len_soft} chars)`);
+    warnings.push(`Subject line exceeds soft limit (${subjectLen}/${CONFIG.max_subject_len_soft}${lenSuffix})`);
   }
 
   if (subjectErrors.length === 0) {
-    successes.push(`✅ Commit subject layout and length are valid: "${lines[0]}"`);
+    if (revert) {
+      successes.push(`✅ Commit subject layout and length are valid (revert of "${revert.original}")`);
+    } else {
+      successes.push(`✅ Commit subject layout and length are valid: "${lines[0]}"`);
+    }
   } else {
     subjectErrors.forEach(err => errors.push("- " + err));
   }
@@ -496,7 +537,14 @@ export function validateMakefileContext(fullCommit, commitPatch, CONFIG, state)
         successes.push(`✅ PKG_VERSION is dynamically defined: '${newVersion}', skipping subject validation`);
       } else {
         const cleanSubject = subject.replace(/^(fixup!|squash!)\s+/, '');
-        if (!matchVersionString(cleanSubject, newVersion)) {
+        const isRevert = CONFIG.allow_revert !== false && parseRevertSubject(cleanSubject) !== null;
+        if (isRevert) {
+          // A revert restores the PKG_VERSION that preceded the reverted commit,
+          // while its subject quotes that commit (and therefore the version being
+          // undone). Requiring the restored version here would force the author
+          // away from the `git revert` subject format.
+          successes.push(`✅ Commit reverts a previous change, skipping subject validation for restored PKG_VERSION '${newVersion}'`);
+        } else if (!matchVersionString(cleanSubject, newVersion)) {
           errors.push(`- Makefile introduces PKG_VERSION '${newVersion}', but this version string is missing in the commit subject line. Please mention the new version in the subject, e.g. '<package>: update to ${newVersion}'.`);
         } else {
           successes.push(`✅ PKG_VERSION bump matches context information inside subject line (${newVersion})`);
@@ -1410,7 +1458,18 @@ export async function validatePkgReleaseBumps(commitDetails, CONFIG, fetchFileCo
   const fileChanges = {}; // filePath -> { added: [], deleted: [] }
   const candidateFiles = [];
 
+  // A revert puts PKG_VERSION and PKG_RELEASE back to the values that preceded
+  // the reverted commit, so the version moves backwards and PKG_RELEASE is
+  // whatever it was before instead of 1. Record which commits touched a file so
+  // packages changed by reverts alone can skip the bump requirements. The
+  // PR-wide patch fallback carries no per-commit message and therefore never
+  // qualifies, keeping the audit strict when the origin of a change is unknown.
+  const nonRevertedFiles = new Set();
+
   for (const item of commitDetails) {
+    const isRevertCommit = CONFIG.allow_revert !== false &&
+      parseRevertSubject((item.fullCommit?.commit?.message || '').split('\n')[0]) !== null;
+
     if (item.commitPatch) {
       const states = parseDiffFileStates(item.commitPatch);
       states.addedFiles.forEach(f => addedFiles.add(f));
@@ -1448,6 +1507,7 @@ export async function validatePkgReleaseBumps(commitDetails, CONFIG, fetchFileCo
     const files = getChangedFilesFromPatch(item.commitPatch);
     for (const file of files) {
       modifiedFiles.add(file);
+      if (!isRevertCommit) nonRevertedFiles.add(file);
       if (isHiddenOrSpecial(file)) continue;
 
       // Ignore test files that serve only within CI/CD (e.g. test.sh, test-version.sh)
@@ -1493,6 +1553,11 @@ export async function validatePkgReleaseBumps(commitDetails, CONFIG, fetchFileCo
       return empty;
     }
 
+    // Only revert commits touched this package, so its PKG_VERSION and
+    // PKG_RELEASE are back at the values that preceded the reverted commit.
+    const pkgFiles = [...modifiedFiles].filter(file => file === pkgRoot || file.startsWith(pkgRoot + '/'));
+    const isRevertOnly = pkgFiles.length > 0 && pkgFiles.every(file => !nonRevertedFiles.has(file));
+
     // OPTIMIZATION: If the Makefile itself was not modified in the PR,
     // then the version and release cannot have changed (bumped = false).
     // We can skip fetching the Makefile contents entirely!
@@ -1522,6 +1587,11 @@ export async function validatePkgReleaseBumps(commitDetails, CONFIG, fetchFileCo
       headRelease = resolveMakefileVar(headContent, 'PKG_RELEASE');
 
       if (isNew) {
+        if (isRevertOnly) {
+          // Reverting the removal of a package restores it with the PKG_RELEASE
+          // it was dropped with; it is not a new package starting from scratch.
+          return { errors: [], successes: [`✅ Package \`${pkgRoot}\` is restored by a revert with its previous PKG_RELEASE ('${headRelease || 'not defined'}')`] };
+        }
         if (headRelease !== '1') {
           return { errors: [`New package \`${pkgRoot}\` must start with PKG_RELEASE set to 1 (currently: '${headRelease || 'not defined'}')`], successes: [] };
         }
@@ -1629,6 +1699,18 @@ export async function validatePkgReleaseBumps(commitDetails, CONFIG, fetchFileCo
       };
     }
 
+    // A revert takes the package back to a state that was already released, so
+    // its version legitimately moves backwards and PKG_RELEASE keeps the value
+    // it had before the reverted commit. Requiring a reset to 1 here would ask
+    // for a version bump that a revert must not make.
+    if (isRevertOnly) {
+      const restoredVersion = headVersion || headSourceVer || headSourceDate;
+      const restoredState = restoredVersion
+        ? `PKG_VERSION '${restoredVersion}' with PKG_RELEASE '${headRelease || 'not defined'}'`
+        : `PKG_RELEASE '${headRelease || 'not defined'}'`;
+      return { errors: [], successes: [`✅ Package \`${pkgRoot}\` is reverted to ${restoredState}, matching its state before the reverted commit`] };
+    }
+
     if (versionChanged) {
       if (headRelease !== '1') {
         return { errors: [`Package \`${pkgRoot}\` version updated from '${baseVersion || baseSourceVer || baseSourceDate}' to '${headVersion || headSourceVer || headSourceDate}', but PKG_RELEASE was not reset to 1 (currently: '${headRelease || 'not defined'}')`], successes: [] };