Add workflow to check dependencies on PR review requests#20
Closed
BenjaminLangenakenSF wants to merge 1 commit intomainfrom
Closed
Add workflow to check dependencies on PR review requests#20BenjaminLangenakenSF wants to merge 1 commit intomainfrom
BenjaminLangenakenSF wants to merge 1 commit intomainfrom
Conversation
…liation templates on PR label 'code-review'
Benjvandam
reviewed
Mar 23, 2026
Comment on lines
+103
to
+107
| - name: Setup Node and Silverfin CLI | ||
| if: steps.handles.outputs.handles_json != '[]' | ||
| run: | | ||
| npm install https://github.com/silverfin/silverfin-cli.git | ||
| node ./node_modules/silverfin-cli/bin/cli.js -V |
There was a problem hiding this comment.
Now each time the node_modules and the cli need to be installed. We could consider to add a caching mechanism so that these actions run a lot faster. That is not specific to this PR
Benjvandam
reviewed
Mar 23, 2026
| echo "Command: \`silverfin check-dependencies -h ${handle}\`" | ||
| echo "" | ||
| output=$(node ./node_modules/silverfin-cli/bin/cli.js check-dependencies -h "$handle" 2>&1) || true | ||
| exit_code=$? |
There was a problem hiding this comment.
Bug: The || true on the previous line swallows the real exit code. $? here will always be 0, so job_failed is never set to 1 and the workflow can never fail.
Suggested fix:
output=$(node ./node_modules/silverfin-cli/bin/cli.js check-dependencies -h "$handle" 2>&1) && exit_code=0 || exit_code=$?| '', | ||
| 'Ran for reconciliation templates changed in this PR (triggered by `code-review` label).', | ||
| '', | ||
| '${{ steps.run-check.outputs.results }}', |
There was a problem hiding this comment.
Script injection risk: The CLI output is interpolated directly into JavaScript via ${{ }}. If the output contains a single quote or backtick, it could break the script or execute unintended code.
Safer to pass it via an environment variable:
env:
RESULTS: ${{ steps.run-check.outputs.results }}
script: |
const results = process.env.RESULTS;
BenjaminLangenakenSF
deleted the
add_check_liquid_test_dependencies_workflow_reconciliation_texts
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
2 participants
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.
This workflow checks dependencies for changed reconciliation templates when the 'code-review' label is added to a pull request. It runs the Silverfin CLI command to validate dependencies and posts results as comments.