Skip to content

feat(ci): implement Vitest shards with merge-reports for e2e workflow - #835

Merged
steebchen merged 20 commits into
mainfrom
terragon/e2e-vitest-shards-github-matrix
Sep 15, 2025
Merged

steebchen merged 20 commits into
mainfrom
terragon/e2e-vitest-shards-github-matrix

Conversation

@steebchen

@steebchen steebchen commented Sep 15, 2025

Copy link
Copy Markdown
Member

Summary

  • Split e2e tests across 5 parallel runners using GitHub matrix strategy
  • Add merge-reports job to combine shard results using Vitest's --merge-reports
  • Maintain single 'e2e' check for branch protection compatibility
  • Add artifact upload for each shard and download in merge step for report aggregation

Test plan

  • Verify workflow runs successfully on PR
  • Confirm all 5 shards execute in parallel
  • Check that merge-reports step combines results correctly
  • Validate final 'e2e' job reports overall status

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Tests

    • Parallelized end-to-end tests into multiple shards for faster feedback.
    • Automatically aggregates per-shard outputs into a single, clear final report.
    • Ensures test results are always collected and visible, even on failures.
  • Chores

    • Optimized CI setup and caching to reduce run times.
    • Adjusted workflow triggers to skip heavy e2e runs on Dependabot PRs, conserving resources.

🌿 Generated by Terry


ℹ️ Tag @terragon-labs to ask questions and address PR feedback

📎 Task: https://www.terragonlabs.com/task/7f087853-d89a-47e6-b28a-02ed35f675d5

- Split e2e tests across 5 parallel runners using matrix strategy
- Add merge-reports job to combine shard results
- Maintain single e2e check for branch protection compatibility

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Sep 15, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

Replaces the single delegated e2e workflow with a two-job setup: a matrix-based e2e-shards job running five parallel Vitest shards that upload blob artifacts, followed by an e2e job that downloads all shard artifacts, merges reports locally, and produces the final e2e result.

Changes

Cohort / File(s) Summary
E2E workflow orchestration
.github/workflows/e2e.yml
Replaces delegated e2e with two jobs: (1) e2e-shards matrix [1..5] runs setup and vitest --shard per shard, uploads e2e-results-*.blob artifacts with many API keys passed via secrets; (2) e2e waits for shards, downloads artifacts, and runs vitest --merge-reports to produce final results.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Dev as PR author
    participant GH as GitHub Actions
    participant Shards as e2e-shards (matrix 1..5)
    participant Store as Artifacts
    participant Final as e2e (merge)
    participant VT as Vitest

    Dev->>GH: Open/Update PR
    GH->>Shards: Trigger matrix [1..5] (if not dependabot)
    par Parallel shards
        Shards->>Shards: Checkout, setup Node/pnpm, install
        Shards->>VT: Run vitest --shard i/5 --reporter=blob
        note right of Shards: Produces e2e-results-i.blob
        Shards->>Store: Upload artifact e2e-results-i.blob (always)
    end
    GH->>Final: Start after all shards (needs)
    Final->>Store: Download e2e-results-*
    Final->>Final: Move blobs to .vitest-reports
    Final->>VT: vitest --merge-reports
    VT-->>Final: Combined report
    Final-->>GH: Publish final e2e job result
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title concisely and accurately summarizes the primary change: implementing Vitest test sharding and merge-reports for the e2e workflow. It uses a conventional commit prefix ("feat(ci):"), is specific to the CI change, and matches the PR objectives and changed workflow files, so a reviewer scanning history can quickly understand the main intent.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch terragon/e2e-vitest-shards-github-matrix

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: 5

🧹 Nitpick comments (7)
.github/workflows/e2e.yml (7)

28-31: Make version collection robust if .tool-versions is missing

Current commands fail if the file is absent or format changes. Provide defaults.

-      - name: Collect versions
+      - name: Collect versions
         run: |
-          echo "nodejs_version=$(cat .tool-versions | grep 'nodejs' | cut -d ' ' -f 2)" >> "$GITHUB_ENV"
-          echo "pnpm_version=$(cat .tool-versions | grep 'pnpm' | cut -d ' ' -f 2)" >> "$GITHUB_ENV"
+          nodejs="$(awk '/^nodejs[[:space:]]/{print $2; exit}' .tool-versions 2>/dev/null || true)"
+          pnpmv="$(awk '/^pnpm[[:space:]]/{print $2; exit}' .tool-versions 2>/dev/null || true)"
+          echo "nodejs_version=${nodejs:-22}" >> "$GITHUB_ENV"
+          echo "pnpm_version=${pnpmv:-9}" >> "$GITHUB_ENV"

69-75: Fail fast if artifact is missing

Ensure we don’t silently pass when the blob wasn’t produced.

       - name: Upload shard results
         uses: actions/upload-artifact@v5
         with:
           name: e2e-results-${{ matrix.shard }}
           path: e2e-results-${{ matrix.shard }}.blob
+          if-no-files-found: error
           retention-days: 1

86-90: Mirror version-collection hardening in merge job

Keep both jobs consistent and resilient.

-      - name: Collect versions
+      - name: Collect versions
         run: |
-          echo "nodejs_version=$(cat .tool-versions | grep 'nodejs' | cut -d ' ' -f 2)" >> "$GITHUB_ENV"
-          echo "pnpm_version=$(cat .tool-versions | grep 'pnpm' | cut -d ' ' -f 2)" >> "$GITHUB_ENV"
+          nodejs="$(awk '/^nodejs[[:space:]]/{print $2; exit}' .tool-versions 2>/dev/null || true)"
+          pnpmv="$(awk '/^pnpm[[:space:]]/{print $2; exit}' .tool-versions 2>/dev/null || true)"
+          echo "nodejs_version=${nodejs:-22}" >> "$GITHUB_ENV"
+          echo "pnpm_version=${pnpmv:-9}" >> "$GITHUB_ENV"

103-114: Simplify artifact download and avoid moving files; upload merged report

Use merge-multiple to place all blobs in one dir and persist the merged result for debugging.

       - name: Download all shard results
         uses: actions/download-artifact@v5
         with:
           path: ./e2e-results
           pattern: e2e-results-*
+          merge-multiple: true
-
-      - name: Merge reports
-        run: |
-          # Move all .blob files to root directory
-          find ./e2e-results -name "*.blob" -exec mv {} ./ \;
-          # Merge reports
-          cross-env-shell "DATABASE_URL=${DATABASE_URL:-postgres://postgres:pw@localhost:5432/test}" E2E_TEST=true vitest run -c vitest/vitest.e2e.config.mts --merge-reports
+      - name: Merge reports
+        run: |
+          set -euo pipefail
+          ls -1 ./e2e-results/*.blob
+          cross-env-shell "E2E_TEST=true vitest run -c vitest/vitest.e2e.config.mts --merge-reports --reporter=blob --outputFile=e2e-results-merged.blob"
+        env:
+          DATABASE_URL: postgresql://postgres:postgres@localhost:5432/test
+
+      - name: Upload merged report
+        uses: actions/upload-artifact@v5
+        with:
+          name: e2e-results-merged
+          path: e2e-results-merged.blob
+          retention-days: 7

1-14: Harden workflow defaults and permissions

Set least-privilege permissions and default shell flags once to reduce repetition and risk.

 name: e2e
 on:
   workflow_dispatch:
   pull_request:
@@
 concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}
   cancel-in-progress: true
+
+permissions:
+  contents: read
+
+defaults:
+  run:
+    shell: bash

37-41: Optional: pin cache dependency path

Explicitly pin lockfile path for cache stability in monorepos.

       - uses: actions/setup-node@v5
         with:
           node-version: ${{ env.nodejs_version }}
-          cache: pnpm
+          cache: pnpm
+          cache-dependency-path: pnpm-lock.yaml

45-68: Consolidate env configuration

Consider moving shared API keys to job-level env to avoid duplication if more steps are added.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c4fc4c6 and dcd5fe2.

📒 Files selected for processing (1)
  • .github/workflows/e2e.yml (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-09-15T13:15:00.724Z
Learnt from: CR
PR: theopenco/llmgateway#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T13:15:00.724Z
Learning: Applies to apps/gateway/src/api.e2e.ts : Put parallelized .each() E2E tests in apps/gateway/src/api.e2e.ts (uses concurrent mode)

Applied to files:

  • .github/workflows/e2e.yml
📚 Learning: 2025-09-15T13:16:05.355Z
Learnt from: CR
PR: theopenco/llmgateway#0
File: AGENTS.md:0-0
Timestamp: 2025-09-15T13:16:05.355Z
Learning: Applies to apps/gateway/src/api.e2e.ts : Put parallelizable .each() E2E tests in apps/gateway/src/api.e2e.ts (uses concurrent mode)

Applied to files:

  • .github/workflows/e2e.yml
🪛 Checkov (3.2.334)
.github/workflows/e2e.yml

[medium] 49-50: Basic Auth Credentials

(CKV_SECRET_4)

🔇 Additional comments (2)
.github/workflows/e2e.yml (2)

116-129: LGTM: single ‘e2e’ check correctly gates on shards and merge

Final status aggregation matches branch protection needs.

If branch protection targets a specific job name, confirm it’s “e2e” after this change.


45-49: cross-env-shell availability — resolved
cross-env-shell is declared in package.json as "7.0.3" (package.json, line 41); no change required.

Comment thread .github/workflows/e2e.yml
Comment thread .github/workflows/e2e.yml
secrets: inherit
e2e-shards:
runs-on: ubuntu-latest
if: ${{ github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}

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

Guard PR-only context in job condition to avoid expression errors on workflow_dispatch

Accessing github.event.pull_request.* when event_name != pull_request can throw. Gate the condition by event_name first.

-    if: ${{ github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}
+    if: ${{ github.event_name == 'pull_request' && github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}
📝 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
if: ${{ github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}
if: ${{ github.event_name == 'pull_request' && github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}
🤖 Prompt for AI Agents
In .github/workflows/e2e.yml around line 18, the job condition accesses
github.event.pull_request.* unguarded which can throw for non-pull_request
events (e.g. workflow_dispatch); update the if expression to first check the
event name (github.event_name == 'pull_request') and only then evaluate the
other pull_request-specific checks (actor and head.repo.full_name equality) so
the expression short-circuits and avoids errors on non-PR triggers.

Comment thread .github/workflows/e2e.yml
Comment thread .github/workflows/e2e.yml Outdated
Comment thread .github/workflows/e2e.yml Outdated
Comment on lines +49 to +50
cross-env-shell "DATABASE_URL=${DATABASE_URL:-postgres://postgres:pw@localhost:5432/test}" E2E_TEST=true vitest run -c vitest/vitest.e2e.config.mts --no-file-parallelism --shard=${{ matrix.shard }}/5 --reporter=blob --outputFile=e2e-results-${{ matrix.shard }}.blob
env:

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

Basic auth string detected in DATABASE_URL default (static analysis)

A hardcoded postgres://user:pass@... URL was flagged. The diff above removes the inline default. If you need non-default creds, store the full URL in a masked secret like CI_DATABASE_URL and pass via env.

🧰 Tools
🪛 Checkov (3.2.334)

[medium] 49-50: Basic Auth Credentials

(CKV_SECRET_4)

🤖 Prompt for AI Agents
.github/workflows/e2e.yml around lines 49-50: the workflow contains a hardcoded
DATABASE_URL default with embedded credentials; remove the inline
postgres://user:pass URL and instead read DATABASE_URL only from the environment
or from a masked secret (e.g., use secrets.CI_DATABASE_URL passed into the job
and set env: DATABASE_URL: ${{ secrets.CI_DATABASE_URL }}), ensure no plaintext
credentials remain in the file, and update the cross-env-shell invocation to
reference the environment variable without a static default so CI uses the
secret-provided URL.

- Enhanced run.yml to support artifact uploading with optional parameters
- Fixed artifact action version from v5 to v4
- Reused run.yml workflow for all shard jobs to reduce duplication
- Maintained proper artifact sharing for merge-reports functionality

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@steebchen steebchen changed the title feat(ci): implement Vitest shards for e2e workflow feat(ci): implement Vitest shards with merge-reports for e2e workflow Sep 15, 2025

@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: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/run.yml (1)

46-49: Remove -x to avoid leaking secrets from reusable workflow commands

set -x will echo shell commands. Since callers may inline credentials in inputs.cmd (as in e2e.yml), secrets can be printed to logs.

-      - name: ${{ inputs.cmd }}
-        run: |
-          set -eux
-          ${{ inputs.cmd }}
+      - name: ${{ inputs.cmd }}
+        run: |
+          set -euo pipefail
+          ${{ inputs.cmd }}
♻️ Duplicate comments (1)
.github/workflows/e2e.yml (1)

19-19: Guard PR-only context to avoid expression errors on workflow_dispatch

Accessing github.event.pull_request.* when event_name != pull_request can error. Gate by event first.

-    if: ${{ github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}
+    if: ${{ github.event_name == 'pull_request' && github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}

Apply to all shard jobs.

🧹 Nitpick comments (7)
.github/workflows/run.yml (3)

9-18: Guard artifact inputs when upload_artifacts is true

If artifact_name/path are missing while upload_artifacts=true, the upload step will fail late.

       artifact_path:
         required: false
         type: string
+
+  # Early validation
+  # (runs even if the main command fails, to fail-fast on bad inputs)
+  # Note: harmless no-op when upload_artifacts is false.
+  # You can remove if you prefer to rely on the upload step's own validation.
+  # ---

Add an early check step before “Upload artifacts”:

+      - name: Validate artifact inputs
+        if: ${{ inputs.upload_artifacts == true }}
+        run: |
+          test -n "${{ inputs.artifact_name }}" && test -n "${{ inputs.artifact_path }}"

69-75: Fail hard if artifact is missing; consider longer retention for flake triage

Make the upload step error on missing files; 1-day retention may be too short for debugging.

       uses: actions/upload-artifact@v4
       with:
         name: ${{ inputs.artifact_name }}
         path: ${{ inputs.artifact_path }}
-        retention-days: 1
+        if-no-files-found: error
+        retention-days: 3

If you prefer 1 day, keep it—just add if-no-files-found: error.


24-27: Pin actions by commit SHA (supply‑chain hardening)

actions/checkout@v5 is tag-based. Pin to a specific SHA to avoid tag hijacking.

-        uses: actions/checkout@v5
+        uses: actions/checkout@v5
+        # TODO: pin to a specific SHA for checkout@v5

Happy to supply exact SHAs if you want.

.github/workflows/e2e.yml (4)

16-24: Provision Postgres in reusable workflow context (you can’t attach services to “uses”)

Since shard jobs use a reusable workflow, you can’t define services here. Start Postgres in run.yml instead.

Add to .github/workflows/run.yml before “${{ inputs.cmd }}” step:

+      - name: Start Postgres (for E2E)
+        if: ${{ contains(inputs.cmd, 'vitest') && !secrets.CI_DATABASE_URL }}
+        run: |
+          docker run -d --rm --name pg -p 5432:5432 \
+            -e POSTGRES_USER=postgres -e POSTGRES_PASSWORD=postgres -e POSTGRES_DB=test \
+            postgres:16
+          for i in {1..30}; do
+            docker exec pg pg_isready -U postgres -d test && break
+            sleep 2
+          done

This avoids GH “services” and works with the reusable workflow. If you prefer true services, drop the reusable workflow for shard jobs and define services directly here.


99-105: Merging reports doesn’t need DB; drop DATABASE_URL/E2E_TEST

Vitest --merge-reports reads blobs only. Removing DB env eliminates another secret surface.

-          cross-env-shell "DATABASE_URL=${DATABASE_URL:-postgres://postgres:pw@localhost:5432/test}" E2E_TEST=true vitest run -c vitest/vitest.e2e.config.mts --merge-reports
+          vitest run -c vitest/vitest.e2e.config.mts --merge-reports

Optional: upload the merged report as an artifact for triage.

+      - name: Upload merged report
+        uses: actions/upload-artifact@v4
+        with:
+          name: e2e-merged-report
+          path: ./.vitest-report/**
+          retention-days: 3

16-24: Add sane timeouts to prevent hung jobs burning minutes

Set timeout-minutes on shards, merge, and aggregator.

   e2e-shard-1:
     uses: ./.github/workflows/run.yml
+    timeout-minutes: 45
@@
   e2e-shard-2:
     uses: ./.github/workflows/run.yml
+    timeout-minutes: 45
@@
   e2e-shard-3:
     uses: ./.github/workflows/run.yml
+    timeout-minutes: 45
@@
   e2e-shard-4:
     uses: ./.github/workflows/run.yml
+    timeout-minutes: 45
@@
   e2e-shard-5:
     uses: ./.github/workflows/run.yml
+    timeout-minutes: 45
@@
   merge-reports:
     runs-on: ubuntu-latest
+    timeout-minutes: 20
@@
   e2e:
     runs-on: ubuntu-latest
+    timeout-minutes: 5

Also applies to: 26-34, 36-44, 46-54, 56-64, 66-75, 106-118


71-75: Pin actions by commit SHA here too (checkout/setup-node/pnpm/download-artifact)

Same supply-chain hardening as in run.yml.

Example:

-        uses: actions/checkout@v5
+        uses: actions/checkout@v5
+        # TODO: pin to a specific SHA for checkout@v5

Apply similarly to actions/setup-node@v5, pnpm/action-setup@v4, actions/download-artifact@v4.

Also applies to: 85-89, 93-98

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between dcd5fe2 and a40bd29.

📒 Files selected for processing (2)
  • .github/workflows/e2e.yml (1 hunks)
  • .github/workflows/run.yml (2 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
.github/workflows/e2e.yml

[medium] 21-22: Basic Auth Credentials

(CKV_SECRET_4)

Comment thread .github/workflows/e2e.yml Outdated
steebchen and others added 2 commits September 15, 2025 16:11
- Changed upload-artifact from v5 to v4 (latest available version)
- Changed download-artifact from v5 to v4 for consistency

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

@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: 2

♻️ Duplicate comments (3)
.github/workflows/e2e.yml (3)

18-18: Guard PR-only context to avoid expression errors on workflow_dispatch

Accessing github.event.pull_request.* unguarded can error for non-PR events.

-    if: ${{ github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}
+    if: ${{ github.event_name == 'pull_request' && github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}

19-21: Disable fail-fast so one failing shard doesn’t cancel the rest

Needed to ensure all shards finish and artifacts are available for merge.

     strategy:
+      fail-fast: false
       matrix:
         shard: [1, 2, 3, 4, 5]

16-22: Add Postgres service and a sane timeout

Shards rely on DATABASE_URL but no DB service is defined; also add timeout to prevent runaway jobs.

   e2e-shards:
     runs-on: ubuntu-latest
+    timeout-minutes: 45
+    services:
+      postgres:
+        image: postgres:16
+        env:
+          POSTGRES_USER: postgres
+          POSTGRES_PASSWORD: postgres
+          POSTGRES_DB: test
+        ports:
+          - 5432:5432
+        options: >-
+          --health-cmd="pg_isready -U postgres -d test"
+          --health-interval=10s
+          --health-timeout=5s
+          --health-retries=5
🧹 Nitpick comments (6)
.github/workflows/e2e.yml (6)

28-31: Harden version discovery; fail fast with clear defaults

Current grep can yield empty versions and break setup-node. Add defaults and validation.

       - name: Collect versions
         run: |
-          echo "nodejs_version=$(cat .tool-versions | grep 'nodejs' | cut -d ' ' -f 2)" >> "$GITHUB_ENV"
-          echo "pnpm_version=$(cat .tool-versions | grep 'pnpm' | cut -d ' ' -f 2)" >> "$GITHUB_ENV"
+          set -euo pipefail
+          nodejs_version="$(awk '/^nodejs /{print $2}' .tool-versions || true)"
+          pnpm_version="$(awk '/^pnpm /{print $2}' .tool-versions || true)"
+          echo "nodejs_version=${nodejs_version:-lts/*}" >> "$GITHUB_ENV"
+          echo "pnpm_version=${pnpm_version:-9}" >> "$GITHUB_ENV"

86-90: Mirror version hardening here as well

Same grep issue exists in merge job.

       - name: Collect versions
         run: |
-          echo "nodejs_version=$(cat .tool-versions | grep 'nodejs' | cut -d ' ' -f 2)" >> "$GITHUB_ENV"
-          echo "pnpm_version=$(cat .tool-versions | grep 'pnpm' | cut -d ' ' -f 2)" >> "$GITHUB_ENV"
+          set -euo pipefail
+          nodejs_version="$(awk '/^nodejs /{print $2}' .tool-versions || true)"
+          pnpm_version="$(awk '/^pnpm /{print $2}' .tool-versions || true)"
+          echo "nodejs_version=${nodejs_version:-lts/*}" >> "$GITHUB_ENV"
+          echo "pnpm_version=${pnpm_version:-9}" >> "$GITHUB_ENV"

103-108: Optional: flatten artifact download

Setting merge-multiple flattens into a single directory and simplifies the move.

       - name: Download all shard results
         uses: actions/download-artifact@v4
         with:
           path: ./e2e-results
           pattern: e2e-results-*
+          merge-multiple: true

116-129: Single-check promise: confirm branch protection behavior

Multiple jobs will still appear in PR checks. If you truly need only ‘e2e’ to gate merges, ensure branch protection requires only this job; otherwise failed shard jobs will block merges. Consider setting continue-on-error on shard/merge jobs and drive status solely from this final job, but note it changes needs.*.result semantics.


121-129: Minor: log detailed outcomes for easier triage

Print needs.*.result values before exiting.

       - name: Check shard results
         run: |
+          echo "e2e-shards result: ${{ needs.e2e-shards.result }}"
+          echo "merge-reports result: ${{ needs.merge-reports.result }}"
           if [[ "${{ needs.e2e-shards.result }}" == "success" && "${{ needs.merge-reports.result }}" == "success" ]]; then
             echo "All e2e tests passed"
             exit 0
           else
             echo "E2E tests failed"
             exit 1
           fi

1-14: Least-privilege token

Set default workflow permissions.

YAML to add near the top of the workflow:

permissions:
  contents: read
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a40bd29 and 7404d2b.

📒 Files selected for processing (1)
  • .github/workflows/e2e.yml (1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: theopenco/llmgateway#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T13:15:00.724Z
Learning: Applies to apps/gateway/src/api.e2e.ts : Put parallelized .each() E2E tests in apps/gateway/src/api.e2e.ts (uses concurrent mode)
Learnt from: CR
PR: theopenco/llmgateway#0
File: AGENTS.md:0-0
Timestamp: 2025-09-15T13:16:05.355Z
Learning: Applies to apps/gateway/src/api.e2e.ts : Put parallelizable .each() E2E tests in apps/gateway/src/api.e2e.ts (uses concurrent mode)
🪛 Checkov (3.2.334)
.github/workflows/e2e.yml

[medium] 49-50: Basic Auth Credentials

(CKV_SECRET_4)

🔇 Additional comments (1)
.github/workflows/e2e.yml (1)

69-75: Artifact naming and retention look fine

Shard artifacts are uniquely named and retained briefly; aligns with merge step.

Comment thread .github/workflows/e2e.yml
Comment thread .github/workflows/e2e.yml Outdated
steebchen and others added 3 commits September 15, 2025 17:29
- Add || true to shard commands to prevent exit code 1 from failing the job
- This allows blob files to be uploaded even when tests fail
- The merge-reports step will handle the actual test result reporting

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

@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: 1

♻️ Duplicate comments (4)
.github/workflows/e2e.yml (4)

18-18: Guard PR context to avoid expression errors on non‑PR triggers

Accessing github.event.pull_request.* on workflow_dispatch will error. Gate by event_name first.

-    if: ${{ github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}
+    if: ${{ github.event_name == 'pull_request' && github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}

19-22: Disable fail‑fast so one failing shard doesn’t cancel the rest

Ensures all shards run and artifacts are collected.

     strategy:
+      fail-fast: false
       matrix:
         shard: [1, 2, 3, 4, 5]

16-27: Provision Postgres service and set a timeout

Without a DB service the shards will fail on DATABASE_URL. Add a Postgres service with healthchecks and a sane timeout.

   e2e-shards:
     runs-on: ubuntu-latest
+    timeout-minutes: 45
+    services:
+      postgres:
+        image: postgres:16
+        env:
+          POSTGRES_USER: postgres
+          POSTGRES_PASSWORD: postgres
+          POSTGRES_DB: test
+        ports:
+          - 5432:5432
+        options: >-
+          --health-cmd="pg_isready -U postgres -d test"
+          --health-interval=10s
+          --health-timeout=5s
+          --health-retries=5

45-50: Remove -x and the hardcoded DATABASE_URL; avoid secret/credential echoes

-set -x prints commands; inline basic-auth URL is flagged (CKV_SECRET_4). Use safer shell flags and move DB config to env/vars.

-      - name: Setup and run e2e shard ${{ matrix.shard }}
-        run: |
-          set -eux
-          pnpm run setup
-          export DATABASE_URL=postgres://postgres:pw@localhost:5432/test
-          E2E_TEST=true pnpm vitest run -c vitest/vitest.e2e.config.mts --no-file-parallelism --shard=${{ matrix.shard }}/5 --reporter=blob --outputFile=e2e-results-${{ matrix.shard }}.blob
+      - name: Setup and run e2e shard ${{ matrix.shard }}
+        run: |
+          set -euo pipefail
+          pnpm run setup
+          E2E_TEST=true pnpm vitest run -c vitest/vitest.e2e.config.mts --no-file-parallelism --shard=${{ matrix.shard }}/5 --reporter=blob --outputFile=e2e-results-${{ matrix.shard }}.blob
         env:
+          # Local service DB (non-secret vars; avoid embedding credentials in code)
+          PGHOST: localhost
+          PGPORT: 5432
+          PGUSER: postgres
+          PGPASSWORD: postgres
+          PGDATABASE: test
+          DATABASE_URL: postgresql://${PGUSER}:${PGPASSWORD}@${PGHOST}:${PGPORT}/${PGDATABASE}
           OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
           ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}
           INFERENCE_NET_API_KEY: ${{ secrets.INFERENCE_NET_API_KEY }}
🧹 Nitpick comments (3)
.github/workflows/e2e.yml (3)

28-31: Make version parsing resilient

Add set -euo pipefail and tolerate missing .tool-versions entries.

-      - name: Collect versions
-        run: |
-          echo "nodejs_version=$(cat .tool-versions | grep 'nodejs' | cut -d ' ' -f 2)" >> "$GITHUB_ENV"
-          echo "pnpm_version=$(cat .tool-versions | grep 'pnpm' | cut -d ' ' -f 2)" >> "$GITHUB_ENV"
+      - name: Collect versions
+        run: |
+          set -euo pipefail
+          echo "nodejs_version=$(grep -E '^nodejs ' .tool-versions | awk '{print $2}')" >> "$GITHUB_ENV" || true
+          echo "pnpm_version=$(grep -E '^pnpm ' .tool-versions | awk '{print $2}')" >> "$GITHUB_ENV" || true

110-115: Harden the merge step and publish merged artifacts

Add shell safety and upload the merged report for debugging.

-      - name: Merge reports
-        run: |
-          # Move all .blob files to root directory
-          find ./e2e-results -name "*.blob" -exec mv {} ./ \;
-          # Merge reports
-          pnpm vitest run -c vitest/vitest.e2e.config.mts --merge-reports
+      - name: Merge reports
+        run: |
+          set -euo pipefail
+          # Move all .blob files to root directory
+          find ./e2e-results -name "*.blob" -exec mv {} ./ \;
+          # Merge reports
+          pnpm vitest run -c vitest/vitest.e2e.config.mts --merge-reports
+      - name: Upload merged report
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: e2e-merged-report
+          path: |
+            ./*.blob
+            ./coverage/**/*
+          retention-days: 7

117-130: Optional: simplify the final gate

You can express the gate in job condition and skip the shell step.

-  e2e:
-    runs-on: ubuntu-latest
-    needs: [e2e-shards, merge-reports]
-    if: always()
-    steps:
-      - name: Check shard results
-        run: |
-          if [[ "${{ needs.e2e-shards.result }}" == "success" && "${{ needs.merge-reports.result }}" == "success" ]]; then
-            echo "All e2e tests passed"
-            exit 0
-          else
-            echo "E2E tests failed"
-            exit 1
-          fi
+  e2e:
+    runs-on: ubuntu-latest
+    needs: [e2e-shards, merge-reports]
+    if: always()
+    steps:
+      - name: Check shard results
+        run: |
+          [[ "${{ needs.e2e-shards.result }}" == "success" && "${{ needs.merge-reports.result }}" == "success" ]]
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7404d2b and 13a23c2.

📒 Files selected for processing (1)
  • .github/workflows/e2e.yml (1 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
.github/workflows/e2e.yml

[medium] 49-50: Basic Auth Credentials

(CKV_SECRET_4)

Comment thread .github/workflows/e2e.yml
steebchen and others added 3 commits September 15, 2025 17:20
- Create .vitest-reports directory for blob files (required by Vitest)
- Move blob files to .vitest-reports instead of root directory
- Add proper environment variables to merge-reports command

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Add if: always() to ensure upload step runs even if tests fail
- Add if-no-files-found: ignore to prevent job failure when artifact missing
- This ensures shard jobs always complete successfully for proper workflow flow

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

@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: 0

♻️ Duplicate comments (4)
.github/workflows/e2e.yml (4)

18-18: Guard PR-only context to avoid expression errors on non-PR triggers

Accessing github.event.pull_request.* unguarded can error on workflow_dispatch. Add an event gate.

-    if: ${{ github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}
+    if: ${{ github.event_name == 'pull_request' && github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}

19-22: Disable fail-fast so one failing shard doesn’t cancel others

Fail-fast can cancel remaining shards, preventing full artifact set and merge.

     strategy:
+      fail-fast: false
       matrix:
         shard: [1, 2, 3, 4, 5]

16-27: Provision Postgres service and add a sane job timeout

Shards export a DB URL but no DB is running on the runner. Add a Postgres service and a timeout.

   e2e-shards:
     runs-on: ubuntu-latest
+    timeout-minutes: 45
+    services:
+      postgres:
+        image: postgres:16
+        env:
+          POSTGRES_USER: postgres
+          POSTGRES_DB: test
+          POSTGRES_HOST_AUTH_METHOD: trust
+        ports:
+          - 5432:5432
+        options: >-
+          --health-cmd="pg_isready -U postgres -d test"
+          --health-interval=10s
+          --health-timeout=5s
+          --health-retries=5

45-51: Stop masking test failures and remove credential echoing

  • set -x echoes commands (including DB creds).
  • export DATABASE_URL with inline basic auth is flagged and unnecessary if you use a service.
  • || true makes shards always succeed, breaking the final gate.
       - name: Setup and run e2e shard ${{ matrix.shard }}
         run: |
-          set -eux
+          set -euo pipefail
           pnpm run setup
-          export DATABASE_URL=postgres://postgres:pw@localhost:5432/test
-          E2E_TEST=true pnpm vitest run -c vitest/vitest.e2e.config.mts --no-file-parallelism --shard=${{ matrix.shard }}/5 --reporter=blob --outputFile=e2e-results-${{ matrix.shard }}.blob || true
+          E2E_TEST=true pnpm vitest run -c vitest/vitest.e2e.config.mts --no-file-parallelism --shard=${{ matrix.shard }}/5 --reporter=blob --outputFile=e2e-results-${{ matrix.shard }}.blob
         env:
+          # Local service DB (ephemeral, no password; see service trust auth)
+          DATABASE_URL: postgresql://postgres@localhost:5432/test
🧹 Nitpick comments (4)
.github/workflows/e2e.yml (4)

70-77: Keep shard artifacts longer and warn on missing files

1 day is tight for debugging; prefer 7. Use warn to surface missing blobs without failing the step.

       - name: Upload shard results
         if: always()
         uses: actions/upload-artifact@v4
         with:
           name: e2e-results-${{ matrix.shard }}
           path: e2e-results-${{ matrix.shard }}.blob
-          retention-days: 1
-          if-no-files-found: ignore
+          retention-days: 7
+          if-no-files-found: warn

112-119: Upload merged outputs for inspection

Preserve merged blobs/coverage for PR triage.

       - name: Merge reports
         run: |
           # Create .vitest-reports directory and move blob files there
           mkdir -p .vitest-reports
           find ./e2e-results -name "*.blob" -exec mv {} .vitest-reports/ \;
           # Merge reports
           E2E_TEST=true pnpm vitest run -c vitest/vitest.e2e.config.mts --merge-reports
+
+      - name: Upload merged report
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: e2e-merged-report
+          path: |
+            .vitest-reports/*.blob
+            coverage/**/*
+          retention-days: 7
+          if-no-files-found: warn

28-31: Harden version collection steps

Add strict shell flags and fail fast if .tool-versions entries are missing.

-      - name: Collect versions
-        run: |
-          echo "nodejs_version=$(cat .tool-versions | grep 'nodejs' | cut -d ' ' -f 2)" >> "$GITHUB_ENV"
-          echo "pnpm_version=$(cat .tool-versions | grep 'pnpm' | cut -d ' ' -f 2)" >> "$GITHUB_ENV"
+      - name: Collect versions
+        run: |
+          set -euo pipefail
+          echo "nodejs_version=$(grep -E '^nodejs ' .tool-versions | cut -d ' ' -f 2)" >> "$GITHUB_ENV"
+          echo "pnpm_version=$(grep -E '^pnpm ' .tool-versions | cut -d ' ' -f 2)" >> "$GITHUB_ENV"

Also applies to: 89-92


1-14: Restrict default token permissions (defense-in-depth)

Minimal permissions are sufficient here.

 name: e2e
 on:
   workflow_dispatch:
   pull_request:
@@
 concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}
   cancel-in-progress: true
+
+permissions:
+  contents: read
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 13a23c2 and a24ea0f.

📒 Files selected for processing (1)
  • .github/workflows/e2e.yml (1 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
.github/workflows/e2e.yml

[medium] 49-50: Basic Auth Credentials

(CKV_SECRET_4)

⏰ 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). (1)
  • GitHub Check: e2e-shards (2)
🔇 Additional comments (2)
.github/workflows/e2e.yml (2)

112-118: LGTM: Correct Vitest merge flow

Moving blobs into .vitest-reports and invoking --merge-reports is the right pattern. Nice.


120-133: Final gate relies on shard failures surfacing—remove || true as above

Once || true is removed and fail-fast is disabled, this gate will correctly reflect failures across shards and merge-reports.

@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: 1

♻️ Duplicate comments (3)
.github/workflows/e2e.yml (3)

18-18: Guard PR-only context in job condition

Accessing github.event.pull_request.* on non-PR events (workflow_dispatch) can error. Gate by event_name first.

Apply:

-    if: ${{ github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}
+    if: ${{ github.event_name == 'pull_request' && github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}

16-23: Add Postgres service and a sane timeout

Shards rely on DATABASE_URL but no DB service is defined; localhost:5432 will fail on GitHub runners. Also add a timeout.

Apply:

   e2e-shards:
     runs-on: ubuntu-latest
+    timeout-minutes: 45
+    services:
+      postgres:
+        image: postgres:16
+        env:
+          POSTGRES_USER: postgres
+          POSTGRES_PASSWORD: postgres
+          POSTGRES_DB: test
+        ports:
+          - 5432:5432
+        options: >-
+          --health-cmd="pg_isready -U postgres -d test"
+          --health-interval=10s
+          --health-timeout=5s
+          --health-retries=5

46-51: Stop echoing and inlining DB credentials; move to env and drop -x

set -eux echoes commands and the inline export contains basic-auth (flagged by CKV_SECRET_4). Don’t print or inline secrets.

Apply:

-          set -eux
+          set -euo pipefail
           pnpm run setup
-          export DATABASE_URL=postgres://postgres:pw@localhost:5432/test
-          E2E_TEST=true pnpm vitest run -c vitest/vitest.e2e.config.mts --no-file-parallelism --shard=${{ matrix.shard }}/5 --reporter=blob --outputFile=e2e-results-${{ matrix.shard }}.blob
+          E2E_TEST=true pnpm vitest run -c vitest/vitest.e2e.config.mts --no-file-parallelism --shard=${{ matrix.shard }}/5 --reporter=blob --outputFile=e2e-results-${{ matrix.shard }}.blob
         env:
+          # Local service DB (non-secret), avoid echoing in commands
+          DATABASE_URL: postgresql://postgres:postgres@localhost:5432/test
🧹 Nitpick comments (5)
.github/workflows/e2e.yml (5)

71-78: Tweak artifact retention and missing-file behavior

Keep artifacts a bit longer and warn (not ignore) when missing for visibility.

Apply:

           path: e2e-results-${{ matrix.shard }}.blob
-          retention-days: 1
-          if-no-files-found: ignore
+          if-no-files-found: warn
+          retention-days: 7

113-120: Upload merged report artifact

Preserve merged blobs (and coverage if produced) for debugging and summaries.

Apply:

           # Merge reports
           E2E_TEST=true pnpm vitest run -c vitest/vitest.e2e.config.mts --merge-reports
+
+      - name: Upload merged report
+        uses: actions/upload-artifact@v4
+        with:
+          name: e2e-merged-report
+          path: |
+            .vitest-reports/*.blob
+            coverage/**
+          if-no-files-found: warn
+          retention-days: 7

29-33: Version discovery is fragile

Parsing .tool-versions with grep/cut can yield empty vars if file layout changes; add set -euo pipefail and validate values, or source versions from a single place (asdf/action or packageManager in package.json).

Also applies to: 90-94


1-14: Least-privilege permissions

Add minimal permissions to the workflow to reduce token scope.

Apply:

 name: e2e
 on:
   workflow_dispatch:
   pull_request:
@@
 concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}
   cancel-in-progress: true
+
+permissions:
+  contents: read

80-84: Add timeouts to remaining jobs

Protect runners from hanging merges/aggregators.

Apply:

   merge-reports:
     runs-on: ubuntu-latest
+    timeout-minutes: 20
@@
   e2e:
     runs-on: ubuntu-latest
+    timeout-minutes: 5

Also applies to: 121-124

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a24ea0f and b828f9e.

📒 Files selected for processing (1)
  • .github/workflows/e2e.yml (1 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
.github/workflows/e2e.yml

[medium] 50-51: Basic Auth Credentials

(CKV_SECRET_4)

⏰ 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). (3)
  • GitHub Check: e2e-shards (1)
  • GitHub Check: e2e-shards (3)
  • GitHub Check: e2e-shards (2)
🔇 Additional comments (1)
.github/workflows/e2e.yml (1)

19-22: Nice: fail-fast: false

Prevents shard cancellation and preserves artifacts. Good call.

Comment thread .github/workflows/e2e.yml

@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: 0

🧹 Nitpick comments (1)
apps/gateway/src/api-individual.e2e.ts (1)

120-135: Make the unsupported-model sentinel explicit (avoid 'undefined' suffix).

Rename the ambiguous sentinel to a clearly fake model; the test assertion is correct — the 400 is thrown in apps/gateway/src/chat/chat.ts (Model ${requestedModel} does not support JSON output mode).

-                model: "anthropic/claude-3-5-sonnet-20241022--undefined-wip-test",
+                model: "anthropic/claude-3-5-sonnet-20241022--no-json-test",

Optional: extract to a top-level const (e.g., UNSUPPORTED_JSON_MODEL) or add a one-line comment explaining the sentinel.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b828f9e and 9b288b7.

📒 Files selected for processing (1)
  • apps/gateway/src/api-individual.e2e.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Always use top-level import; never use require() or dynamic imports

Files:

  • apps/gateway/src/api-individual.e2e.ts
{apps/{api,gateway}/src,packages/db}/**/*.ts?(x)

📄 CodeRabbit inference engine (CLAUDE.md)

Use Drizzle ORM with the latest object syntax

Files:

  • apps/gateway/src/api-individual.e2e.ts
apps/{api,gateway}/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

For read queries, use db().query.

.findMany() or db().query.
.findFirst()

Files:

  • apps/gateway/src/api-individual.e2e.ts
**/*.e2e.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Place end-to-end tests in files matching *.e2e.ts

Place end-to-end tests in files named *.e2e.ts

Files:

  • apps/gateway/src/api-individual.e2e.ts
apps/gateway/src/api-individual.e2e.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Put isolated E2E tests in apps/gateway/src/api-individual.e2e.ts

Put isolated E2E test cases in apps/gateway/src/api-individual.e2e.ts

Files:

  • apps/gateway/src/api-individual.e2e.ts
{apps/api,apps/gateway,apps/ui,apps/docs,packages}/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Always use top-level import; never use require() or dynamic imports (e.g., import(), next/dynamic)

Files:

  • apps/gateway/src/api-individual.e2e.ts
{apps/api,apps/gateway,packages/db}/**/*.ts

📄 CodeRabbit inference engine (AGENTS.md)

{apps/api,apps/gateway,packages/db}/**/*.ts: Use Drizzle ORM with the latest object syntax for database access
For reads, use db().query.

.findMany() or db().query.
.findFirst()

Files:

  • apps/gateway/src/api-individual.e2e.ts
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
PR: theopenco/llmgateway#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T13:15:00.724Z
Learning: Applies to apps/gateway/src/api.e2e.ts : Put parallelized .each() E2E tests in apps/gateway/src/api.e2e.ts (uses concurrent mode)
Learnt from: CR
PR: theopenco/llmgateway#0
File: AGENTS.md:0-0
Timestamp: 2025-09-15T13:16:05.355Z
Learning: Applies to apps/gateway/src/api.e2e.ts : Put parallelizable .each() E2E tests in apps/gateway/src/api.e2e.ts (uses concurrent mode)
Learnt from: CR
PR: theopenco/llmgateway#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T13:15:00.724Z
Learning: Applies to apps/gateway/src/api-individual.e2e.ts : Put isolated E2E tests in apps/gateway/src/api-individual.e2e.ts
Learnt from: CR
PR: theopenco/llmgateway#0
File: AGENTS.md:0-0
Timestamp: 2025-09-15T13:16:05.355Z
Learning: Applies to apps/gateway/src/api-individual.e2e.ts : Put isolated E2E test cases in apps/gateway/src/api-individual.e2e.ts
Learnt from: CR
PR: theopenco/llmgateway#0
File: AGENTS.md:0-0
Timestamp: 2025-09-15T13:16:05.355Z
Learning: Applies to **/*.e2e.ts : Place end-to-end tests in files named *.e2e.ts
Learnt from: CR
PR: theopenco/llmgateway#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T13:15:00.724Z
Learning: Applies to **/*.e2e.ts : Place end-to-end tests in files matching *.e2e.ts
📚 Learning: 2025-09-15T13:16:05.355Z
Learnt from: CR
PR: theopenco/llmgateway#0
File: AGENTS.md:0-0
Timestamp: 2025-09-15T13:16:05.355Z
Learning: Applies to apps/gateway/src/api-individual.e2e.ts : Put isolated E2E test cases in apps/gateway/src/api-individual.e2e.ts

Applied to files:

  • apps/gateway/src/api-individual.e2e.ts
📚 Learning: 2025-09-15T13:15:00.724Z
Learnt from: CR
PR: theopenco/llmgateway#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-15T13:15:00.724Z
Learning: Applies to apps/gateway/src/api-individual.e2e.ts : Put isolated E2E tests in apps/gateway/src/api-individual.e2e.ts

Applied to files:

  • apps/gateway/src/api-individual.e2e.ts
⏰ 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). (2)
  • GitHub Check: build / run
  • GitHub Check: e2e-shards (1)

steebchen and others added 3 commits September 15, 2025 19:44
- Combine merge-reports and e2e jobs into single e2e job
- The merge-reports step will fail if tests fail, eliminating need for separate status check
- Simplifies workflow while maintaining same functionality
- Final job is named 'e2e' for branch protection compatibility

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
- Remove unnecessary cross-env-shell and DATABASE_URL from merge step
- Merge reports only processes existing blob files, no DB connection needed
- Cleaner and more efficient command

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>

@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: 0

♻️ Duplicate comments (3)
.github/workflows/e2e.yml (3)

18-18: Guard PR-only context to avoid expression errors on non-PR triggers

Add an event_name check before accessing pull_request fields.

-    if: ${{ github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}
+    if: ${{ github.event_name == 'pull_request' && github.actor != 'dependabot[bot]' && github.event.pull_request.head.repo.full_name == github.repository }}

17-23: Add DB service and a sane timeout for shards

Shards export a localhost DATABASE_URL but no Postgres service is defined; tests will flake/fail. Also set a job timeout.

   e2e-shards:
     runs-on: ubuntu-latest
+    timeout-minutes: 45
+    services:
+      postgres:
+        image: postgres:16
+        env:
+          POSTGRES_USER: postgres
+          POSTGRES_PASSWORD: postgres
+          POSTGRES_DB: test
+        ports:
+          - 5432:5432
+        options: >-
+          --health-cmd="pg_isready -U postgres -d test"
+          --health-interval=10s
+          --health-timeout=5s
+          --health-retries=5

46-52: Remove command echoing, inline DB creds, and failure masking

-set -x leaks commands; exporting a postgres://user:pass URL is a secret risk; || true hides test failures.

       - name: Setup and run e2e shard ${{ matrix.shard }}
         run: |
-          set -eux
+          set -euo pipefail
           pnpm run setup
-          export DATABASE_URL=postgres://postgres:pw@localhost:5432/test
-          E2E_TEST=true pnpm vitest run -c vitest/vitest.e2e.config.mts --no-file-parallelism --shard=${{ matrix.shard }}/5 --reporter=blob --outputFile=e2e-results-${{ matrix.shard }}.blob || true
+          E2E_TEST=true pnpm vitest run -c vitest/vitest.e2e.config.mts --no-file-parallelism --shard=${{ matrix.shard }}/5 --reporter=blob --outputFile=e2e-results-${{ matrix.shard }}.blob
         env:
+          # Use the local service DB; keep creds out of the command line
+          DATABASE_URL: postgresql://postgres:postgres@localhost:5432/test
           OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
           ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}
           INFERENCE_NET_API_KEY: ${{ secrets.INFERENCE_NET_API_KEY }}
           TOGETHER_AI_API_KEY: ${{ secrets.TOGETHER_AI_API_KEY }}
           GOOGLE_AI_STUDIO_API_KEY: ${{ secrets.GOOGLE_AI_STUDIO_API_KEY }}
           CLOUD_RIFT_API_KEY: ${{ secrets.CLOUD_RIFT_API_KEY }}
           MISTRAL_API_KEY: ${{ secrets.MISTRAL_API_KEY }}
           X_AI_API_KEY: ${{ secrets.X_AI_API_KEY }}
           GROQ_API_KEY: ${{ secrets.GROQ_API_KEY }}
           DEEPSEEK_API_KEY: ${{ secrets.DEEPSEEK_API_KEY }}
           PERPLEXITY_API_KEY: ${{ secrets.PERPLEXITY_API_KEY }}
           NOVITA_AI_API_KEY: ${{ secrets.NOVITA_AI_API_KEY }}
           MOONSHOT_API_KEY: ${{ secrets.MOONSHOT_API_KEY }}
           ALIBABA_API_KEY: ${{ secrets.ALIBABA_API_KEY }}
           NEBIUS_API_KEY: ${{ secrets.NEBIUS_API_KEY }}
           Z_AI_API_KEY: ${{ secrets.Z_AI_API_KEY }}
           ROUTEWAY_API_KEY: ${{ secrets.ROUTEWAY_API_KEY }}
🧹 Nitpick comments (5)
.github/workflows/e2e.yml (5)

71-79: Make shard artifacts more useful for debugging

Keep artifacts a bit longer and warn if missing instead of ignoring.

       - name: Upload shard results
         if: always()
         uses: actions/upload-artifact@v4
         with:
           name: e2e-results-${{ matrix.shard }}
           path: e2e-results-${{ matrix.shard }}.blob
-          retention-days: 1
-          if-no-files-found: ignore
+          retention-days: 7
+          if-no-files-found: warn

80-84: Add timeout to merge job

Prevents hanging merges from blocking the single required check.

   e2e:
     runs-on: ubuntu-latest
+    timeout-minutes: 20
     needs: e2e-shards
     if: always()

107-112: Optionally flatten downloads to avoid find/mv

download-artifact can merge multiple into a single folder.

       - name: Download all shard results
         uses: actions/download-artifact@v4
         with:
           path: ./e2e-results
           pattern: e2e-results-*
+          merge-multiple: true

113-119: Harden merge step and preserve merged outputs

Add strict shell flags and upload merged artifacts for later inspection.

       - name: Merge reports and show final results
         run: |
-          # Create .vitest-reports directory and move blob files there
+          set -euo pipefail
+          # Create .vitest-reports directory and move blob files there
           mkdir -p .vitest-reports
-          find ./e2e-results -name "*.blob" -exec mv {} .vitest-reports/ \;
+          find ./e2e-results -name "*.blob" -exec mv {} .vitest-reports/ \;
           # Merge reports - this will fail if tests failed, providing the final status
           pnpm vitest run -c vitest/vitest.e2e.config.mts --merge-reports
+      - name: Upload merged report
+        if: always()
+        uses: actions/upload-artifact@v4
+        with:
+          name: e2e-merged-report
+          path: |
+            .vitest-reports/*.blob
+            coverage/**
+          retention-days: 7

1-14: Set minimal default permissions at workflow level

Reduces token scope for all jobs.

 name: e2e
 on:
   workflow_dispatch:
   pull_request:
     paths:
       - "apps/gateway/**"
       - "packages/models/**"
       - "**/*.e2e.ts"
       - ".github/workflows/e2e.yml"
 
+permissions:
+  contents: read
+
 concurrency:
   group: ${{ github.workflow }}-${{ github.ref }}
   cancel-in-progress: true
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9b288b7 and baf01bd.

📒 Files selected for processing (1)
  • .github/workflows/e2e.yml (1 hunks)
🧰 Additional context used
🪛 Checkov (3.2.334)
.github/workflows/e2e.yml

[medium] 50-51: Basic Auth Credentials

(CKV_SECRET_4)

⏰ 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). (3)
  • GitHub Check: e2e-shards (3)
  • GitHub Check: e2e-shards (1)
  • GitHub Check: e2e-shards (2)
🔇 Additional comments (2)
.github/workflows/e2e.yml (2)

19-22: Good: fail-fast disabled for matrix

Keeps other shards running and artifacts available even if one fails.


80-120: Confirm single-check branch protection behavior

With two jobs under one workflow, some repos protect by workflow name (“e2e”), others by job names. Ensure branch protection targets the workflow run or only the final e2e job, not both.

Would you like me to adjust job names or add a noop “gate” job to guarantee a single required check?

@steebchen
steebchen merged commit 8fcfe55 into main Sep 15, 2025
15 checks passed
@steebchen
steebchen deleted the terragon/e2e-vitest-shards-github-matrix branch September 15, 2025 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant