Add reusable check_dependencies workflow (workflow_call)#22
Merged
BenjaminLangenakenSF merged 6 commits intomainfrom Mar 27, 2026
Merged
Add reusable check_dependencies workflow (workflow_call)#22BenjaminLangenakenSF merged 6 commits intomainfrom
BenjaminLangenakenSF merged 6 commits intomainfrom
Conversation
Expose as workflow_call only; includes check-auth then per-handle silverfin check-dependencies with PR comments. Callers pass triggers (pull_request labeled, workflow_dispatch) and secrets: inherit. Made-with: Cursor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a reusable GitHub Actions workflow Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 3
🧹 Nitpick comments (2)
.github/workflows/check_dependencies.yml (2)
170-173: Consider using a more unique heredoc delimiter.If CLI output happens to contain the literal string
CHECK_EOF, the output will be truncated. A UUID-style delimiter reduces this risk.Suggested improvement
- echo "results<<CHECK_EOF" >> $GITHUB_OUTPUT + delimiter="EOF_$(uuidgen || date +%s%N)" + echo "results<<$delimiter" >> $GITHUB_OUTPUT cat check_results.txt >> $GITHUB_OUTPUT - echo "CHECK_EOF" >> $GITHUB_OUTPUT + echo "$delimiter" >> $GITHUB_OUTPUT🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/check_dependencies.yml around lines 170 - 173, The heredoc uses a short delimiter "CHECK_EOF" which can collide with CLI output; change to a more unique delimiter (e.g. a UUID-style token) and use the same token for all three heredoc lines that write to $GITHUB_OUTPUT so the delimiter can't appear in check_results.txt; update the three places that reference CHECK_EOF (the echo starting delimiter, the echo closing delimiter, and any related variable writes such as job_failed) to the new unique token and ensure the opening and closing tokens match exactly.
130-132: Pin the Silverfin CLI to a specific version or commit for reproducible builds.Installing directly from the git repo without a version tag means builds can break unexpectedly if the CLI changes. Consider pinning to a release tag or commit SHA.
Example with version pinning
- npm install https://github.com/silverfin/silverfin-cli.git + npm install https://github.com/silverfin/silverfin-cli.git#v1.x.xOr use a specific commit SHA for maximum reproducibility.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/check_dependencies.yml around lines 130 - 132, The workflow currently installs the Silverfin CLI directly from the GitHub repo using the npm install command and then runs node ./node_modules/silverfin-cli/bin/cli.js -V; update this to pin the CLI to a specific release tag or commit SHA by changing the install target to a pinned reference (e.g., replace the URL used in the npm install command with one that includes a tag or commit hash) so the subsequent node ./node_modules/silverfin-cli/bin/cli.js -V call runs a reproducible, versioned package.
🤖 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/check_dependencies.yml:
- Around line 136-138: The workflow step currently writes the secret via literal
interpolation '`${{ secrets.CONFIG_JSON }}`' which breaks if the JSON contains
single quotes; instead expose the secret as an environment variable (e.g.,
CONFIG_JSON set to ${{ secrets.CONFIG_JSON }}) for that run step and write it
using the env variable with proper quoting to the target file
($HOME/.silverfin/config.json). Update the step that creates the directory and
writes the file so it references the environment variable CONFIG_JSON (not the
literal secret string) and uses double-quoting when redirecting the value into
the config.json to preserve the JSON content.
- Around line 7-8: Add an explicit inputs contract under workflow_call by
declaring inputs.pull_request_number (optional) so callers via workflow_dispatch
must supply or acknowledge that input; update any places that read
context.payload.inputs?.pull_request_number to fall back to
inputs.pull_request_number (i.e., use
context.payload.inputs?.pull_request_number ?? inputs.pull_request_number) so
the workflow works whether invoked via workflow_call or workflow_dispatch;
target the workflow_call section and all uses of pull_request_number in script
invocations and steps that currently reference
context.payload.inputs?.pull_request_number.
- Around line 67-71: The workflow currently interpolates the step output
directly into the shell via changed_files="${{ steps.pr-files.outputs.paths }}",
which is unsafe for paths with shell metacharacters; instead expose the step
output as an environment variable (e.g., CHANGED_FILES) and reference that
variable in the script (use quoted "$CHANGED_FILES") so the rest of the logic
that uses pattern, filtered_names and grep/sed/sort -u operates on the
safely-quoted value; update the job/step to set env: CHANGED_FILES: ${{
steps.pr-files.outputs.paths }} and change the assignment to use
"$CHANGED_FILES".
---
Nitpick comments:
In @.github/workflows/check_dependencies.yml:
- Around line 170-173: The heredoc uses a short delimiter "CHECK_EOF" which can
collide with CLI output; change to a more unique delimiter (e.g. a UUID-style
token) and use the same token for all three heredoc lines that write to
$GITHUB_OUTPUT so the delimiter can't appear in check_results.txt; update the
three places that reference CHECK_EOF (the echo starting delimiter, the echo
closing delimiter, and any related variable writes such as job_failed) to the
new unique token and ensure the opening and closing tokens match exactly.
- Around line 130-132: The workflow currently installs the Silverfin CLI
directly from the GitHub repo using the npm install command and then runs node
./node_modules/silverfin-cli/bin/cli.js -V; update this to pin the CLI to a
specific release tag or commit SHA by changing the install target to a pinned
reference (e.g., replace the URL used in the npm install command with one that
includes a tag or commit hash) so the subsequent node
./node_modules/silverfin-cli/bin/cli.js -V call runs a reproducible, versioned
package.
🪄 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: cb7d50da-e1e7-4a07-8d84-61c5d945daf8
📒 Files selected for processing (1)
.github/workflows/check_dependencies.yml
check_auth refreshes CONFIG_JSON via gh secret set, which emails admins. silverfin check-dependencies only reads local Liquid Test YAML and does not need API config or tokens. Made-with: Cursor
Optional string input documents the caller contract; scripts use payload.pull_request, payload.inputs, then WORKFLOW_CALL_PR_NUMBER from inputs. Made-with: Cursor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
.github/workflows/check_dependencies.yml (1)
60-64:⚠️ Potential issue | 🟠 MajorAvoid direct step-output interpolation inside shell script.
changed_files="${{ steps.pr-files.outputs.paths }}"can be broken by shell metacharacters in filenames. Use an env variable and quote it.Proposed fix
- name: Get reconciliation handles to check id: handles + env: + CHANGED_FILES: ${{ steps.pr-files.outputs.paths }} run: | - changed_files="${{ steps.pr-files.outputs.paths }}" + changed_files="$CHANGED_FILES" pattern="reconciliation_texts/([^/]+)/" if [ -n "$changed_files" ]; then filtered_names=($(printf "%s\n" "$changed_files" | grep -oE "$pattern" | sed "s|reconciliation_texts/||;s|/||" | sort -u))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/check_dependencies.yml around lines 60 - 64, The workflow currently assigns the step output directly into a shell variable with changed_files="${{ steps.pr-files.outputs.paths }}" which is unsafe for filenames with shell metacharacters; instead expose the output via an environment variable (e.g., CHANGED_FILES set to ${{ steps.pr-files.outputs.paths }} in the step's env) and then reference it safely in the script as a quoted variable "$CHANGED_FILES" (or use readarray/IFS to split into an array) while keeping the existing pattern, filtered_names, grep/sed pipeline and variable names (pattern, filtered_names) intact.
🧹 Nitpick comments (1)
.github/workflows/check_dependencies.yml (1)
124-125: Pin Silverfin CLI to a specific commit SHA for reproducible CI.Installing from a floating Git URL (no branch or commit specified) can change behavior between runs. Since silverfin/silverfin-cli has no tagged releases, pin to the main branch's latest commit SHA using:
npm install https://github.com/silverfin/silverfin-cli.git@<commit-sha>(replace with the actual commit SHA, e.g., from the repository's commits page).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/check_dependencies.yml around lines 124 - 125, The CI step that installs silverfin-cli currently uses a floating Git URL (npm install https://github.com/silverfin/silverfin-cli.git) which makes builds unreproducible; change that npm install invocation to pin to a specific commit SHA by appending @<commit-sha> (e.g., npm install https://github.com/silverfin/silverfin-cli.git@<commit-sha>) and keep the subsequent verification step (node ./node_modules/silverfin-cli/bin/cli.js -V) unchanged so the workflow installs the exact commit every run.
🤖 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/check_dependencies.yml:
- Around line 23-40: The workflow is checking out pr.base.sha so scans run
against the base commit; capture pr.head.sha in the "Get PR details" step (e.g.,
setOutput "head_sha" from pr.head.sha) and update the "Checkout base branch"
step to checkout that head SHA (use ref: ${{ steps.pr-details.outputs.head_sha
}}) so the check-dependencies job runs against the PR HEAD; keep the existing
base_sha output if it’s needed elsewhere but ensure actions/checkout@v4 uses the
new head_sha output.
- Around line 136-156: The loop currently uses for handle in $(echo
"$HANDLES_JSON" | jq -r ".[]") which is vulnerable to word-splitting; replace
that iteration with a safe read-from-substitution pattern so each JSON element
is read intact (e.g. use: while IFS= read -r handle; do ... done < <(jq -r '.[]'
<<< "$HANDLES_JSON") ), keeping the body that executes node
./node_modules/silverfin-cli/bin/cli.js check-dependencies -h "$handle" and the
use of exit_code, output and job_failed unchanged.
---
Duplicate comments:
In @.github/workflows/check_dependencies.yml:
- Around line 60-64: The workflow currently assigns the step output directly
into a shell variable with changed_files="${{ steps.pr-files.outputs.paths }}"
which is unsafe for filenames with shell metacharacters; instead expose the
output via an environment variable (e.g., CHANGED_FILES set to ${{
steps.pr-files.outputs.paths }} in the step's env) and then reference it safely
in the script as a quoted variable "$CHANGED_FILES" (or use readarray/IFS to
split into an array) while keeping the existing pattern, filtered_names,
grep/sed pipeline and variable names (pattern, filtered_names) intact.
---
Nitpick comments:
In @.github/workflows/check_dependencies.yml:
- Around line 124-125: The CI step that installs silverfin-cli currently uses a
floating Git URL (npm install https://github.com/silverfin/silverfin-cli.git)
which makes builds unreproducible; change that npm install invocation to pin to
a specific commit SHA by appending @<commit-sha> (e.g., npm install
https://github.com/silverfin/silverfin-cli.git@<commit-sha>) and keep the
subsequent verification step (node ./node_modules/silverfin-cli/bin/cli.js -V)
unchanged so the workflow installs the exact commit every run.
🪄 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: ab5698b9-43ee-4f2a-a93d-4a4d7314eb04
📒 Files selected for processing (1)
.github/workflows/check_dependencies.yml
Avoids embedding paths in the shell script body (metacharacter safety). Made-with: Cursor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/check_dependencies.yml (1)
45-51:⚠️ Potential issue | 🟠 MajorRun
check-dependenciesagainst PR head content, not base SHA.The workflow currently checks out
pr.base.sha, so thesilverfin check-dependenciescommand runs against pre-change files. This can miss regressions introduced by the PR or fail to find newly added templates.Proposed fix
core.setOutput("base_sha", pr.base.sha); + core.setOutput("head_sha", pr.head.sha); # Check out base branch (used for config.json / handle resolution; file list from API) - - name: Checkout base branch + - name: Checkout PR head uses: actions/checkout@v4 with: - ref: ${{ steps.pr-details.outputs.base_sha }} + ref: ${{ steps.pr-details.outputs.head_sha }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/check_dependencies.yml around lines 45 - 51, The workflow is checking out pr.base.sha so silverfin check-dependencies runs against the base content; change the checkout step ("Checkout base branch" / uses: actions/checkout@v4) to use the PR head SHA instead (replace ref: ${{ steps.pr-details.outputs.base_sha }} with ref: ${{ steps.pr-details.outputs.head_sha }} or ${{ github.event.pull_request.head.sha }}), so the "silverfin check-dependencies" step runs against the PR head content and will catch changes/new templates introduced by the PR.
🧹 Nitpick comments (1)
.github/workflows/check_dependencies.yml (1)
140-144: Consider pinning the CLI version for reproducibility.Installing from the default branch (
https://github.com/silverfin/silverfin-cli.git) means workflow behavior may change unexpectedly when the CLI is updated. Consider pinning to a specific commit or tag if reproducibility is important.- npm install https://github.com/silverfin/silverfin-cli.git + npm install https://github.com/silverfin/silverfin-cli.git#v1.2.3 # pin to specific version🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/check_dependencies.yml around lines 140 - 144, The workflow step "Setup Node and Silverfin CLI" currently installs the CLI from the repository default branch (npm install https://github.com/silverfin/silverfin-cli.git) which is not reproducible; change that install line to pin a specific tag or commit (e.g. append #<tag-or-commit>) and keep the verification invocation (node ./node_modules/silverfin-cli/bin/cli.js -V) so the workflow uses a fixed known CLI version and still validates the install.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In @.github/workflows/check_dependencies.yml:
- Around line 45-51: The workflow is checking out pr.base.sha so silverfin
check-dependencies runs against the base content; change the checkout step
("Checkout base branch" / uses: actions/checkout@v4) to use the PR head SHA
instead (replace ref: ${{ steps.pr-details.outputs.base_sha }} with ref: ${{
steps.pr-details.outputs.head_sha }} or ${{ github.event.pull_request.head.sha
}}), so the "silverfin check-dependencies" step runs against the PR head content
and will catch changes/new templates introduced by the PR.
---
Nitpick comments:
In @.github/workflows/check_dependencies.yml:
- Around line 140-144: The workflow step "Setup Node and Silverfin CLI"
currently installs the CLI from the repository default branch (npm install
https://github.com/silverfin/silverfin-cli.git) which is not reproducible;
change that install line to pin a specific tag or commit (e.g. append
#<tag-or-commit>) and keep the verification invocation (node
./node_modules/silverfin-cli/bin/cli.js -V) so the workflow uses a fixed known
CLI version and still validates the install.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 84343e9c-9ff3-49e5-99d1-bf482e9c602e
📒 Files selected for processing (1)
.github/workflows/check_dependencies.yml
Resolve handles and config.json from the PR branch, not the merge base. Keep base_sha output for potential future use. Made-with: Cursor
Made-with: Cursor
Contributor
Author
|
This code was already approved in below PR: 
|
BenjaminLangenakenSF
deleted the
add-check-dependencies-reusable-workflow
branch
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
Adds
.github/workflows/check_dependencies.ymlas a reusable workflow (on: workflow_callonly).check_auththen checks changed reconciliation templates from the caller repo (samegithub.eventasrun_tests/check_tests).code-review,workflow_dispatch) stay in consumer repos; this file holds the implementation.Merge order
Merge this PR before updating consumer repos that reference
@main(e.g. be_market PR delegating to this workflow).Related
Pairs with the be_market PR that replaces the inline workflow with
uses: silverfin/bso_github_actions/.github/workflows/check_dependencies.yml@main.