Skip to content

LCORE-2499: prevent HF from downloading#1854

Merged
tisnik merged 3 commits into
lightspeed-core:mainfrom
radofuchs:HF_e2e_fix
Jun 16, 2026
Merged

LCORE-2499: prevent HF from downloading#1854
tisnik merged 3 commits into
lightspeed-core:mainfrom
radofuchs:HF_e2e_fix

Conversation

@radofuchs

@radofuchs radofuchs commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Description

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • [] End to end tests improvement
  • Benchmarks improvement

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

  • Assisted-by: (e.g., Claude, CodeRabbit, Ollama, etc., N/A if not used)
  • Generated by: (e.g., tool name and version; N/A if not used)

Related Tickets & Documents

  • Related Issue #
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • Please provide detailed steps to perform tests related to this code change.
  • How were the fix/results from this change verified? Please provide relevant screenshots or results.

Summary by CodeRabbit

  • Chores
    • Updated CI workflows to cache and pre-download the Hugging Face SentenceTransformer model to reduce E2E runtime.
    • Updated Docker Compose configurations to mount a Hugging Face cache directory into relevant containers and enable offline mode to prevent Hub update checks.

@radofuchs radofuchs requested a review from are-ces June 5, 2026 08:32
@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: cf9389ce-67f1-423c-8d19-66526e0904db

📥 Commits

Reviewing files that changed from the base of the PR and between d8dc873 and 531b09b.

📒 Files selected for processing (5)
  • .github/workflows/e2e_tests.yaml
  • .github/workflows/e2e_tests_providers.yaml
  • .github/workflows/e2e_tests_rhaiis.yaml
  • .github/workflows/e2e_tests_rhelai.yaml
  • docker-compose.yaml
💤 Files with no reviewable changes (1)
  • docker-compose.yaml
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
  • GitHub Check: mypy
  • GitHub Check: Pyright
  • GitHub Check: build-pr
  • GitHub Check: Pylinter
  • GitHub Check: spectral
  • GitHub Check: unit_tests (3.13)
  • GitHub Check: unit_tests (3.12)
  • GitHub Check: integration_tests (3.13)
  • GitHub Check: E2E: library mode / ci / group 2
  • GitHub Check: E2E: server mode / ci / group 1
  • GitHub Check: E2E: server mode / ci / group 3
  • GitHub Check: E2E: library mode / ci / group 1
  • GitHub Check: E2E: library mode / ci / group 3
  • GitHub Check: E2E Tests for Lightspeed Evaluation job
  • GitHub Check: E2E: server mode / ci / group 2
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-6-on-pull-request
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-05-06T08:35:54.687Z
Learnt from: radofuchs
Repo: lightspeed-core/lightspeed-stack PR: 1690
File: .github/workflows/e2e_tests_providers.yaml:279-285
Timestamp: 2026-05-06T08:35:54.687Z
Learning: In .github/workflows/e2e_tests_providers.yaml and related e2e workflow files, the show_logs step should not use docker compose logs with --tail or --since (i.e., keep logs unbounded). The quick connectivity test runs once immediately after container startup, so the log output is small and a log tail limit is unnecessary. If you adjust this, add a rationale comment in the workflow explaining why unbounded logs are acceptable and ensure CI behavior remains deterministic.

Applied to files:

  • .github/workflows/e2e_tests_rhaiis.yaml
  • .github/workflows/e2e_tests.yaml
  • .github/workflows/e2e_tests_providers.yaml
  • .github/workflows/e2e_tests_rhelai.yaml
🪛 zizmor (1.25.2)
.github/workflows/e2e_tests_rhaiis.yaml

[error] 136-136: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

.github/workflows/e2e_tests_providers.yaml

[error] 199-199: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

.github/workflows/e2e_tests_rhelai.yaml

[error] 150-150: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🛑 Comments failed to post (3)
.github/workflows/e2e_tests_providers.yaml (2)

199-199: ⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/e2e_tests_providers.yaml | sed -n '195,205p'

Repository: lightspeed-core/lightspeed-stack

Length of output: 508


🏁 Script executed:

# Check if the file exists and get its total line count
wc -l .github/workflows/e2e_tests_providers.yaml

Repository: lightspeed-core/lightspeed-stack

Length of output: 123


🏁 Script executed:

# Search for all actions/cache usages in the file
rg "actions/cache" .github/workflows/e2e_tests_providers.yaml -n

Repository: lightspeed-core/lightspeed-stack

Length of output: 111


🏁 Script executed:

# Check for other unpinned GitHub Actions in the file
rg "uses: [a-zA-Z0-9/_-]+@(v[0-9]|latest)" .github/workflows/e2e_tests_providers.yaml -n

Repository: lightspeed-core/lightspeed-stack

Length of output: 148


🏁 Script executed:

# Verify the suggested commit SHA exists for actions/cache v4.1.1
git ls-remote --heads --tags https://github.com/actions/cache.git | grep -E "(3624ceb22c1c5a301c8db4169662070a689d9ea8|v4\.1\.1)"

Repository: lightspeed-core/lightspeed-stack

Length of output: 134


🌐 Web query:

GitHub Actions pinning commit SHA security best practice

💡 Result:

Pinning GitHub Actions to a full-length commit SHA is a security best practice that protects against supply chain attacks, such as malicious code being injected into an existing tag [1][2][3]. Why it is a best practice: - Immutability: Tags and branch names are mutable references in Git, meaning they can be updated to point to different commits [2][4][5]. If an attacker gains control of a repository, they could overwrite a tag with malicious code, which would automatically execute in any workflow using that tag [3][6]. A full-length commit SHA uniquely identifies an exact point in the codebase and cannot be changed, ensuring that your workflow consistently runs the specific, reviewed code you expect [2][4][5]. - Policy Enforcement: GitHub supports organization and repository-level policies that enforce SHA pinning [7][4][8]. When enabled, these policies cause workflows to fail if they attempt to use actions pinned to tags or branches, helping administrators proactively limit the impact of compromised dependencies across their infrastructure [7][4]. Recommended implementation: - Use full-length SHA: Always use the full commit SHA (40 characters) rather than a shortened version [1][4][3]. - Maintain readability: Include the corresponding version tag as a comment in your workflow file to maintain readability and make it easier to track updates [3][9][8]. Example: uses: actions/checkout@11bd719 # v4.2.2 - Automate updates: To manage the maintenance burden of updating pinned SHAs, use automated dependency management tools like Renovate or Dependabot [5][3][6]. Specifically, tools like Renovate support "helpers" (e.g., helpers:pinGitHubActionDigests) that can automatically create pull requests to update your pinned SHAs while keeping the version comment current [3][6]. - Verify the source: When selecting a SHA, ensure it comes from the official action repository and not a fork [1]. Note that pinning protects against tag retargeting, but it does not protect against vulnerabilities present in the code of the specific commit you have pinned; you should still vet the actions you use [6].

Citations:


Pin actions/cache to a commit SHA.

uses: actions/cache@v4 is mutable and violates immutable action pinning requirements. Tags can be retargeted by attackers to inject malicious code.

Suggested fix
-        uses: actions/cache@v4
+        uses: actions/cache@3624ceb22c1c5a301c8db4169662070a689d9ea8 # v4.1.1
📝 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.

        uses: actions/cache@3624ceb22c1c5a301c8db4169662070a689d9ea8 # v4.1.1
🧰 Tools
🪛 zizmor (1.25.2)

[error] 199-199: 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/e2e_tests_providers.yaml at line 199, The
`actions/cache@v4` reference uses a mutable version tag which poses a security
risk. Replace the `uses: actions/cache@v4` statement with a pinned commit SHA
instead of the tag (e.g., `uses: actions/cache@<specific-commit-sha>`) to ensure
the action version is immutable and cannot be retargeted by attackers. Obtain
the correct commit SHA for the v4 release from the actions/cache repository.

Source: Linters/SAST tools


202-209: ⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for pyproject.toml and uv.lock
echo "=== Checking for pyproject.toml ==="
if [ -f "pyproject.toml" ]; then
  echo "Found pyproject.toml"
  # Search for sentence-transformers version
  grep -n "sentence-transformers" pyproject.toml || echo "Not found in pyproject.toml"
else
  echo "pyproject.toml not found"
fi

echo ""
echo "=== Checking for uv.lock ==="
if [ -f "uv.lock" ]; then
  echo "Found uv.lock"
  # Search for sentence-transformers version
  grep -A 5 "sentence-transformers" uv.lock | head -20 || echo "Not found in uv.lock"
else
  echo "uv.lock not found"
fi

echo ""
echo "=== Checking for requirements.txt or similar ==="
find . -maxdepth 2 -name "*requirements*.txt" -o -name "setup.py" -o -name "setup.cfg" | grep -v ".git"

Repository: lightspeed-core/lightspeed-stack

Length of output: 1490


🏁 Script executed:

#!/bin/bash
# Read the workflow file around lines 202-209
echo "=== Workflow file content around lines 195-220 ==="
sed -n '195,220p' .github/workflows/e2e_tests_providers.yaml

echo ""
echo "=== Full context around cache and prefetch ==="
grep -n -B 5 -A 15 "Pre-download HuggingFace embedding model" .github/workflows/e2e_tests_providers.yaml

Repository: lightspeed-core/lightspeed-stack

Length of output: 2155


Add version hash to cache key and pin sentence-transformers version for deterministic CI.

The cache key at line 202 is static and doesn't include dependency version hashes, so it won't invalidate when dependencies change. Additionally, pip install -q sentence-transformers at line 208 installs a floating version, making CI behavior non-reproducible.

Suggested fix
       - name: Cache HuggingFace embedding model
         uses: actions/cache@v4
         with:
           path: /tmp/hf-cache
-          key: hf-sentence-transformers-all-mpnet-base-v2
+          key: hf-sentence-transformers-all-mpnet-base-v2-${{ hashFiles('**/pyproject.toml', '**/uv.lock') }}
+          restore-keys: |
+            hf-sentence-transformers-all-mpnet-base-v2-
...
-          pip install -q sentence-transformers
+          pip install -q sentence-transformers==5.5.1
🤖 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/e2e_tests_providers.yaml around lines 202 - 209, The cache
key `hf-sentence-transformers-all-mpnet-base-v2` is static and will not
invalidate when dependencies change, and the `pip install -q
sentence-transformers` command installs a floating version making CI
non-reproducible. Update the cache key to include a dependency hash (using
hashFiles or a version hash) so it invalidates when dependencies change, and pin
the sentence-transformers package to a specific version in the pip install
command to ensure reproducible builds across CI runs.
.github/workflows/e2e_tests.yaml (1)

40-41: ⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Read the exact bytes at line 40-41 to check quote characters
head -41 .github/workflows/e2e_tests.yaml | tail -2 | od -c

Repository: lightspeed-core/lightspeed-stack

Length of output: 119


🏁 Script executed:

# Also read with cat -n for context
sed -n '40,42p' .github/workflows/e2e_tests.yaml | cat -A

Repository: lightspeed-core/lightspeed-stack

Length of output: 199


🌐 Web query:

actions/checkout submodules input quote handling YAML string

💡 Result:

For the submodules input in the actions/checkout action, there is no special requirement for quoting beyond standard YAML syntax rules [1][2][3]. The submodules input accepts specific string values—true or recursive—which are treated as standard YAML scalars [1][2]. General Guidelines for Quoting in GitHub Actions: 1. Standard YAML Input: When passing the submodules input in a workflow file, you can provide the value without quotes or with either single or double quotes, as it is a standard string input [4][3]. Example: yaml - uses: actions/checkout@v4 with: submodules: true or yaml - uses: actions/checkout@v4 with: submodules: 'true' 2. Expression Syntax (${{ ... }}): If you are passing the value dynamically using an expression, remember that GitHub Actions expressions strictly support only single quotes for string literals [5][6][7]. Double quotes inside ${{ ... }} will cause a parsing error [5][6]. Example: yaml - uses: actions/checkout@v4 with: submodules: ${{ 'true' }} 3. Internal Quoting (Context): While standard inputs follow YAML rules, be aware that the internal implementation of actions/checkout sometimes wraps specific commands (such as those used for internal submodule foreach operations) in quotes to ensure shell pipelines are handled correctly [8][9]. This is an implementation detail of the action itself and does not change how you define the submodules input in your workflow YAML [1][9].

Citations:


Remove or replace the typographic quotes with ASCII quotes for consistency, though this is not functionally required.

Line 41 contains typographic (curly) quotes around recursive rather than ASCII straight quotes. While YAML parsers handle typographic quotes correctly as string delimiters and actions/checkout treats the input as a standard YAML scalar, using ASCII quotes is the conventional practice for YAML workflows and improves portability. The suggested diff should be:

Suggested fix
-          submodules: 'recursive'
+          submodules: 'recursive'
🧰 Tools
🪛 zizmor (1.25.2)

[warning] 27-41: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🤖 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/e2e_tests.yaml around lines 40 - 41, The submodules
parameter in the e2e_tests.yaml workflow file uses typographic (curly) quotes
around the value 'recursive' instead of ASCII straight quotes. Replace the
typographic quotes with standard ASCII single or double quotes around the string
'recursive' on the line containing submodules: to follow YAML workflow
conventions and improve portability.
🔇 Additional comments (3)
.github/workflows/e2e_tests.yaml (1)

120-133: This was already flagged in prior review comments on this file (unpinned actions/cache, static cache key, and unpinned sentence-transformers).

.github/workflows/e2e_tests_rhaiis.yaml (1)

135-147: Same issue pattern already raised on .github/workflows/e2e_tests_providers.yaml (unpinned actions/cache, static cache key, and unpinned sentence-transformers in HF prefetch block).

.github/workflows/e2e_tests_rhelai.yaml (1)

149-161: Same issue pattern already raised on .github/workflows/e2e_tests_providers.yaml (unpinned actions/cache, static cache key, and unpinned sentence-transformers in HF prefetch block).


Walkthrough

This PR adds HuggingFace model caching to CI and Docker services to mitigate rate-limiting. Five GitHub Actions workflows pre-download the sentence-transformers model to /tmp/hf-cache, export its path, and two Docker Compose files mount this cache directory while operating in offline mode to prevent Hub connectivity checks.

Changes

HuggingFace Model Caching

Layer / File(s) Summary
CI workflow model prefetching
.github/workflows/e2e_tests.yaml, .github/workflows/e2e_tests_providers.yaml, .github/workflows/e2e_tests_rhaiis.yaml, .github/workflows/e2e_tests_rhelai.yaml
Five E2E test workflows each add a cache layer for /tmp/hf-cache and a pre-download step that installs sentence-transformers, downloads the all-mpnet-base-v2 model into the cache, and exports HF_CACHE_PATH environment variable for downstream Docker service steps.
Docker services cache mounting and offline mode
docker-compose-library.yaml, docker-compose.yaml
Both lightspeed-stack and llama-stack services mount the HuggingFace cache directory via HF_CACHE_PATH (with fallback defaults) and set HF_HUB_OFFLINE=1 to disable Hub update checks during container execution.

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'LCORE-2499: prevent HF from downloading' clearly describes the main objective of preventing Hugging Face downloads, which is the primary purpose reflected across all file changes (caching strategy, offline mode, cache mounting).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🤖 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/e2e_tests.yaml:
- Around line 117-121: Update the static cache key used with actions/cache@v4
(the key value "hf-sentence-transformers-all-mpnet-base-v2") to include a
versioning component so caches can be invalidated automatically; modify the
`key` to append a dynamic value such as `${{ github.run_number }}`, `${{
github.sha }}`, or a hash of lockfiles, and add a `restore-keys` fallback
pattern to speed up restores on partial hits (keep the `path` and action
unchanged, only adjust the `key` and add `restore-keys`).
- Line 118: Replace the floating action reference "uses: actions/cache@v4" with
a pinned commit SHA (e.g., "uses:
actions/cache@3624ceb22c1c5a301c8db4169662070a689d9ea8") to hard-pin the action
for supply-chain security and add a trailing comment noting the semantic version
(v4.1.1) you pinned from for future maintainability; update any tests or docs
that expect the unpinned form if necessary.
- Line 127: Replace the unpinned pip install command "pip install -q
sentence-transformers" with a pinned version to ensure CI reproducibility; pick
the exact version used by the project (from pyproject.toml or requirements.txt)
or a known-compatible release and update the workflow step to install that
specific version instead of the floating package.

In `@docker-compose.yaml`:
- Around line 61-62: Remove the hardcoded HF_HUB_OFFLINE=1 entry that forces
offline mode and instead make offline mode conditional: stop setting
HF_HUB_OFFLINE in the compose env block and document/expect the CI pipeline to
export HF_HUB_OFFLINE=1 (or set it in the CI job environment) when HF_CACHE_PATH
is pre-populated; alternatively add a local setup script to pre-download the
embedding into the mounted HF_CACHE_PATH (or ./tmp/.hf-cache) before running
docker compose up so the container can start without offline mode; apply the
same change to the docker-compose-library.yaml and ensure references to
HF_CACHE_PATH remain unchanged.
🪄 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: ASSERTIVE

Plan: Pro

Run ID: d23081cc-fdbd-4c27-b2a0-8197a258dbe2

📥 Commits

Reviewing files that changed from the base of the PR and between bf9b8a7 and d8dc873.

📒 Files selected for processing (3)
  • .github/workflows/e2e_tests.yaml
  • docker-compose-library.yaml
  • docker-compose.yaml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
  • GitHub Check: check_dependencies
  • GitHub Check: Pylinter
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: E2E: library mode / ci / group 3
  • GitHub Check: E2E: library mode / ci / group 1
  • GitHub Check: E2E: library mode / ci / group 2
  • GitHub Check: E2E: server mode / ci / group 3
  • GitHub Check: E2E: server mode / ci / group 2
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: blublinsky
Repo: lightspeed-core/lightspeed-stack PR: 972
File: src/models/config.py:459-513
Timestamp: 2026-01-12T10:58:50.251Z
Learning: In the lightspeed-stack repository, when a user claims a fix is done but the code still shows the original issue, verify the current state of the code before accepting the fix.
📚 Learning: 2026-05-20T08:09:30.641Z
Learnt from: max-svistunov
Repo: lightspeed-core/lightspeed-stack PR: 1580
File: docs/design/llama-stack-config-merge/poc-results/library-mode/synthesized-run.yaml:107-110
Timestamp: 2026-05-20T08:09:30.641Z
Learning: In Llama-stack config YAMLs, when defining a Llama Guard safety shield entry, set `provider_shield_id` to the *guard model identifier* (e.g., `meta-llama/Llama-Guard-3-8B`). Do not use a chat/generative model id (e.g., `openai/gpt-4o-mini`): a chat-model id (or `native_override`) indicates only an override landed and does **not** mean the safety shield is actually gating queries. Ensure any E2E coverage for the related implementation (JIRA/E2E tests) exercises a real Llama Guard model to verify that the shield is effective.

Applied to files:

  • docker-compose.yaml
  • docker-compose-library.yaml
📚 Learning: 2026-05-20T08:09:36.724Z
Learnt from: max-svistunov
Repo: lightspeed-core/lightspeed-stack PR: 1580
File: src/client.py:104-108
Timestamp: 2026-05-20T08:09:36.724Z
Learning: In the lightspeed-stack repo, the synthesized `run.yaml` file handling in `src/client.py` (`_synthesize_library_config`) uses a fixed `/tmp` path intentionally in the PoC (PR `#1580`). The durable production requirements are tracked in spec doc R10 (docs/design/llama-stack-config-merge/llama-stack-config-merge.md): persistent known path overwritten each boot, file mode 0600 set via explicit create flag (not umask), and a `--synthesized-config-output` CLI flag for debugging. The PoC code is scheduled for removal pre-merge; the implementation JIRA "Unified llama_stack.config schema + synthesizer" inherits R10's requirements.

Applied to files:

  • docker-compose-library.yaml
📚 Learning: 2026-05-06T08:35:54.687Z
Learnt from: radofuchs
Repo: lightspeed-core/lightspeed-stack PR: 1690
File: .github/workflows/e2e_tests_providers.yaml:279-285
Timestamp: 2026-05-06T08:35:54.687Z
Learning: In .github/workflows/e2e_tests_providers.yaml and related e2e workflow files, the show_logs step should not use docker compose logs with --tail or --since (i.e., keep logs unbounded). The quick connectivity test runs once immediately after container startup, so the log output is small and a log tail limit is unnecessary. If you adjust this, add a rationale comment in the workflow explaining why unbounded logs are acceptable and ensure CI behavior remains deterministic.

Applied to files:

  • .github/workflows/e2e_tests.yaml
🪛 zizmor (1.25.2)
.github/workflows/e2e_tests.yaml

[error] 118-118: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🔇 Additional comments (3)
docker-compose.yaml (2)

82-116: LGTM!

The lightspeed-stack service in server mode does not include HuggingFace cache mounts or offline mode settings, which is architecturally correct: the llama-stack service handles embedding model loading, and lightspeed-stack orchestrates requests without direct HuggingFace dependency.


24-24: HuggingFace cache mount target matches the llama-stack container default

  • docker-compose.yaml mounts ${HF_CACHE_PATH:-./tmp/.hf-cache} to /opt/app-root/src/.cache/huggingface for llama-stack (image built from deploy/llama-stack/test.containerfile).
  • That container sets ENV HOME="/opt/app-root/src" and there are no HF_HOME/HF_CACHE/TRANSFORMERS_CACHE overrides in the llama-stack entrypoint/config, so HuggingFace’s default $HOME/.cache/huggingface matches the mount target.
docker-compose-library.yaml (1)

61-62: ⚡ Quick win

Make HF_HUB_OFFLINE=1 conditional to avoid breaking local runs
docker-compose-library.yaml hardcodes HF_HUB_OFFLINE=1, which applies to all environments and forces HuggingFace into offline mode—so startup will require pre-populated HF cache content (e.g., via a CI pre-download step or manual cache population). Repo search finds no docs/scripts explaining local HF cache/offline setup, so local development can easily fail. Default offline to off locally (e.g., HF_HUB_OFFLINE=${HF_HUB_OFFLINE:-0}) and/or document the required local pre-download/cache-mount steps.

Comment on lines +117 to +121
- name: Cache HuggingFace embedding model
uses: actions/cache@v4
with:
path: /tmp/hf-cache
key: hf-sentence-transformers-all-mpnet-base-v2

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add versioning to cache key to enable invalidation.

The cache key hf-sentence-transformers-all-mpnet-base-v2 is static and doesn't include version information. If the model is updated upstream or the sentence-transformers library changes model handling, stale cached artifacts will persist until manually invalidated.

📦 Suggested improvement
       - name: Cache HuggingFace embedding model
         uses: actions/cache@v4
         with:
           path: /tmp/hf-cache
-          key: hf-sentence-transformers-all-mpnet-base-v2
+          key: hf-sentence-transformers-all-mpnet-base-v2-${{ hashFiles('**/pyproject.toml') }}
+          restore-keys: |
+            hf-sentence-transformers-all-mpnet-base-v2-

Include a hash of dependency lockfiles or a date component (e.g., ${{ github.run_number }} or ${{ github.sha }}) in the key. Use restore-keys as a fallback to speed up cache misses.

🧰 Tools
🪛 zizmor (1.25.2)

[error] 118-118: 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/e2e_tests.yaml around lines 117 - 121, Update the static
cache key used with actions/cache@v4 (the key value
"hf-sentence-transformers-all-mpnet-base-v2") to include a versioning component
so caches can be invalidated automatically; modify the `key` to append a dynamic
value such as `${{ github.run_number }}`, `${{ github.sha }}`, or a hash of
lockfiles, and add a `restore-keys` fallback pattern to speed up restores on
partial hits (keep the `path` and action unchanged, only adjust the `key` and
add `restore-keys`).

grep -A 3 "llama_stack:" lightspeed-stack.yaml

- name: Cache HuggingFace embedding model
uses: actions/cache@v4

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Pin action to commit SHA for supply-chain security.

The actions/cache@v4 reference is not pinned to a specific commit SHA. GitHub Actions security best practices require pinning to immutable commit hashes to prevent supply-chain attacks.

🔒 Recommended fix
-      - name: Cache HuggingFace embedding model
-        uses: actions/cache@v4
+      - name: Cache HuggingFace embedding model  
+        uses: actions/cache@3624ceb22c1c5a301c8db4169662070a689d9ea8  # v4.1.1

Use actions/cache@3624ceb22c1c5a301c8db4169662070a689d9ea8 (current v4.1.1) or the latest commit from the v4 branch. Add a comment with the version for maintainability.

📝 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.

Suggested change
uses: actions/cache@v4
uses: actions/cache@3624ceb22c1c5a301c8db4169662070a689d9ea8 # v4.1.1
🧰 Tools
🪛 zizmor (1.25.2)

[error] 118-118: 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/e2e_tests.yaml at line 118, Replace the floating action
reference "uses: actions/cache@v4" with a pinned commit SHA (e.g., "uses:
actions/cache@3624ceb22c1c5a301c8db4169662070a689d9ea8") to hard-pin the action
for supply-chain security and add a trailing comment noting the semantic version
(v4.1.1) you pinned from for future maintainability; update any tests or docs
that expect the unpinned form if necessary.

env:
HF_HOME: /tmp/hf-cache
run: |
pip install -q sentence-transformers

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Pin sentence-transformers version for reproducibility.

Installing sentence-transformers without a version constraint can lead to non-deterministic CI behavior if the library releases breaking changes or updates model-loading logic.

📌 Suggested fix
-          pip install -q sentence-transformers
+          pip install -q sentence-transformers==3.3.1

Pin to the version currently in use (check pyproject.toml or requirements.txt if available). If the project doesn't use sentence-transformers directly, pin to a known-compatible version.

📝 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.

Suggested change
pip install -q sentence-transformers
pip install -q sentence-transformers==3.3.1
🤖 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/e2e_tests.yaml at line 127, Replace the unpinned pip
install command "pip install -q sentence-transformers" with a pinned version to
ensure CI reproducibility; pick the exact version used by the project (from
pyproject.toml or requirements.txt) or a known-compatible release and update the
workflow step to install that specific version instead of the floating package.

Comment thread docker-compose.yaml
Comment on lines +61 to +62
# Prevent HuggingFace Hub update checks (HTTP 429 rate-limiting in CI from parallel jobs).
- HF_HUB_OFFLINE=1

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for documentation on local vs CI compose usage
rg -i -C3 'docker.compose.*local|local.*docker|development.*setup' --type=md README.md CONTRIBUTING.md docs/

# Check if there's a separate compose override file for local development
fd 'docker-compose.*override|docker-compose.*local' --type=f

Repository: lightspeed-core/lightspeed-stack

Length of output: 5038


🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Locate docker-compose.yaml
echo "== docker-compose.yaml candidates =="
fd -i 'docker-compose\.ya?ml' . || true

# Show the referenced lines (if file at repo root; otherwise find the closest match and print the requested vicinity)
echo
echo "== docker-compose.yaml excerpt around lines 55-75 =="
for f in $(fd -i 'docker-compose\.ya?ml' . -x echo 2>/dev/null | tr '\n' ' '); do
  echo "--- $f ---"
  nl -ba "$f" | sed -n '55,80p' || true
done

echo
echo "== occurrences of HF_HUB_OFFLINE / HF_CACHE_PATH =="
rg -n --hidden --no-ignore -S 'HF_HUB_OFFLINE|HF_CACHE_PATH' . || true

echo
echo "== occurrences of huggingface offline/cache/pre-download =="
rg -n --hidden --no-ignore -S 'hugging.?face|hf-cache|hf cache|pre-?download' . || true

Repository: lightspeed-core/lightspeed-stack

Length of output: 11390


🏁 Script executed:

#!/bin/bash
set -euo pipefail

mapfile -t files < <(fd -i 'docker-compose\.ya?ml' .)

echo "== docker-compose.yaml candidates =="
printf '%s\n' "${files[@]:-<none>}"

echo
echo "== docker-compose.yaml excerpt around lines 55-80 (each match) =="
if [ "${`#files`[@]}" -eq 0 ]; then
  exit 0
fi

for f in "${files[@]}"; do
  echo "--- $f ---"
  nl -ba "$f" | sed -n '55,80p'
  echo
done

echo "== occurrences of HF_HUB_OFFLINE / HF_CACHE_PATH =="
rg -n --hidden --no-ignore -S 'HF_HUB_OFFLINE|HF_CACHE_PATH' . || true

echo
echo "== occurrences of huggingface offline/cache/pre-download =="
rg -n --hidden --no-ignore -S 'hugging.?face|hf-cache|HF_HOME|HF_DATASETS_CACHE|pre-?download' . || true

Repository: lightspeed-core/lightspeed-stack

Length of output: 133


🏁 Script executed:

#!/bin/bash
set -euo pipefail

mapfile -t files < <(fd -i 'docker-compose\.ya?ml' .)

echo "== docker-compose.yaml candidates =="
printf '%s\n' "${files[@]:-<none>}"

echo
echo "== docker-compose.yaml excerpt around lines 55-80 (each match) =="
if [ "${`#files`[@]}" -eq 0 ]; then
  exit 0
fi

for f in "${files[@]}"; do
  echo "--- $f ---"
  nl -ba "$f" | sed -n '55,80p'
  echo
done

echo "== occurrences of HF_HUB_OFFLINE / HF_CACHE_PATH =="
rg -n --hidden --no-ignore -S 'HF_HUB_OFFLINE|HF_CACHE_PATH' . || true

echo
echo "== occurrences of huggingface offline/cache/pre-download =="
rg -n --hidden --no-ignore -S 'hugging.?face|hf-cache|HF_HOME|HF_DATASETS_CACHE|pre-?download' . || true

Repository: lightspeed-core/lightspeed-stack

Length of output: 133


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== docker-compose.yaml lines 50-80 =="
sed -n '50,80p' docker-compose.yaml

echo
echo "== docker-compose-library.yaml lines 50-80 =="
sed -n '50,80p' docker-compose-library.yaml

echo
echo "== Search: HF_HUB_OFFLINE / HF_CACHE_PATH guidance in docs README workflows =="
rg -n --hidden --no-ignore -S 'HF_HUB_OFFLINE|HF_CACHE_PATH|hf-cache|HF_HOME' README.md CONTRIBUTING.md docs .github/workflows || true

echo
echo "== Show e2e_tests workflow around HF cache pre-download lines =="
# Tight context around the lines indicated earlier
rg -n 'Cache HuggingFace embedding model|Pre-download HuggingFace embedding model|HF_CACHE_PATH=/tmp/hf-cache' .github/workflows/e2e_tests.yaml -n
awk 'NR>=90 && NR<=160 {print NR":"$0}' .github/workflows/e2e_tests.yaml

Repository: lightspeed-core/lightspeed-stack

Length of output: 6528


🏁 Script executed:

#!/bin/bash
set -euo pipefail
echo "== docker-compose override/local candidates =="
fd -i "docker-compose.*override.*\.ya?ml|docker-compose.*local.*\.ya?ml|docker-compose\.override\.ya?ml" .
echo
echo "== docker-compose_library.yaml override/local candidates =="
fd -i "docker-compose-library.*override.*\.ya?ml|docker-compose-library.*local.*\.ya?ml" .

Repository: lightspeed-core/lightspeed-stack

Length of output: 184


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Find Containerfile/Dockerfile references to .cache/huggingface =="
rg -n --hidden --no-ignore -S '\.cache/huggingface|HF_HOME|HF_HUB_OFFLINE|sentence-transformers' deploy .github/workflows || true

echo
echo "== Show relevant parts of deploy/lightspeed-stack/Containerfile =="
if [ -f deploy/lightspeed-stack/Containerfile ]; then
  nl -ba deploy/lightspeed-stack/Containerfile | sed -n '90,170p'
fi

echo
echo "== Show relevant parts of deploy/llama-stack/test.containerfile (if present) =="
if [ -f deploy/llama-stack/test.containerfile ]; then
  nl -ba deploy/llama-stack/test.containerfile | sed -n '1,120p'
fi

Repository: lightspeed-core/lightspeed-stack

Length of output: 894


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== deploy/lightspeed-stack/Containerfile (around .cache/huggingface) =="
sed -n '105,150p' deploy/lightspeed-stack/Containerfile

echo
echo "== deploy/llama-stack/test.containerfile (around line ~36) =="
sed -n '1,80p' deploy/llama-stack/test.containerfile

Repository: lightspeed-core/lightspeed-stack

Length of output: 3857


🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n --hidden --no-ignore -S 'HF_HUB_OFFLINE' README.md CONTRIBUTING.md docs .github/workflows deploy || true
echo
rg -n --hidden --no-ignore -S 'hf-cache|HF_CACHE_PATH|pre-download HuggingFace|sentence-transformers/all-mpnet-base-v2' README.md docs .github/workflows || true

Repository: lightspeed-core/lightspeed-stack

Length of output: 2310


Hardcoded HF_HUB_OFFLINE=1 will block local embedding downloads unless the HF cache is pre-populated.

docker-compose.yaml (and docker-compose-library.yaml) unconditionally sets HF_HUB_OFFLINE=1 while mounting ${HF_CACHE_PATH:-./tmp/.hf-cache} into the container’s HuggingFace cache. CI avoids failures by pre-downloading the embedding model into /tmp/hf-cache and exporting HF_CACHE_PATH=/tmp/hf-cache before docker compose up.

Local docker compose up has no cache pre-populated (Containerfile only creates the cache directory), and the docs describe that the embedding model can be downloaded automatically on first start-up—offline mode contradicts this.

      # Prevent HuggingFace Hub update checks (HTTP 429 rate-limiting in CI from parallel jobs).
      - HF_HUB_OFFLINE=1
💡 Recommended fixes

Option 1: Conditional offline mode via environment variable (preferred)

       # Prevent HuggingFace Hub update checks (HTTP 429 rate-limiting in CI from parallel jobs).
-      - HF_HUB_OFFLINE=1
+      - HF_HUB_OFFLINE=${HF_HUB_OFFLINE:-0}

Set HF_HUB_OFFLINE=1 explicitly in the CI workflow environment (where the cache is already pre-filled). Apply the same change to docker-compose-library.yaml.

Option 2: Local pre-download step + docs

Add a local setup script to pre-download the embedding model into ./tmp/.hf-cache and document running it before docker compose up.

🤖 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 `@docker-compose.yaml` around lines 61 - 62, Remove the hardcoded
HF_HUB_OFFLINE=1 entry that forces offline mode and instead make offline mode
conditional: stop setting HF_HUB_OFFLINE in the compose env block and
document/expect the CI pipeline to export HF_HUB_OFFLINE=1 (or set it in the CI
job environment) when HF_CACHE_PATH is pre-populated; alternatively add a local
setup script to pre-download the embedding into the mounted HF_CACHE_PATH (or
./tmp/.hf-cache) before running docker compose up so the container can start
without offline mode; apply the same change to the docker-compose-library.yaml
and ensure references to HF_CACHE_PATH remain unchanged.

@radofuchs radofuchs changed the title prevent HF from downloading LCORE-2499: prevent HF from downloading Jun 5, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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/e2e_tests_providers.yaml:
- Line 199: The `actions/cache@v4` reference uses a mutable version tag which
poses a security risk. Replace the `uses: actions/cache@v4` statement with a
pinned commit SHA instead of the tag (e.g., `uses:
actions/cache@<specific-commit-sha>`) to ensure the action version is immutable
and cannot be retargeted by attackers. Obtain the correct commit SHA for the v4
release from the actions/cache repository.
- Around line 202-209: The cache key
`hf-sentence-transformers-all-mpnet-base-v2` is static and will not invalidate
when dependencies change, and the `pip install -q sentence-transformers` command
installs a floating version making CI non-reproducible. Update the cache key to
include a dependency hash (using hashFiles or a version hash) so it invalidates
when dependencies change, and pin the sentence-transformers package to a
specific version in the pip install command to ensure reproducible builds across
CI runs.

In @.github/workflows/e2e_tests.yaml:
- Around line 40-41: The submodules parameter in the e2e_tests.yaml workflow
file uses typographic (curly) quotes around the value 'recursive' instead of
ASCII straight quotes. Replace the typographic quotes with standard ASCII single
or double quotes around the string 'recursive' on the line containing
submodules: to follow YAML workflow conventions and improve portability.
🪄 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: ASSERTIVE

Plan: Pro

Run ID: cf9389ce-67f1-423c-8d19-66526e0904db

📥 Commits

Reviewing files that changed from the base of the PR and between d8dc873 and 531b09b.

📒 Files selected for processing (5)
  • .github/workflows/e2e_tests.yaml
  • .github/workflows/e2e_tests_providers.yaml
  • .github/workflows/e2e_tests_rhaiis.yaml
  • .github/workflows/e2e_tests_rhelai.yaml
  • docker-compose.yaml
💤 Files with no reviewable changes (1)
  • docker-compose.yaml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (17)
  • GitHub Check: mypy
  • GitHub Check: Pyright
  • GitHub Check: build-pr
  • GitHub Check: Pylinter
  • GitHub Check: spectral
  • GitHub Check: unit_tests (3.13)
  • GitHub Check: unit_tests (3.12)
  • GitHub Check: integration_tests (3.13)
  • GitHub Check: E2E: library mode / ci / group 2
  • GitHub Check: E2E: server mode / ci / group 1
  • GitHub Check: E2E: server mode / ci / group 3
  • GitHub Check: E2E: library mode / ci / group 1
  • GitHub Check: E2E: library mode / ci / group 3
  • GitHub Check: E2E Tests for Lightspeed Evaluation job
  • GitHub Check: E2E: server mode / ci / group 2
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-on-pull-request
  • GitHub Check: Konflux kflux-prd-rh02 / lightspeed-stack-0-6-on-pull-request
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-05-06T08:35:54.687Z
Learnt from: radofuchs
Repo: lightspeed-core/lightspeed-stack PR: 1690
File: .github/workflows/e2e_tests_providers.yaml:279-285
Timestamp: 2026-05-06T08:35:54.687Z
Learning: In .github/workflows/e2e_tests_providers.yaml and related e2e workflow files, the show_logs step should not use docker compose logs with --tail or --since (i.e., keep logs unbounded). The quick connectivity test runs once immediately after container startup, so the log output is small and a log tail limit is unnecessary. If you adjust this, add a rationale comment in the workflow explaining why unbounded logs are acceptable and ensure CI behavior remains deterministic.

Applied to files:

  • .github/workflows/e2e_tests_rhaiis.yaml
  • .github/workflows/e2e_tests.yaml
  • .github/workflows/e2e_tests_providers.yaml
  • .github/workflows/e2e_tests_rhelai.yaml
🪛 zizmor (1.25.2)
.github/workflows/e2e_tests_rhaiis.yaml

[error] 136-136: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

.github/workflows/e2e_tests_providers.yaml

[error] 199-199: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

.github/workflows/e2e_tests_rhelai.yaml

[error] 150-150: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)

(unpinned-uses)

🔇 Additional comments (3)
.github/workflows/e2e_tests.yaml (1)

120-133: This was already flagged in prior review comments on this file (unpinned actions/cache, static cache key, and unpinned sentence-transformers).

.github/workflows/e2e_tests_rhaiis.yaml (1)

135-147: Same issue pattern already raised on .github/workflows/e2e_tests_providers.yaml (unpinned actions/cache, static cache key, and unpinned sentence-transformers in HF prefetch block).

.github/workflows/e2e_tests_rhelai.yaml (1)

149-161: Same issue pattern already raised on .github/workflows/e2e_tests_providers.yaml (unpinned actions/cache, static cache key, and unpinned sentence-transformers in HF prefetch block).

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.

Actionable comments posted: 3

🤖 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/e2e_tests_providers.yaml:
- Line 199: The `actions/cache@v4` reference uses a mutable version tag which
poses a security risk. Replace the `uses: actions/cache@v4` statement with a
pinned commit SHA instead of the tag (e.g., `uses:
actions/cache@<specific-commit-sha>`) to ensure the action version is immutable
and cannot be retargeted by attackers. Obtain the correct commit SHA for the v4
release from the actions/cache repository.
- Around line 202-209: The cache key
`hf-sentence-transformers-all-mpnet-base-v2` is static and will not invalidate
when dependencies change, and the `pip install -q sentence-transformers` command
installs a floating version making CI non-reproducible. Update the cache key to
include a dependency hash (using hashFiles or a version hash) so it invalidates
when dependencies change, and pin the sentence-transformers package to a
specific version in the pip install command to ensure reproducible builds across
CI runs.

In @.github/workflows/e2e_tests.yaml:
- Around line 40-41: The submodules parameter in the e2e_tests.yaml workflow
file uses typographic (curly) quotes around the value 'recursive' instead of
ASCII straight quotes. Replace the typographic quotes with standard ASCII single
or double quotes around the string 'recursive' on the line containing
submodules: to follow YAML workflow conventions and improve portability.
🪄 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: ASSERTIVE

Plan: Pro

Run ID: cf9389ce-67f1-423c-8d19-66526e0904db

📥 Commits

Reviewing files that changed from the base of the PR and between d8dc873 and 531b09b.

📒 Files selected for processing (5)
  • .github/workflows/e2e_tests.yaml
  • .github/workflows/e2e_tests_providers.yaml
  • .github/workflows/e2e_tests_rhaiis.yaml
  • .github/workflows/e2e_tests_rhelai.yaml
  • docker-compose.yaml
💤 Files with no reviewable changes (1)
  • docker-compose.yaml
📜 Review details
🔇 Additional comments (3)
.github/workflows/e2e_tests.yaml (1)

120-133: This was already flagged in prior review comments on this file (unpinned actions/cache, static cache key, and unpinned sentence-transformers).

.github/workflows/e2e_tests_rhaiis.yaml (1)

135-147: Same issue pattern already raised on .github/workflows/e2e_tests_providers.yaml (unpinned actions/cache, static cache key, and unpinned sentence-transformers in HF prefetch block).

.github/workflows/e2e_tests_rhelai.yaml (1)

149-161: Same issue pattern already raised on .github/workflows/e2e_tests_providers.yaml (unpinned actions/cache, static cache key, and unpinned sentence-transformers in HF prefetch block).

🛑 Comments failed to post (3)
.github/workflows/e2e_tests_providers.yaml (2)

199-199: ⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n .github/workflows/e2e_tests_providers.yaml | sed -n '195,205p'

Repository: lightspeed-core/lightspeed-stack

Length of output: 508


🏁 Script executed:

# Check if the file exists and get its total line count
wc -l .github/workflows/e2e_tests_providers.yaml

Repository: lightspeed-core/lightspeed-stack

Length of output: 123


🏁 Script executed:

# Search for all actions/cache usages in the file
rg "actions/cache" .github/workflows/e2e_tests_providers.yaml -n

Repository: lightspeed-core/lightspeed-stack

Length of output: 111


🏁 Script executed:

# Check for other unpinned GitHub Actions in the file
rg "uses: [a-zA-Z0-9/_-]+@(v[0-9]|latest)" .github/workflows/e2e_tests_providers.yaml -n

Repository: lightspeed-core/lightspeed-stack

Length of output: 148


🏁 Script executed:

# Verify the suggested commit SHA exists for actions/cache v4.1.1
git ls-remote --heads --tags https://github.com/actions/cache.git | grep -E "(3624ceb22c1c5a301c8db4169662070a689d9ea8|v4\.1\.1)"

Repository: lightspeed-core/lightspeed-stack

Length of output: 134


🌐 Web query:

GitHub Actions pinning commit SHA security best practice

💡 Result:

Pinning GitHub Actions to a full-length commit SHA is a security best practice that protects against supply chain attacks, such as malicious code being injected into an existing tag [1][2][3]. Why it is a best practice: - Immutability: Tags and branch names are mutable references in Git, meaning they can be updated to point to different commits [2][4][5]. If an attacker gains control of a repository, they could overwrite a tag with malicious code, which would automatically execute in any workflow using that tag [3][6]. A full-length commit SHA uniquely identifies an exact point in the codebase and cannot be changed, ensuring that your workflow consistently runs the specific, reviewed code you expect [2][4][5]. - Policy Enforcement: GitHub supports organization and repository-level policies that enforce SHA pinning [7][4][8]. When enabled, these policies cause workflows to fail if they attempt to use actions pinned to tags or branches, helping administrators proactively limit the impact of compromised dependencies across their infrastructure [7][4]. Recommended implementation: - Use full-length SHA: Always use the full commit SHA (40 characters) rather than a shortened version [1][4][3]. - Maintain readability: Include the corresponding version tag as a comment in your workflow file to maintain readability and make it easier to track updates [3][9][8]. Example: uses: actions/checkout@11bd719 # v4.2.2 - Automate updates: To manage the maintenance burden of updating pinned SHAs, use automated dependency management tools like Renovate or Dependabot [5][3][6]. Specifically, tools like Renovate support "helpers" (e.g., helpers:pinGitHubActionDigests) that can automatically create pull requests to update your pinned SHAs while keeping the version comment current [3][6]. - Verify the source: When selecting a SHA, ensure it comes from the official action repository and not a fork [1]. Note that pinning protects against tag retargeting, but it does not protect against vulnerabilities present in the code of the specific commit you have pinned; you should still vet the actions you use [6].

Citations:


Pin actions/cache to a commit SHA.

uses: actions/cache@v4 is mutable and violates immutable action pinning requirements. Tags can be retargeted by attackers to inject malicious code.

Suggested fix
-        uses: actions/cache@v4
+        uses: actions/cache@3624ceb22c1c5a301c8db4169662070a689d9ea8 # v4.1.1
📝 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.

        uses: actions/cache@3624ceb22c1c5a301c8db4169662070a689d9ea8 # v4.1.1
🧰 Tools
🪛 zizmor (1.25.2)

[error] 199-199: 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/e2e_tests_providers.yaml at line 199, The
`actions/cache@v4` reference uses a mutable version tag which poses a security
risk. Replace the `uses: actions/cache@v4` statement with a pinned commit SHA
instead of the tag (e.g., `uses: actions/cache@<specific-commit-sha>`) to ensure
the action version is immutable and cannot be retargeted by attackers. Obtain
the correct commit SHA for the v4 release from the actions/cache repository.

Source: Linters/SAST tools


202-209: ⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for pyproject.toml and uv.lock
echo "=== Checking for pyproject.toml ==="
if [ -f "pyproject.toml" ]; then
  echo "Found pyproject.toml"
  # Search for sentence-transformers version
  grep -n "sentence-transformers" pyproject.toml || echo "Not found in pyproject.toml"
else
  echo "pyproject.toml not found"
fi

echo ""
echo "=== Checking for uv.lock ==="
if [ -f "uv.lock" ]; then
  echo "Found uv.lock"
  # Search for sentence-transformers version
  grep -A 5 "sentence-transformers" uv.lock | head -20 || echo "Not found in uv.lock"
else
  echo "uv.lock not found"
fi

echo ""
echo "=== Checking for requirements.txt or similar ==="
find . -maxdepth 2 -name "*requirements*.txt" -o -name "setup.py" -o -name "setup.cfg" | grep -v ".git"

Repository: lightspeed-core/lightspeed-stack

Length of output: 1490


🏁 Script executed:

#!/bin/bash
# Read the workflow file around lines 202-209
echo "=== Workflow file content around lines 195-220 ==="
sed -n '195,220p' .github/workflows/e2e_tests_providers.yaml

echo ""
echo "=== Full context around cache and prefetch ==="
grep -n -B 5 -A 15 "Pre-download HuggingFace embedding model" .github/workflows/e2e_tests_providers.yaml

Repository: lightspeed-core/lightspeed-stack

Length of output: 2155


Add version hash to cache key and pin sentence-transformers version for deterministic CI.

The cache key at line 202 is static and doesn't include dependency version hashes, so it won't invalidate when dependencies change. Additionally, pip install -q sentence-transformers at line 208 installs a floating version, making CI behavior non-reproducible.

Suggested fix
       - name: Cache HuggingFace embedding model
         uses: actions/cache@v4
         with:
           path: /tmp/hf-cache
-          key: hf-sentence-transformers-all-mpnet-base-v2
+          key: hf-sentence-transformers-all-mpnet-base-v2-${{ hashFiles('**/pyproject.toml', '**/uv.lock') }}
+          restore-keys: |
+            hf-sentence-transformers-all-mpnet-base-v2-
...
-          pip install -q sentence-transformers
+          pip install -q sentence-transformers==5.5.1
🤖 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/e2e_tests_providers.yaml around lines 202 - 209, The cache
key `hf-sentence-transformers-all-mpnet-base-v2` is static and will not
invalidate when dependencies change, and the `pip install -q
sentence-transformers` command installs a floating version making CI
non-reproducible. Update the cache key to include a dependency hash (using
hashFiles or a version hash) so it invalidates when dependencies change, and pin
the sentence-transformers package to a specific version in the pip install
command to ensure reproducible builds across CI runs.
.github/workflows/e2e_tests.yaml (1)

40-41: ⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# Read the exact bytes at line 40-41 to check quote characters
head -41 .github/workflows/e2e_tests.yaml | tail -2 | od -c

Repository: lightspeed-core/lightspeed-stack

Length of output: 119


🏁 Script executed:

# Also read with cat -n for context
sed -n '40,42p' .github/workflows/e2e_tests.yaml | cat -A

Repository: lightspeed-core/lightspeed-stack

Length of output: 199


🌐 Web query:

actions/checkout submodules input quote handling YAML string

💡 Result:

For the submodules input in the actions/checkout action, there is no special requirement for quoting beyond standard YAML syntax rules [1][2][3]. The submodules input accepts specific string values—true or recursive—which are treated as standard YAML scalars [1][2]. General Guidelines for Quoting in GitHub Actions: 1. Standard YAML Input: When passing the submodules input in a workflow file, you can provide the value without quotes or with either single or double quotes, as it is a standard string input [4][3]. Example: yaml - uses: actions/checkout@v4 with: submodules: true or yaml - uses: actions/checkout@v4 with: submodules: 'true' 2. Expression Syntax (${{ ... }}): If you are passing the value dynamically using an expression, remember that GitHub Actions expressions strictly support only single quotes for string literals [5][6][7]. Double quotes inside ${{ ... }} will cause a parsing error [5][6]. Example: yaml - uses: actions/checkout@v4 with: submodules: ${{ 'true' }} 3. Internal Quoting (Context): While standard inputs follow YAML rules, be aware that the internal implementation of actions/checkout sometimes wraps specific commands (such as those used for internal submodule foreach operations) in quotes to ensure shell pipelines are handled correctly [8][9]. This is an implementation detail of the action itself and does not change how you define the submodules input in your workflow YAML [1][9].

Citations:


Remove or replace the typographic quotes with ASCII quotes for consistency, though this is not functionally required.

Line 41 contains typographic (curly) quotes around recursive rather than ASCII straight quotes. While YAML parsers handle typographic quotes correctly as string delimiters and actions/checkout treats the input as a standard YAML scalar, using ASCII quotes is the conventional practice for YAML workflows and improves portability. The suggested diff should be:

Suggested fix
-          submodules: 'recursive'
+          submodules: 'recursive'
🧰 Tools
🪛 zizmor (1.25.2)

[warning] 27-41: credential persistence through GitHub Actions artifacts (artipacked): does not set persist-credentials: false

(artipacked)

🤖 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/e2e_tests.yaml around lines 40 - 41, The submodules
parameter in the e2e_tests.yaml workflow file uses typographic (curly) quotes
around the value 'recursive' instead of ASCII straight quotes. Replace the
typographic quotes with standard ASCII single or double quotes around the string
'recursive' on the line containing submodules: to follow YAML workflow
conventions and improve portability.

@tisnik tisnik left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tisnik tisnik merged commit e429854 into lightspeed-core:main Jun 16, 2026
29 of 33 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants