Skip to content

Commit-format violations merged unaddressed despite COMMITS.md 'required change' language #107

Description

@fullsend-ai-retro

What happened

On PR #90, all 3 commits and the PR title used fix(ci):, which is a forbidden type+scope combination per COMMITS.md. The review agent correctly identified this in all 4 review runs, rating it medium severity and citing COMMITS.md line 112: 'Flag violations as a required change — they are not cosmetic.' Despite this, the verdict was comment-only (not request-changes) because the pr-review skill step 6f maps 1 medium finding to comment-only. The human reviewer (rh-hemartin approved) without addressing the finding, and the PR was merged with the forbidden prefix intact. The review agent never escalated despite its own text quoting COMMITS.md's 'required change' language.

What could go better

There is a gap between what COMMITS.md says (commit-format violations are 'required changes' that reviewers must flag) and how the review agent's verdict logic treats them (1 medium finding = comment-only = non-blocking). The review agent correctly found the violation and correctly cited COMMITS.md's language, but its verdict did not match the severity the repo's conventions prescribe. This is related to open issue #45 (verdict inconsistency) but addresses a different scenario: #45 covers multiple-medium-finding cases where text and verdict diverge, while this covers a single-medium finding where the repo's commit conventions explicitly demand blocking enforcement. Confidence is medium — the fix requires choosing between elevating the finding's severity and adding a convention-aware verdict rule.

Proposed change

Two options, either of which would close the gap: (A) In the pr-review skill step 6f verdict criteria, add a special case: when a finding's category is commit-format and the finding cites COMMITS.md's 'required change' language, treat it as equivalent to a high finding for verdict purposes (i.e., force request-changes even if it's the only medium finding). (B) In the code-review skill or the style-conventions sub-agent, escalate commit-format violations that match COMMITS.md's 'Forbidden type + scope combinations' table from medium to high severity, making them inherently blocking under the existing verdict logic. Option B is simpler and more consistent — high severity directly maps to request-changes in step 6f without adding verdict special cases.

Validation criteria

On a future PR where commit messages use a forbidden type+scope combination from COMMITS.md (e.g., fix(ci), feat(ci), fix(e2e), feat(e2e)), the review agent should produce a request-changes verdict rather than comment-only. The finding should block merge until the commit format is corrected.


Generated by retro agent from #90

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions