ci(SP-4166): add reusable schema sync check workflow#5
Open
isasmendiagus wants to merge 1 commit intomainfrom
Open
ci(SP-4166): add reusable schema sync check workflow#5isasmendiagus wants to merge 1 commit intomainfrom
isasmendiagus wants to merge 1 commit intomainfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughIntroduces a reusable GitHub Actions workflow that checks whether a source schema file and a vendored target schema file are identical, accepts Changes
Sequence Diagram(s)sequenceDiagram
participant Caller as Caller Workflow
participant Runner as GitHub Runner
participant RepoA as Caller Repo (working dir)
participant SchemaRepo as External Schema Repo
Caller->>Runner: invoke schema-sync-check with inputs
Runner->>RepoA: checkout caller repo
Runner->>SchemaRepo: checkout external schema repo into .schema-source
Runner->>Runner: resolve absolute paths to source & target files
Runner->>Runner: validate both files exist (error if missing)
Runner->>Runner: run `diff -u source target`
alt no differences
Runner->>Caller: log "schemas in sync" and set result=ok
else differences
Runner->>Caller: log error, print example curl, set result=out-of-sync
alt fail-on-diff = true
Runner->>Caller: emit failing ::error:: and exit 1
else fail-on-diff = false
Runner->>Caller: emit ::warning:: (non-failing)
end
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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
🧹 Nitpick comments (2)
.github/workflows/schema-sync-check.yml (2)
28-32: Consider making the schema repository name configurable.The workflow assumes the schema repository is always named
schemaunder the same owner. This works for your current use case but limits reusability. If you want broader adoption, consider adding an optional input for the repository name.Additionally, no
refis specified, so it defaults to the repository's default branch. This is likely intentional for "source of truth" semantics, but worth noting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/schema-sync-check.yml around lines 28 - 32, Update the "Checkout schema repo" step to accept a configurable repository name instead of hardcoding repository: ${{ github.repository_owner }}/schema; add a workflow input (e.g. inputs.schema_repo or INPUT_SCHEMA_REPO) and use it in the checkout action so the value can be overridden, and optionally add an input for ref (branch/tag) so actions/checkout@v4 can use a specific ref; make sure to keep the checkout target path as .schema-source to preserve downstream references.
37-60: Avoid direct interpolation of inputs in shell scripts.Directly interpolating
${{ inputs.target-file }}into the shell script (line 39) could be risky if inputs contain shell metacharacters. Whileworkflow_callinputs typically come from trusted workflow files, it's safer to pass inputs via environment variables.♻️ Proposed fix using environment variables
- name: Compare schemas id: compare shell: bash + env: + SOURCE_FILE: ${{ inputs.source-file }} + TARGET_FILE: ${{ inputs.target-file }} + REPO_OWNER: ${{ github.repository_owner }} run: | - source_path=".schema-source/${{ inputs.source-file }}" - target_path="${{ inputs.target-file }}" + source_path=".schema-source/${SOURCE_FILE}" + target_path="${TARGET_FILE}" if [ ! -f "$source_path" ]; then - echo "::error::Source schema not found: ${{ inputs.source-file }}" + echo "::error::Source schema not found: ${SOURCE_FILE}" exit 1 fi if [ ! -f "$target_path" ]; then echo "::error::Local vendored schema not found: $target_path" exit 1 fi if diff -u "$source_path" "$target_path"; then echo "Schema is in sync." else echo "" echo "::error::Schema out of sync: $target_path" echo "" echo "To fix, run:" - echo " curl -sL https://raw.githubusercontent.com/${{ github.repository_owner }}/schema/main/${{ inputs.source-file }} -o $target_path" + echo " curl -sL https://raw.githubusercontent.com/${REPO_OWNER}/schema/main/${SOURCE_FILE} -o $target_path" echo "result=out-of-sync" >> "$GITHUB_OUTPUT" fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/schema-sync-check.yml around lines 37 - 60, The workflow directly interpolates inputs like `${{ inputs.target-file }}` into the shell script (used to build `source_path` and `target_path`), which can allow shell metacharacters to be interpreted; change the job step to pass inputs into the shell via environment variables (e.g., set SOURCE_FILE and TARGET_FILE in the step's env) and then reference those env vars inside the script when constructing `source_path` and `target_path`, and ensure any values written to `GITHUB_OUTPUT` (e.g., `result=out-of-sync`) continue to be appended safely; update the occurrences that reference `${{ inputs.source-file }}` and `${{ inputs.target-file }}` in the script to use the new env vars instead.
🤖 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/schema-sync-check.yml:
- Line 58: The curl line hardcodes "main"; add a workflow input (e.g.,
source-ref with default "main") and use that input instead of the literal "main"
in the echo/curl command (the line containing curl -sL
https://raw.githubusercontent.com/${{ github.repository_owner }}/schema/main/${{
inputs.source-file }} -o $target_path) and also use the same input when running
the checkout step (the checkout action's ref) so both checkout and the curl
fetch the same branch/ref; update references to use ${{ inputs.source-ref }} (or
similarly named input) throughout the job.
---
Nitpick comments:
In @.github/workflows/schema-sync-check.yml:
- Around line 28-32: Update the "Checkout schema repo" step to accept a
configurable repository name instead of hardcoding repository: ${{
github.repository_owner }}/schema; add a workflow input (e.g. inputs.schema_repo
or INPUT_SCHEMA_REPO) and use it in the checkout action so the value can be
overridden, and optionally add an input for ref (branch/tag) so
actions/checkout@v4 can use a specific ref; make sure to keep the checkout
target path as .schema-source to preserve downstream references.
- Around line 37-60: The workflow directly interpolates inputs like `${{
inputs.target-file }}` into the shell script (used to build `source_path` and
`target_path`), which can allow shell metacharacters to be interpreted; change
the job step to pass inputs into the shell via environment variables (e.g., set
SOURCE_FILE and TARGET_FILE in the step's env) and then reference those env vars
inside the script when constructing `source_path` and `target_path`, and ensure
any values written to `GITHUB_OUTPUT` (e.g., `result=out-of-sync`) continue to
be appended safely; update the occurrences that reference `${{
inputs.source-file }}` and `${{ inputs.target-file }}` in the script to use the
new env vars instead.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: baf837e7-af36-4486-a70f-ee934025a02a
📒 Files selected for processing (1)
.github/workflows/schema-sync-check.yml
isasmendiagus
force-pushed
the
feat/SP-4166/schema-sync-ci
branch
from
014db52 to
5719594
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
schema-sync-check.ymlthat consumer repos can call to verify their vendored schema is in sync with the source of truthsource-file,target-file,fail-on-diffcurlcommand to fixTest plan
scanoss.py) calls this workflow and CI passes when schemas are in sync🤖 Generated with Claude Code
Summary by CodeRabbit