github: scripts: commit_prefix_check: add config format rules on linter

[cosmo0920]

I supported to handle config_format: prefix for committing under config_format directory.

This PR resolves the following linting error on #11345:

 ❌ Commit dfabadf4ff failed:
Subject prefix 'config_format:' does not match files changed.
Expected one of: cf_yaml:


Commit prefix validation failed.

---

Enter [N/A] in the box, if an item is not applicable to your change.

Testing
Before we can approve your change; please submit the following in a comment:

• Example configuration file for the change
• Debug log output from testing the change

• Attached Valgrind output that shows no leaks or memory corruption was found

If this is a change to packaging of containers or native binaries then please confirm it works for all targets.

• Run local packaging test showing all targets (including any new ones) build.
• Set ok-package-test label to test for all targets (requires maintainer to do).

Documentation

• Documentation required for this feature

Backporting

• Backport to latest stable release.

---

Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.

Summary by CodeRabbit

• Chores
  • Tightened commit message validation to require a single umbrella prefix for configuration-format changes (config_format:) and prevent mixing other non-build prefixes unless covered by the umbrella rule.
• Tests
  • Added tests enforcing the umbrella-prefix behavior, validating allowed config_format subjects and rejecting disallowed prefixes for configuration-format edits.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

📝 Walkthrough

Walkthrough

This change updates commit-prefix inference for src/ paths (special-casing src/fluent-bit.c, src/flb_*.c, and src/<dir>/...) and enforces a strict config_format: umbrella prefix rule: commits touching config_format files must use config_format: and cannot mix other non-build prefixes.

Changes

Cohort / File(s)	Summary
Commit Prefix Validation Logic ​.github/scripts/commit_prefix_check.py	Updated infer_prefix_from_paths to: map src/fluent-bit.c → bin:, map src/flb_*.c → derived xxx:, and map src/<dir>/... → <dir>: for multi-component paths; refactored control flow to use early continues. In validate_commit, added an umbrella rule requiring config_format: when inferred and disallowing mixing other non-build prefixes with it.
Validation Test Suite ​.github/scripts/tests/test_commit_lint.py	Added tests enforcing the config_format: umbrella: valid config_format: subject, rejections when cf_yaml: or yaml: are used for config_format files, and valid multi-file config_format: umbrella case.

Sequence Diagram(s)

(omitted — changes are linting/control logic confined to a single script and tests)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

• github: scripts: commit_linter: Handle bin prefix for fluent-bit.c  #11336: Adds special-case mapping for src/fluent-bit.c → bin: in the prefix inference logic.
• github: commit_linter: Make more flexibility for commit linter #11251: Modifies commit-prefix inference/validation logic for src/* paths and related tests.
• workflows: add new commit linter tool #11245: Updates infer_prefix_from_paths and validate_commit including config_format: umbrella behavior and associated tests.

Suggested reviewers

• niedbalski
• patrick-stephens
• celalettin1286

Poem

🐰 I hop through paths and prefixes bright,
I tuck flb_ names just out of sight,
I crown config_format with one clear rule,
Tests thump their paws — the linter’s cool,
Commits now bounce on carrot-light delight.

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly summarizes the main change: adding config_format: prefix validation rules to the commit linter script, which directly matches the changes in the raw summary.
Docstring Coverage	✅ Passed	Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• 📝 Generate docstrings

---

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 625143e and e38f2af.

📒 Files selected for processing (1)

• .github/scripts/commit_prefix_check.py

🚧 Files skipped from review as they are similar to previous changes (1)

• .github/scripts/commit_prefix_check.py

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: f8b6366650

ℹ️ 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".

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (2)

.github/scripts/commit_prefix_check.py (1)

183-193: Clarify error message or remove unused variables.

Lines 188-189 compute expected_list and expected_str but line 192 hardcodes "Expected one of: config_format:" in the error message. This creates two issues:

1. The computed variables are unused (dead code).
2. If other valid prefixes like build: are also inferred, the error message doesn't mention them, which could confuse users.

Consider either:

• Using expected_str in the message to show all valid options, or
• Removing lines 188-189 and adding a comment explaining why only config_format: is shown (if the strict enforcement is intentional regardless of other prefixes).

🔎 Option 1: Use expected_str to show all valid prefixes

 if "config_format:" in expected_lower and subj_lower != "config_format:":
     expected_list = sorted(expected)
     expected_str = ", ".join(expected_list)
     return False, (
         f"Subject prefix '{subject_prefix}' does not match files changed.\n"
-        f"Expected one of: config_format:"
+        f"Files under config_format/ require 'config_format:' prefix.\n"
+        f"All inferred prefixes: {expected_str}"
     )

🔎 Option 2: Remove unused variables and clarify intent

 if "config_format:" in expected_lower and subj_lower != "config_format:":
-    expected_list = sorted(expected)
-    expected_str = ", ".join(expected_list)
     return False, (
         f"Subject prefix '{subject_prefix}' does not match files changed.\n"
-        f"Expected one of: config_format:"
+        f"Files under config_format/ require the umbrella prefix 'config_format:'"
     )

.github/scripts/tests/test_commit_lint.py (1)

625-693: LGTM: Comprehensive test coverage for config_format strict rule.

The four new test cases effectively validate the strict config_format: prefix requirement:

1. ✅ Valid usage with umbrella prefix
2. ✅ Rejection of internal implementation prefix (cf_yaml:)
3. ✅ Rejection of generic prefix (yaml:)
4. ✅ Multiple config_format files requiring umbrella prefix

The tests are well-documented with clear docstrings explaining the intent and behavior.

Optional enhancement: Consider adding a test case for the mixed scenario where both config_format files and other component files are modified together, to explicitly document the expected behavior:

📝 Suggested test case for mixed component scenario

def test_config_format_mixed_with_other_components():
    """
    Test behavior when both config_format and non-config_format files are modified.
    
    This documents whether commits can use config_format: as an umbrella
    to cover changes in both config_format/ and other components, or if
    such changes must be split into separate commits.
    """
    commit = make_commit(
        "config_format: update yaml parser and router\n\nSigned-off-by: User",
        [
            "src/config_format/flb_cf_yaml.c",
            "src/flb_router.c"
        ]
    )
    ok, msg = validate_commit(commit)
    # Update assertion based on intended behavior:
    # assert ok is False  # if config_format should be isolated
    # assert ok is True   # if umbrella can cover mixed changes

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9191f70 and f8b6366.

📒 Files selected for processing (2)

• .github/scripts/commit_prefix_check.py
• .github/scripts/tests/test_commit_lint.py

🧰 Additional context used

🧬 Code graph analysis (1)

.github/scripts/tests/test_commit_lint.py (1)

.github/scripts/commit_prefix_check.py (1)

• validate_commit (136-238)

🔇 Additional comments (2)

.github/scripts/commit_prefix_check.py (2)

65-86: LGTM: Clear handling of src/ path patterns.

The refactored src/ handling correctly distinguishes between:

• src/fluent-bit.c → bin:
• src/flb_*.c → extracted component name
• src/<dir>/... → directory name

The use of continue statements prevents cascading else logic and makes the flow easier to follow.

---

208-208: Add test case for mixed config_format + other component scenario.

The umbrella prefix logic allows config_format: to cover multiple config_format files, which is correct. However, there is no test coverage for commits that mix config_format files with other components (e.g., ["src/config_format/flb_cf_yaml.c", "src/flb_router.c"] with subject config_format:).

The current code would allow such mixed commits due to line 212's umbrella prefix check, creating an inconsistency with the strict rule's intent. Compare:

• in_tail: + ["plugins/in_tail/tail.c", "src/flb_router.c"] → rejected (in_tail not an umbrella)
• config_format: + ["src/config_format/...", "src/flb_router.c"] → allowed (config_format is an umbrella)

Either the design should be clarified as intentional (config_format can span multiple subsystems) or a stricter constraint should be added to prevent mixing. Add a test case to explicitly validate the expected behavior.

[cosmo0920]

@coderabbitai review

[coderabbitai]

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI Agents

In @.github/scripts/commit_prefix_check.py:
- Around line 188-192: Remove the unused variables or use them in the error
message: either delete the lines that create expected_list and expected_str, or
include expected_str in the returned error; update the message in the function
that returns (False, ...) so it reads something like "Subject prefix
'{subject_prefix}' does not match files changed. Expected prefix:
{expected_str}" (use the existing expected variable values or the computed
expected_str) and ensure the wording reflects a single valid prefix instead of
"Expected one of:"; adjust references in the function that builds this error
accordingly (function handling subject_prefix validation in
commit_prefix_check.py).

🧹 Nitpick comments (1)

.github/scripts/commit_prefix_check.py (1)

199-202: Consider removing the unused exclusion.

The check for "cmakelists.txt:" on line 201 excludes a prefix that is never added to expected_lower. Only "build:" is added when CMakeLists.txt files are encountered (lines 47-48). While harmless, this check is unnecessary.

🔎 Optional cleanup

     if "config_format:" in expected_lower:
         non_build = {
             p for p in expected_lower
-            if p not in ("build:", "cmakelists.txt:")
+            if p != "build:"
         }

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f8b6366 and 625143e.

📒 Files selected for processing (1)

• .github/scripts/commit_prefix_check.py

🔇 Additional comments (2)

.github/scripts/commit_prefix_check.py (2)

65-86: Refactoring improves readability.

The explicit handling of src/ paths with early continue statements makes the logic clearer and easier to follow. The three cases are well-separated:

1. src/fluent-bit.c → bin:
2. src/flb_*.c → component prefix
3. src/<dir>/... → directory prefix

---

224-224: Correctly addresses past review feedback.

Removing config_format: from the umbrella list is appropriate. With the updated infer_prefix_from_paths logic (lines 82-84), paths under src/config_format/ directly map to config_format:, so umbrella handling is no longer needed. The strict validation rules (lines 183-210) ensure proper enforcement of the config_format: prefix.

This change resolves the concern raised in the past review comment.