Skip to content

ci: Add codeowners-coverage to precommit#112330

Closed
ryan953 wants to merge 4 commits intomasterfrom
ryan953/codeowner-coverage-in-precommit
Closed

ci: Add codeowners-coverage to precommit#112330
ryan953 wants to merge 4 commits intomasterfrom
ryan953/codeowner-coverage-in-precommit

Conversation

@ryan953
Copy link
Copy Markdown
Member

@ryan953 ryan953 commented Apr 7, 2026

If we put this in pre-commit maybe people will be able to keep it up to date more easily.

Ideally thought the suggest command is something that can be more flexible and automatic. If it were to find whatever agent is available on your system and run automatically then people wouldn't need to setup ollama and the check could fix itself if it was failing.
https://pypi.org/project/codeowners-coverage/
https://github.com/getsentry/codeowners-coverage

@ryan953 ryan953 requested review from a team as code owners April 7, 2026 04:50
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 7, 2026
Comment thread .pre-commit-config.yaml Outdated
Comment thread .pre-commit-config.yaml Outdated
Comment thread .pre-commit-config.yaml Outdated
Comment thread .pre-commit-config.yaml Outdated
@ryan953 ryan953 force-pushed the ryan953/codeowner-coverage-in-precommit branch from 517bee5 to fc32487 Compare April 7, 2026 05:01
Comment thread .pre-commit-config.yaml
Comment on lines +79 to +83
- id: codeowner-coverage
name: codeowner coverage
entry: .venv/bin/codeowners-coverage check
pass_filenames: false
language: system
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The codeowner-coverage pre-commit hook is missing always_run: true, causing it to be skipped on commits where no files are staged, thus bypassing the check.
Severity: MEDIUM

Suggested Fix

Add always_run: true to the codeowner-coverage hook configuration in .pre-commit-config.yaml. This will ensure the hook runs on every commit, regardless of the files being changed, maintaining consistent code ownership validation.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: .pre-commit-config.yaml#L79-L83

Potential issue: The new `codeowner-coverage` pre-commit hook is configured without the
`always_run: true` property. According to the pre-commit framework's behavior, a hook
that does not specify a `files` pattern and lacks `always_run: true` will be skipped
during commits where no files are staged. Since the purpose of the `codeowners-coverage
check` is to perform a repository-wide validation on every commit, this omission means
the check will not run reliably. This defeats the purpose of the hook, which is intended
to replace a GitHub Actions workflow that previously ran unconditionally.

Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 6570eb4. Configure here.

Comment thread .pre-commit-config.yaml

- id: codeowner-coverage
name: codeowner coverage
entry: .venv/bin/codeowners-coverage check
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing --allow-dirty-baseline flag in pre-commit hook

Medium Severity

The old CI workflow used codeowners-coverage check --allow-dirty-baseline, but the new pre-commit hook entry only runs codeowners-coverage check without that flag. The --allow-dirty-baseline flag permits the check to pass when the baseline file (.github/codeowners-coverage-baseline.txt) has uncommitted changes. In a pre-commit context, this is especially important — when a developer modifies the baseline file and stages it, the hook runs before the commit is finalized, so the baseline is inherently "dirty" and the check will fail.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 6570eb4. Configure here.

Comment thread .pre-commit-config.yaml
Comment on lines +79 to +83
- id: codeowner-coverage
name: codeowner coverage
entry: .venv/bin/codeowners-coverage check
pass_filenames: false
language: system
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The codeowner-coverage pre-commit hook is missing always_run: true, causing it to be skipped on pull requests that only contain deleted files.
Severity: MEDIUM

Suggested Fix

Add always_run: true to the codeowner-coverage hook configuration in .pre-commit-config.yaml. This will ensure the hook runs on every commit, regardless of the files changed, which is necessary for a global coverage check.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: .pre-commit-config.yaml#L79-L83

Potential issue: The `codeowner-coverage` pre-commit hook is configured with
`pass_filenames: false` but lacks `always_run: true`. The associated CI workflow is
designed to run pre-commit only on added or modified files. Consequently, if a pull
request contains only file deletions, the list of files to check is empty, and the
`codeowner-coverage` hook is silently skipped. This is a regression, as it replaces a
dedicated workflow that likely ran unconditionally, and it allows a reduction in
CODEOWNERS coverage to go undetected in pull requests that only remove files.

@ryan953 ryan953 closed this Apr 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant