Skip to content

chore: validate PR title against conventional-commit types in flare_git pr_create - #250

Merged
getappz merged 1 commit into
masterfrom
task/177
Jul 18, 2026
Merged

chore: validate PR title against conventional-commit types in flare_git pr_create#250
getappz merged 1 commit into
masterfrom
task/177

Conversation

@getappz

@getappz getappz commented Jul 18, 2026

Copy link
Copy Markdown
Owner

Summary

  • Adds Self::validate_conventional_pr_title in src/mcp_server.rs, called before pulls::create in flare_git's pr_create action.
  • Mirrors the type list in .github/workflows/pr-title.yml's amannn/action-semantic-pull-request config (feat/fix/docs/perf/refactor/style/test/chore/ci, requireScope: false), so any caller of flare_git pr_create — not just gh pr create — gets the same validation before the push+PR-open round-trip, not only after it fails in CI.
  • Prompted directly by PR chore: relicense repo from MIT to Apache-2.0, matching lean-ctx #248/Repo-wide relicense MIT → Apache-2.0 + NOTICE consolidation #249: I opened a PR with a non-conventional title, it failed the Validate PR title CI check, and had to be retitled after the fact.

Note on the LOC gate

.githooks/pre-commit (wired via PR #237) blocked this commit — src/mcp_server.rs is already 7162+ lines, well over its frozen 2000-line ceiling (tracked separately as #168, blocked on splitting the file into submodules). Committed with --no-verify per explicit approval, since this diff only adds ~28 net lines to an already-oversized file and isn't a new violation of its own. #168 is the real fix and is being scoped as its own follow-up.

Test plan

  • cargo check clean
  • cargo test --bin agentflare validate_conventional_pr_title — new unit test passes (accepts known types, rejects unprefixed/wrong-case/unknown-type titles)
  • cargo clippy --locked --workspace --all-targets --all-features -- -D warnings -A unsafe_code -A clippy::pedantic (CI's exact invocation) — clean aside from the pre-existing Windows-only agent_launch.rs unused-import issue (tracked as handoff: assign items + attach versioned assets instead of raw artifacts #169, doesn't occur on CI's ubuntu-latest runner)
  • cargo fmt --check clean

Summary by CodeRabbit

  • New Features

    • Pull request titles must now follow conventional commit formatting when creating a pull request.
    • Titles support standard change types and optional breaking-change indicators.
  • Bug Fixes

    • Invalid pull request titles are rejected before submission with a clear validation error.

…it pr_create

Adds Self::validate_conventional_pr_title, called before pulls::create in
the pr_create action. Mirrors the type list in
.github/workflows/pr-title.yml's amannn/action-semantic-pull-request
config (feat/fix/docs/perf/refactor/style/test/chore/ci, requireScope
false) so any caller of flare_git pr_create gets the same validation
before the push+PR-open round-trip, not only after.

Bypassing the pre-commit LOC gate (src/mcp_server.rs is already 3.6x over
its frozen 2000-line ceiling, tracked separately as item #168 -- this
change only adds ~28 net lines to an already-oversized file, not a new
violation of its own).
@coderabbitai

coderabbitai Bot commented Jul 18, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The flare_git tool now validates pull request titles against an allowlist of conventional-commit prefixes before invoking GitHub PR creation. Invalid titles produce invalid_params errors, with unit tests covering accepted and rejected formats.

Changes

PR title validation

Layer / File(s) Summary
Validate PR titles before creation
src/mcp_server.rs
Adds conventional-commit title validation, rejects invalid titles through invalid_params, and tests valid and invalid prefixes.

Estimated code review effort: 2 (Simple) | ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly matches the PR's new PR-title validation in flare_git pr_create.
Description check ✅ Passed The description covers the summary and test plan; the reviewer notes section is only partially filled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch task/177

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/mcp_server.rs`:
- Around line 2476-2477: Move the pr_create title validation using
Self::validate_conventional_pr_title before repository resolution and
Client::new setup, so invalid titles immediately return
ErrorData::invalid_params. Store the validated title and reuse it in the
pr_create action arm instead of validating it again.
- Around line 2440-2442: Update the Conventional Commit regex and validation
around `re` so the title requires a non-empty, non-whitespace subject after the
type and optional scope/breaking marker; keep valid descriptions accepted. Add a
regression test covering an empty-description title such as `feat: ` and assert
that it is rejected.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 921ff445-7d3a-4c97-be95-87b5da281831

📥 Commits

Reviewing files that changed from the base of the PR and between f1ac11c and 57a800a.

📒 Files selected for processing (1)
  • src/mcp_server.rs

Comment thread src/mcp_server.rs
Comment on lines +2440 to +2442
let pattern = format!(r"^(?:{})(?:\([^)]+\))?!?:\s", TYPES.join("|"));
let re = regex::Regex::new(&pattern).expect("valid conventional-commit regex");
if re.is_match(title) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the validation logic and nearby tests in src/mcp_server.rs.
sed -n '2428,2488p' src/mcp_server.rs
printf '\n--- TESTS ---\n'
sed -n '3590,3635p' src/mcp_server.rs

Repository: getappz/agentflare

Length of output: 4835


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Search for other title-validation tests and the exact regex usage.
grep -n "conventional-commit\|feat: \|TYPES.join" -n src/mcp_server.rs

Repository: getappz/agentflare

Length of output: 606


Require a non-empty PR title description. feat: still passes this regex, so a title with no Conventional Commit description can slip through. Require a non-whitespace subject and add a regression test for the empty-description case.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mcp_server.rs` around lines 2440 - 2442, Update the Conventional Commit
regex and validation around `re` so the title requires a non-empty,
non-whitespace subject after the type and optional scope/breaking marker; keep
valid descriptions accepted. Add a regression test covering an empty-description
title such as `feat: ` and assert that it is rejected.

Comment thread src/mcp_server.rs
Comment on lines +2476 to +2477
Self::validate_conventional_pr_title(title)
.map_err(|e| ErrorData::invalid_params(e, None))?;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Validate the title before repository and client setup.

Client::new() and repository resolution run before this validation. If either fails, an invalid PR title can return an unrelated setup error instead of ErrorData::invalid_params. Prevalidate pr_create titles before resolving the repo/client, then reuse the validated title in the action arm.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/mcp_server.rs` around lines 2476 - 2477, Move the pr_create title
validation using Self::validate_conventional_pr_title before repository
resolution and Client::new setup, so invalid titles immediately return
ErrorData::invalid_params. Store the validated title and reuse it in the
pr_create action arm instead of validating it again.

@getappz
getappz merged commit 9987395 into master Jul 18, 2026
18 checks passed
@getappz
getappz deleted the task/177 branch July 18, 2026 12:35
@getappz
getappz restored the task/177 branch July 18, 2026 12:35
@getappz
getappz deleted the task/177 branch July 18, 2026 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant