ci: add Swagger freshness check on proto changes#318
Conversation
Add a GitHub Actions workflow that runs 'make proto-swagger-gen' and fails if the generated swagger.yaml or swagger.json differ from the committed versions. Triggers on PRs touching proto/, client/docs/, or query/tx handler files, and on pushes to main. Closes KiiChain#66
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new GitHub Actions workflow Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/swagger-check.yml:
- Around line 14-16: push path filters in swagger-check.yml only include
"proto/**" and "client/docs/**", so changes to module handler files (e.g.,
query.go and tx.go under x/**/) can skip the Swagger check; update the
workflow's push.paths configuration to also match module handler sources by
adding patterns that include x/**/query.go and x/**/tx.go (and any other
handler/source patterns your modules use) so commits touching those files
trigger the Swagger artifact validation.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bbc37580-8ecd-479b-9a35-f67ea953f891
📒 Files selected for processing (2)
.github/workflows/swagger-check.ymlCHANGELOG.md
| paths: | ||
| - "proto/**" | ||
| - "client/docs/**" |
There was a problem hiding this comment.
push path filters miss module handler changes that can affect Swagger output.
On Line 14-16, pushes to main only trigger for proto/** and client/docs/**. A direct push changing x/**/query.go or x/**/tx.go will skip this check and can let stale Swagger artifacts land.
Suggested patch
push:
branches:
- main
paths:
- "proto/**"
- "client/docs/**"
+ - "x/**/query.go"
+ - "x/**/tx.go"
+ - ".github/workflows/swagger-check.yml"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| paths: | |
| - "proto/**" | |
| - "client/docs/**" | |
| paths: | |
| - "proto/**" | |
| - "client/docs/**" | |
| - "x/**/query.go" | |
| - "x/**/tx.go" | |
| - ".github/workflows/swagger-check.yml" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/swagger-check.yml around lines 14 - 16, push path filters
in swagger-check.yml only include "proto/**" and "client/docs/**", so changes to
module handler files (e.g., query.go and tx.go under x/**/) can skip the Swagger
check; update the workflow's push.paths configuration to also match module
handler sources by adding patterns that include x/**/query.go and x/**/tx.go
(and any other handler/source patterns your modules use) so commits touching
those files trigger the Swagger artifact validation.
There was a problem hiding this comment.
Pull request overview
Adds a GitHub Actions workflow to enforce that generated Swagger artifacts remain in sync with protobuf/API changes, preventing stale API docs from being merged.
Changes:
- Introduces
.github/workflows/swagger-check.ymlto runmake proto-swagger-genand fail if generated Swagger files differ from what’s committed. - Updates
CHANGELOG.mdto document the new CI safeguard.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
CHANGELOG.md |
Documents the addition of the Swagger freshness CI check under “Unreleased”. |
.github/workflows/swagger-check.yml |
New CI workflow to regenerate Swagger and assert swagger.yaml/json are up to date. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - "proto/**" | ||
| - "client/docs/**" | ||
| - "x/**/query.go" | ||
| - "x/**/tx.go" | ||
| - ".github/workflows/swagger-check.yml" | ||
| push: | ||
| branches: | ||
| - main | ||
| paths: | ||
| - "proto/**" | ||
| - "client/docs/**" |
There was a problem hiding this comment.
The workflow path filters don’t include inputs that can change the generated Swagger output (e.g., go.mod/go.sum dependency updates and the generator logic in Makefile / proto/scripts). As-is, Swagger can become stale without this check running. Consider adding go.mod, go.sum, Makefile, and proto/scripts/** to the pull_request/push path lists (or otherwise ensuring the workflow runs when generation inputs change).
| - "proto/**" | |
| - "client/docs/**" | |
| - "x/**/query.go" | |
| - "x/**/tx.go" | |
| - ".github/workflows/swagger-check.yml" | |
| push: | |
| branches: | |
| - main | |
| paths: | |
| - "proto/**" | |
| - "client/docs/**" | |
| - "proto/**" | |
| - "proto/scripts/**" | |
| - "client/docs/**" | |
| - "x/**/query.go" | |
| - "x/**/tx.go" | |
| - "go.mod" | |
| - "go.sum" | |
| - "Makefile" | |
| - ".github/workflows/swagger-check.yml" | |
| push: | |
| branches: | |
| - main | |
| paths: | |
| - "proto/**" | |
| - "proto/scripts/**" | |
| - "client/docs/**" | |
| - "go.mod" | |
| - "go.sum" | |
| - "Makefile" |
| push: | ||
| branches: | ||
| - main | ||
| paths: | ||
| - "proto/**" | ||
| - "client/docs/**" | ||
|
|
There was a problem hiding this comment.
The PR description says pushes to main should also trigger on x//query.go and x//tx.go changes, but the push.paths list only includes proto/** and client/docs/**. Please align the workflow triggers with the documented behavior (either expand push.paths accordingly or update the PR description).
| - "x/**/query.go" | ||
| - "x/**/tx.go" |
There was a problem hiding this comment.
The workflow runs on changes to x//query.go and x//tx.go, but make proto-swagger-gen only scans *.proto files (and dependency protos via go.mod) during generation. This will trigger an expensive docker/go-mod step on handler-only changes that can’t affect Swagger output; consider removing these path patterns to reduce CI time, or document why they’re required.
| - "x/**/query.go" | |
| - "x/**/tx.go" |
The Docker proto-builder container cannot write to the mounted workspace volume on GitHub Actions because it runs as a non-root user different from the runner. Override protoImage to pass --user flag matching the host uid/gid.
buf tries to create /.cache for its module cache, which fails when running as a non-root user. Setting HOME=/tmp lets buf write its cache to /tmp/.cache instead.
|
Hi @jhelison — friendly ping! This PR is ready for review whenever you have a moment. All CI checks are passing. Happy to address any feedback. Thanks! |
|
Hey @Thaleszh, would love to get your eyes on this when you have a moment. Thanks! |
Summary
Add a CI workflow that automatically verifies Swagger files are up to date after proto changes, as requested in #66.
Problem
Proto file changes require running
make proto-swagger-gento regenerateclient/docs/swagger-ui/swagger.yamlandswagger.json. This step is easy to forget, leading to stale API docs.Solution
.github/workflows/swagger-check.ymlproto/,client/docs/, orx/**/query.go/x/**/tx.gofiles; pushes tomainmake proto-swagger-gen(which uses theghcr.io/cosmos/proto-builder:0.13.0Docker image) and then checks for a diff on the generated swagger filesmake proto-swagger-genand commit the resultHow it helps
Closes #66