Skip to content

ci: hooks basic checks - #3

Merged
liukatkat merged 3 commits into
katrina/check-version-hookfrom
katrina/ci
Dec 17, 2025
Merged

ci: hooks basic checks#3
liukatkat merged 3 commits into
katrina/check-version-hookfrom
katrina/ci

Conversation

@liukatkat

@liukatkat liukatkat commented Nov 24, 2025

Copy link
Copy Markdown
Collaborator

What:

This PR adds basic CI to verify that the repository’s hooks work with any Semgrep version greater than or equal to the one specified in semgrep-version.

The PR also adds CI to ensure that the plugin version is updated each time we make a change to the files within the plugin/ directory.

  • .github/workflows/test.yml is the main CI workflow file
  • tests/ contains all the scripts for testing the commands we run in the hooks

Why:

We want to ensure that the plugin remains compatible with semgrep, since some of its hooks rely on specific semgrep features. I considered bundling Semgrep directly or adopting an approach similar to the old mcp repo/ the semgrep-rpc, but neither model fits this plugin well. The main reasons are:

  1. Bundling the Semgrep binary with the plugin would effectively allow multiple Semgrep versions to coexist, which would make debugging far more difficult.
  2. Unlike other repositories with a tightly coupled release process, the plugin’s version does not need to match Semgrep’s release cadence. It’s generally forward-compatible and won’t require updates for every Semgrep release.
  3. The plugin’s main branch effectively serves as its “released” version, which is quite different from some of the other workflows we have.
  4. We can't really add semgrep as a dependency like we have done in the mcp/ semgrep-rpc repo.

Given this, I chose to use CI for enforcing compatibility by verifying that everything in main works against the specific Semgrep version defined in the repo.

Test plan:

CI passing!

@liukatkat liukatkat mentioned this pull request Nov 24, 2025

liukatkat commented Nov 24, 2025

Copy link
Copy Markdown
Collaborator Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@liukatkat liukatkat mentioned this pull request Nov 24, 2025
@liukatkat liukatkat changed the title initial setup ci Nov 24, 2025
@liukatkat
liukatkat marked this pull request as ready for review November 24, 2025 22:03
@liukatkat
liukatkat force-pushed the katrina/check-version-hook branch from 7c90b34 to 398ff60 Compare November 24, 2025 22:04
@liukatkat
liukatkat force-pushed the katrina/check-version-hook branch from 398ff60 to 669f41c Compare November 24, 2025 22:10
@liukatkat liukatkat changed the title ci ci: hooks basic checks Nov 24, 2025
@liukatkat
liukatkat force-pushed the katrina/check-version-hook branch from 669f41c to d7c853a Compare November 24, 2025 22:15
@liukatkat
liukatkat force-pushed the katrina/ci branch 2 times, most recently from 51b7647 to 5d16633 Compare November 24, 2025 23:01
@liukatkat
liukatkat force-pushed the katrina/check-version-hook branch from d7c853a to cb2d2a9 Compare November 24, 2025 23:01
@liukatkat
liukatkat force-pushed the katrina/ci branch 2 times, most recently from b29d29a to 6be7284 Compare November 24, 2025 23:37
@liukatkat
liukatkat requested a review from nmote November 24, 2025 23:55
Comment thread plugin/.claude-plugin/plugin.json
update workflow

tests and mcp

add session start hook
- name: Check if plugin files changed and version bumped
run: |
# Get the base branch (usually main)
git fetch origin ${{ github.event.pull_request.base.ref }}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we need this even though we aren't doing a shallow checkout?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I think so, because I thought we need this to get the currently version on main. I am not extremely familiar with how this is typically done though, so I am not very sure.

(I might merge this first since it is a CI thing that I can probably fix later. I want to make sure we have both the Claude Plugin ready and the Cursor hooks ready by the end of this week for people to try out. But I am more than happy to update the checks!)

@liukatkat
liukatkat merged commit bda8032 into katrina/check-version-hook Dec 17, 2025
5 checks passed
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.

2 participants