ci: port safe workflow fixes from release-1.10.0 to main#13401
Conversation
Reconciles the workflow/CI improvements that landed on release-1.10.0 back into main, EXCLUDING workflows that depend on release-only features not yet present on main. Ported (verified no release-only dependencies): - test-coverage-advisor.yml (new; advisory-only, fully self-contained) - ci.yml, nightly_build.yml, db-migration-validation.yml (incl. the #13249 enhancements), python_test.yml (Python 3.14 support), cross-platform-test.yml (--prerelease=if-necessary-or-explicit), lint-js.yml, py_autofix.yml, typescript_test.yml Held back (depend on features that only exist on release-1.10.0): - extension-migration-checks.yml (extensions / src/bundles / scripts/migrate) - gp-backend-check.yml (release-only scripts/gp/*.py + backend locales/en.json) - regression-stub.yml (release-only regressions/ tracking) - gp-download.yml / gp-upload.yml backend-translation steps (release-only scripts/gp/{download,upload}.py) These converge with main once release-1.10.0 ships. 3-way merged from the common ancestor; the diff was verified to introduce zero references to release-only paths.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR enhances multiple GitHub Actions workflows across frontend testing, database migration validation, and test coverage advisory. It adds a new lint-frontend CI job, improves frontend test caching and resilience, refactors database migration validation to use bearer-token authentication, introduces a new test coverage advisory workflow, and improves checkout configuration for component index updates. ChangesFrontend Testing & Linting Infrastructure
Database Migration Validation
Test Coverage Advisory Workflow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 9✅ Passed checks (9 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 |
✅ Test Coverage AdvisorNo source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/py_autofix.yml (1)
64-73:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winFork PR merge will fail due to missing base repository fetch.
The checkout now explicitly targets the PR head repository (line 66), which changes the
originremote to point to the fork for fork PRs. However, the merge operation at line 73 assumesorigin/${{ github.base_ref }}exists and is current.For fork PRs, the base branch may not exist in the fork or may be stale, causing the merge to fail.
🔧 Proposed fix to fetch base ref before merging
Add a fetch step before the merge to ensure the base ref is available:
- uses: actions/checkout@v6 with: repository: ${{ github.event.pull_request.head.repo.full_name }} ref: ${{ github.event.pull_request.head.ref }} fetch-depth: 0 + - name: Fetch base branch + run: | + git remote add upstream ${{ github.event.pull_request.base.repo.clone_url }} + git fetch upstream ${{ github.base_ref }}:refs/remotes/upstream/${{ github.base_ref }} - name: Merge base branch run: | git config user.name "github-actions[bot]" git config user.email "github-actions[bot]`@users.noreply.github.com`" - git merge origin/${{ github.base_ref }} --no-edit || { + git merge upstream/${{ github.base_ref }} --no-edit || {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/py_autofix.yml around lines 64 - 73, The merge step "Merge base branch" can fail for forked PRs because the checkout targets the PR head repo and origin may not have the base branch; before running git merge origin/${{ github.base_ref }}, fetch the base ref from the base repository and/or ensure origin has it — e.g. in the "Merge base branch" run block add a fetch like git fetch --no-tags --prune origin ${{ github.base_ref }} || git fetch --no-tags --prune ${{ github.event.pull_request.base.repo.full_name }} ${{ github.base_ref }} so that the subsequent git merge origin/${{ github.base_ref }} succeeds. Ensure this change is applied in the same "Merge base branch" step that contains the git merge command..github/workflows/lint-js.yml (1)
20-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winScope
lint-js.ymljob permissions tocontents: read.This workflow only checks out the repo and runs
git rev-parse/merge-base/diff+npx@biomejs/biomecheck; there are nogit push|commit|tagoperations and no actions that create/update PRs/releases. Sinceci.ymlinvokes./.github/workflows/lint-js.ymlwithout apermissionsoverride (andci.ymlcontains nopermissions:key), the job-levelcontents: writeis the effective token scope.🔒 Proposed change
permissions: - contents: write + contents: read🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/lint-js.yml around lines 20 - 21, Change the workflow job permissions to limit token scope: in the permissions block (the permissions: contents: write entry) for the lint-js.yml workflow, replace contents: write with contents: read so the job only needs repository read access; locate the permissions: block in .github/workflows/lint-js.yml and update the value to contents: read, keeping the existing YAML structure intact.
🧹 Nitpick comments (1)
.github/workflows/test-coverage-advisor.yml (1)
39-41: 💤 Low valueHarden
BASE_REFexpansion viaenv.
BASE_REFis interpolated directly into the script (Line 39). The PR base ref is repo-controlled so risk is low, but routing it through the stepenvkeeps the whole script free of${{ }}template injection and silences the zizmor finding.♻️ Proposed change
- name: Detect missing tests id: detect shell: bash + env: + BASE_REF: ${{ github.event.pull_request.base.ref }} run: | set -euo pipefail - - BASE_REF="${{ github.event.pull_request.base.ref }}" git fetch --no-tags --depth=50 origin "$BASE_REF" || true🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/test-coverage-advisor.yml around lines 39 - 41, The script currently interpolates the PR base ref inline via "${{ github.event.pull_request.base.ref }}" into BASE_REF and then uses it in commands like git fetch and MERGE_BASE; move this template expansion into the step's env block by defining an environment variable BASE_REF: ${{ github.event.pull_request.base.ref }} and update the script to read $BASE_REF (used by git fetch --no-tags --depth=50 origin "$BASE_REF" and MERGE_BASE=$(git merge-base HEAD "origin/$BASE_REF" || git rev-parse HEAD~1)), so the shell script no longer contains `${{ }}` template syntax and avoids template-injection warnings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/test-coverage-advisor.yml:
- Around line 99-152: The step "msg" injects PR-controlled outputs directly into
bash (e.g. ${{ steps.detect.outputs.py_src }} and ${{
steps.detect.outputs.fe_src }}) which allows command injection; pass those
outputs into the job's environment and reference safe shell vars instead: add
env entries (e.g. NEED_BE: ${{ steps.detect.outputs.need_be }}, NEED_FE: ${{
steps.detect.outputs.need_fe }}, PY_SRC: ${{ steps.detect.outputs.py_src }},
FE_SRC: ${{ steps.detect.outputs.fe_src }}) on the "Build advisory message" step
and replace direct GitHub expression expansions in the script with the shell
variables NEED_BE/NEED_FE and safe printing using printf '%s\n' "$PY_SRC" and
printf '%s\n' "$FE_SRC" (also update the earlier NEED_BE/NEED_FE uses that
currently expand via ${{ }} to use the shell vars).
- Around line 27-31: Update the workflow to harden and pin external action
references: for the actions/checkout@v6 step add persist-credentials: false
(since the job does not push) and replace the tag with the repository commit SHA
to pin the version; likewise replace peter-evans/find-comment@v3 and
peter-evans/create-or-update-comment@v4 with their respective commit SHAs used
elsewhere in the repo so all uses are hash-pinned for reproducibility and
security.
---
Outside diff comments:
In @.github/workflows/lint-js.yml:
- Around line 20-21: Change the workflow job permissions to limit token scope:
in the permissions block (the permissions: contents: write entry) for the
lint-js.yml workflow, replace contents: write with contents: read so the job
only needs repository read access; locate the permissions: block in
.github/workflows/lint-js.yml and update the value to contents: read, keeping
the existing YAML structure intact.
In @.github/workflows/py_autofix.yml:
- Around line 64-73: The merge step "Merge base branch" can fail for forked PRs
because the checkout targets the PR head repo and origin may not have the base
branch; before running git merge origin/${{ github.base_ref }}, fetch the base
ref from the base repository and/or ensure origin has it — e.g. in the "Merge
base branch" run block add a fetch like git fetch --no-tags --prune origin ${{
github.base_ref }} || git fetch --no-tags --prune ${{
github.event.pull_request.base.repo.full_name }} ${{ github.base_ref }} so that
the subsequent git merge origin/${{ github.base_ref }} succeeds. Ensure this
change is applied in the same "Merge base branch" step that contains the git
merge command.
---
Nitpick comments:
In @.github/workflows/test-coverage-advisor.yml:
- Around line 39-41: The script currently interpolates the PR base ref inline
via "${{ github.event.pull_request.base.ref }}" into BASE_REF and then uses it
in commands like git fetch and MERGE_BASE; move this template expansion into the
step's env block by defining an environment variable BASE_REF: ${{
github.event.pull_request.base.ref }} and update the script to read $BASE_REF
(used by git fetch --no-tags --depth=50 origin "$BASE_REF" and MERGE_BASE=$(git
merge-base HEAD "origin/$BASE_REF" || git rev-parse HEAD~1)), so the shell
script no longer contains `${{ }}` template syntax and avoids template-injection
warnings.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 536dfd22-d89e-4263-a921-6f44b7e8723d
📒 Files selected for processing (7)
.github/workflows/ci.yml.github/workflows/db-migration-validation.yml.github/workflows/lint-js.yml.github/workflows/nightly_build.yml.github/workflows/py_autofix.yml.github/workflows/test-coverage-advisor.yml.github/workflows/typescript_test.yml
💤 Files with no reviewable changes (1)
- .github/workflows/nightly_build.yml
| - name: Checkout code | ||
| uses: actions/checkout@v6 | ||
| with: | ||
| # Full history so merge-base against the PR target works. | ||
| fetch-depth: 0 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm the repo enforces hash-pinning (e.g. zizmor config / other workflows already pinned)
fd -t f -e yml -e yaml . .github | xargs rg -nP 'uses:\s+[\w./-]+@[0-9a-f]{40}' -l || true
fd -H 'zizmor' .github || trueRepository: langflow-ai/langflow
Length of output: 130
Harden test-coverage-advisor checkout and pin actions
actions/checkout@v6(lines 27-31) should setpersist-credentials: false(when not pushing) and be pinned to a commit SHA.- Pin
peter-evans/find-comment@v3(line 156) andpeter-evans/create-or-update-comment@v4(line 164) to commit SHAs; other workflows in this repo already use hash-pinneduses:references.
🧰 Tools
🪛 zizmor (1.25.2)
[warning] 27-31: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false
(artipacked)
[error] 28-28: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/test-coverage-advisor.yml around lines 27 - 31, Update the
workflow to harden and pin external action references: for the
actions/checkout@v6 step add persist-credentials: false (since the job does not
push) and replace the tag with the repository commit SHA to pin the version;
likewise replace peter-evans/find-comment@v3 and
peter-evans/create-or-update-comment@v4 with their respective commit SHAs used
elsewhere in the repo so all uses are hash-pinned for reproducibility and
security.
| - name: Build advisory message | ||
| id: msg | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| NEED_BE="${{ steps.detect.outputs.need_be }}" | ||
| NEED_FE="${{ steps.detect.outputs.need_fe }}" | ||
|
|
||
| BODY_FILE="$(mktemp)" | ||
| echo '<!-- test-coverage-advisor -->' > "$BODY_FILE" | ||
|
|
||
| if [ "$NEED_BE" != "true" ] && [ "$NEED_FE" != "true" ]; then | ||
| { | ||
| echo '### ✅ Test Coverage Advisor' | ||
| echo | ||
| echo 'No source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉' | ||
| echo | ||
| echo '> _Advisory check only — never blocks merge._' | ||
| } >> "$BODY_FILE" | ||
| else | ||
| { | ||
| echo '### ⚠️ Test Coverage Advisor — tests appear to be missing' | ||
| echo | ||
| echo 'This PR changes source code but does **not** add or modify any test files.' | ||
| echo 'This is an **advisory** check: it will **not** block this PR or any other job —' | ||
| echo 'please add tests if the change is testable.' | ||
| echo | ||
| } >> "$BODY_FILE" | ||
| if [ "$NEED_BE" = "true" ]; then | ||
| { | ||
| echo '#### Backend (Python) — no `test_*.py` / `tests/` change found' | ||
| echo '```' | ||
| echo '${{ steps.detect.outputs.py_src }}' | ||
| echo '```' | ||
| } >> "$BODY_FILE" | ||
| fi | ||
| if [ "$NEED_FE" = "true" ]; then | ||
| { | ||
| echo '#### Frontend (TS/TSX) — no `*.test.*` / `*.spec.*` / `tests/` change found' | ||
| echo '```' | ||
| echo '${{ steps.detect.outputs.fe_src }}' | ||
| echo '```' | ||
| } >> "$BODY_FILE" | ||
| fi | ||
| { | ||
| echo | ||
| echo '> _Advisory check only — it always passes and is never a required status._' | ||
| } >> "$BODY_FILE" | ||
| fi | ||
|
|
||
| # Mirror into the run's Job Summary (works even on fork PRs). | ||
| cat "$BODY_FILE" >> "$GITHUB_STEP_SUMMARY" | ||
|
|
||
| echo "body_file=$BODY_FILE" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
Code injection: don't expand PR-controlled outputs directly into the shell.
py_src/fe_src (Lines 131, 139) are file paths taken from the PR diff and are therefore attacker-controllable. git diff --name-only does not quote a single quote in a path, so a malicious PR adding a file like src/backend/x';malicious_cmd;'.py would break out of echo '...' and execute arbitrary commands on the runner. Lines 104–105 follow the same ${{ }}-into-bash pattern (lower risk since the values are true/false, but worth fixing for consistency). Pass these through the step env and reference them as shell variables instead.
🔒 Proposed fix
- name: Build advisory message
id: msg
shell: bash
+ env:
+ NEED_BE: ${{ steps.detect.outputs.need_be }}
+ NEED_FE: ${{ steps.detect.outputs.need_fe }}
+ PY_SRC: ${{ steps.detect.outputs.py_src }}
+ FE_SRC: ${{ steps.detect.outputs.fe_src }}
run: |
set -euo pipefail
- NEED_BE="${{ steps.detect.outputs.need_be }}"
- NEED_FE="${{ steps.detect.outputs.need_fe }}"
-
BODY_FILE="$(mktemp)" if [ "$NEED_BE" = "true" ]; then
{
echo '#### Backend (Python) — no `test_*.py` / `tests/` change found'
echo '```'
- echo '${{ steps.detect.outputs.py_src }}'
+ printf '%s\n' "$PY_SRC"
echo '```'
} >> "$BODY_FILE"
fi
if [ "$NEED_FE" = "true" ]; then
{
echo '#### Frontend (TS/TSX) — no `*.test.*` / `*.spec.*` / `tests/` change found'
echo '```'
- echo '${{ steps.detect.outputs.fe_src }}'
+ printf '%s\n' "$FE_SRC"
echo '```'
} >> "$BODY_FILE"
fi📝 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.
| - name: Build advisory message | |
| id: msg | |
| shell: bash | |
| run: | | |
| set -euo pipefail | |
| NEED_BE="${{ steps.detect.outputs.need_be }}" | |
| NEED_FE="${{ steps.detect.outputs.need_fe }}" | |
| BODY_FILE="$(mktemp)" | |
| echo '<!-- test-coverage-advisor -->' > "$BODY_FILE" | |
| if [ "$NEED_BE" != "true" ] && [ "$NEED_FE" != "true" ]; then | |
| { | |
| echo '### ✅ Test Coverage Advisor' | |
| echo | |
| echo 'No source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉' | |
| echo | |
| echo '> _Advisory check only — never blocks merge._' | |
| } >> "$BODY_FILE" | |
| else | |
| { | |
| echo '### ⚠️ Test Coverage Advisor — tests appear to be missing' | |
| echo | |
| echo 'This PR changes source code but does **not** add or modify any test files.' | |
| echo 'This is an **advisory** check: it will **not** block this PR or any other job —' | |
| echo 'please add tests if the change is testable.' | |
| echo | |
| } >> "$BODY_FILE" | |
| if [ "$NEED_BE" = "true" ]; then | |
| { | |
| echo '#### Backend (Python) — no `test_*.py` / `tests/` change found' | |
| echo '```' | |
| echo '${{ steps.detect.outputs.py_src }}' | |
| echo '```' | |
| } >> "$BODY_FILE" | |
| fi | |
| if [ "$NEED_FE" = "true" ]; then | |
| { | |
| echo '#### Frontend (TS/TSX) — no `*.test.*` / `*.spec.*` / `tests/` change found' | |
| echo '```' | |
| echo '${{ steps.detect.outputs.fe_src }}' | |
| echo '```' | |
| } >> "$BODY_FILE" | |
| fi | |
| { | |
| echo | |
| echo '> _Advisory check only — it always passes and is never a required status._' | |
| } >> "$BODY_FILE" | |
| fi | |
| # Mirror into the run's Job Summary (works even on fork PRs). | |
| cat "$BODY_FILE" >> "$GITHUB_STEP_SUMMARY" | |
| echo "body_file=$BODY_FILE" >> "$GITHUB_OUTPUT" | |
| - name: Build advisory message | |
| id: msg | |
| shell: bash | |
| env: | |
| NEED_BE: ${{ steps.detect.outputs.need_be }} | |
| NEED_FE: ${{ steps.detect.outputs.need_fe }} | |
| PY_SRC: ${{ steps.detect.outputs.py_src }} | |
| FE_SRC: ${{ steps.detect.outputs.fe_src }} | |
| run: | | |
| set -euo pipefail | |
| BODY_FILE="$(mktemp)" | |
| echo '<!-- test-coverage-advisor -->' > "$BODY_FILE" | |
| if [ "$NEED_BE" != "true" ] && [ "$NEED_FE" != "true" ]; then | |
| { | |
| echo '### ✅ Test Coverage Advisor' | |
| echo | |
| echo 'No source changes detected without accompanying tests. Thanks for keeping coverage up! 🎉' | |
| echo | |
| echo '> _Advisory check only — never blocks merge._' | |
| } >> "$BODY_FILE" | |
| else | |
| { | |
| echo '### ⚠️ Test Coverage Advisor — tests appear to be missing' | |
| echo | |
| echo 'This PR changes source code but does **not** add or modify any test files.' | |
| echo 'This is an **advisory** check: it will **not** block this PR or any other job —' | |
| echo 'please add tests if the change is testable.' | |
| echo | |
| } >> "$BODY_FILE" | |
| if [ "$NEED_BE" = "true" ]; then | |
| { | |
| echo '#### Backend (Python) — no `test_*.py` / `tests/` change found' | |
| echo ' |
🧰 Tools
🪛 zizmor (1.25.2)
[info] 104-104: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 105-105: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 131-131: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
[info] 139-139: code injection via template expansion (template-injection): may expand into attacker-controllable code
(template-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/test-coverage-advisor.yml around lines 99 - 152, The step
"msg" injects PR-controlled outputs directly into bash (e.g. ${{
steps.detect.outputs.py_src }} and ${{ steps.detect.outputs.fe_src }}) which
allows command injection; pass those outputs into the job's environment and
reference safe shell vars instead: add env entries (e.g. NEED_BE: ${{
steps.detect.outputs.need_be }}, NEED_FE: ${{ steps.detect.outputs.need_fe }},
PY_SRC: ${{ steps.detect.outputs.py_src }}, FE_SRC: ${{
steps.detect.outputs.fe_src }}) on the "Build advisory message" step and replace
direct GitHub expression expansions in the script with the shell variables
NEED_BE/NEED_FE and safe printing using printf '%s\n' "$PY_SRC" and printf
'%s\n' "$FE_SRC" (also update the earlier NEED_BE/NEED_FE uses that currently
expand via ${{ }} to use the shell vars).
The diskcache-removal change added a `cache_dir` field to the cache settings group, but the field-count guard in test_settings_composition.py (EXPECTED_FIELDS, frozen by the settings split in #13141) was never updated. As a result test_field_count_unchanged failed on main with `assert 106 == 105`. Add "cache_dir" to the snapshot so it matches the live Settings model (106 fields). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
What
Reconciles the workflow/CI improvements that landed on
release-1.10.0back intomain— excluding workflows that depend on release-only features not yet present onmain. Companion to the release-direction sync PR.3-way merged from the common ancestor; verified this PR introduces zero references to release-only paths.
Ported (no release-only dependencies)
test-coverage-advisor.yml(new) — advisory-only check, fully self-containeddb-migration-validation.yml— the feat: Add DB migration validation workflow for nightly builds #13249 enhancementsci.yml,nightly_build.yml,lint-js.yml,py_autofix.yml,typescript_test.yml— general CI fixes(
cross-platform-test.ymlandpython_test.ymlreconciled to no-ops — main was already current.)Held back (depend on features that only exist on
release-1.10.0)These reference paths/scripts that do not exist on
mainyet, so they are intentionally not ported untilrelease-1.10.0ships:extension-migration-checks.ymlsrc/lfx/.../extension/,src/bundles/,scripts/migrate/,BUNDLE_API.mdgp-backend-check.ymlscripts/gp/{bake_note_keys,extract_backend_strings}.py, backendlocales/en.jsonregression-stub.ymlregressions/tracking dirgp-download.yml/gp-upload.yml(backend-translation steps)scripts/gp/{download,upload}.pyThese will converge with
mainoncerelease-1.10.0merges.Summary by CodeRabbit
New Features
Improvements