Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR splits the previous single test matrix into three GitHub Actions jobs: Sequence Diagram(s)sequenceDiagram
participant GH as GitHub Actions
participant Runner as CI Runner
participant NPM as npm (scripts)
participant Postgres as Postgres
participant Oriole as OrioleDB
participant Coveralls as Coveralls
GH->>Runner: Start lint_build job
Runner->>Runner: setup Node "24"\nnpm ci
Runner->>Runner: run lint
Runner->>Runner: run build
GH->>Runner: Start test_postgres job
Runner->>Postgres: start Postgres service (test_env)
Runner->>Runner: npm ci\nrun migrations
Runner->>Postgres: pg_dump (before)
Runner->>Runner: run tests (Jest)
Runner->>Postgres: pg_dump (after)
Runner->>Runner: compare dumps (idempotency)
Runner->>Coveralls: upload coverage
GH->>Runner: Start test_oriole job
Runner->>NPM: npm run test:oriole
NPM->>Oriole: infra:restart:oriole
NPM->>Runner: provision dummy data
Runner->>Runner: npm ci\nrun jest --runInBand (oriole tests)
Runner->>Oriole: pg_dump (before)
Runner->>Oriole: pg_dump (after)
Runner->>Runner: compare dumps (idempotency)
Runner->>Coveralls: upload coverage
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
b8db2b8 to
ecf0b52
Compare
| "docs:export": "tsx ./src/scripts/export-docs.ts", | ||
| "test:dummy-data": "tsx -r dotenv/config ./src/test/db/import-dummy-data.ts", | ||
| "test": "npm run infra:restart && npm run test:dummy-data && jest --no-cache --runInBand", | ||
| "test:oriole": "npm run infra:restart:oriole && npm run test:dummy-data && jest --no-cache --runInBand --testPathIgnorePatterns=src/test/vectors.test.ts", |
There was a problem hiding this comment.
ignore vectors until implementation is ready
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
23-33:⚠️ Potential issue | 🟠 MajorPin GitHub Actions to immutable commit SHAs.
Using floating refs (
@v6,@v5) weakens CI supply-chain guarantees because tags can be retargeted. Pin all action usages to their full 40-character commit SHAs and include the version in a comment for readability.This applies to:
actions/checkout@v6(lines 23, 75, 120)actions/cache@v5(lines 24, 76, 121)actions/setup-node@v6(lines 31, 83, 128)Example format:
- uses: actions/checkout@e2f20e631e6b0fcc4f4191560f8f4a78d1c65f25 # v6.0.0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 23 - 33, Replace floating action tags with immutable 40-char commit SHAs for each GitHub Action usage: update uses: actions/checkout@v6, uses: actions/cache@v5, and uses: actions/setup-node@v6 to their corresponding commit SHAs (and add the semantic version as an end-of-line comment like "# v6.0.0") so the workflow pins to exact commits; ensure you update all occurrences referenced in the review (checkout, cache, setup-node) consistently across the file.
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
90-91: Avoid world-writable permissions for test artifacts unless strictly required.
chmod -R 777 datais broader than needed. Prefer least-privilege perms to reduce accidental cross-step write access.Proposed permission tightening
- mkdir -p data && chmod -R 777 data && \ + mkdir -p data && chmod -R u+rwX,go-rwx data && \ npm run test:coverage- mkdir -p data && chmod -R 777 data && \ + mkdir -p data && chmod -R u+rwX,go-rwx data && \ npm run test:orioleAlso applies to: 135-136
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 90 - 91, Replace broad world-writable chmod with least-privilege permissions: change the pipeline step that currently runs "mkdir -p data && chmod -R 777 data" to create the directory and set tighter perms such as "mkdir -p data && chmod -R 0775 data" (or "0755" if only the runner needs write) so only owner/group (not everyone) can write; update the other occurrence referenced in the workflow (the similar chmod at lines 135-136) the same way. Ensure the permission choice matches test needs (write for owner/group vs owner-only).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 23-33: Replace floating action tags with immutable 40-char commit
SHAs for each GitHub Action usage: update uses: actions/checkout@v6, uses:
actions/cache@v5, and uses: actions/setup-node@v6 to their corresponding commit
SHAs (and add the semantic version as an end-of-line comment like "# v6.0.0") so
the workflow pins to exact commits; ensure you update all occurrences referenced
in the review (checkout, cache, setup-node) consistently across the file.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 90-91: Replace broad world-writable chmod with least-privilege
permissions: change the pipeline step that currently runs "mkdir -p data &&
chmod -R 777 data" to create the directory and set tighter perms such as "mkdir
-p data && chmod -R 0775 data" (or "0755" if only the runner needs write) so
only owner/group (not everyone) can write; update the other occurrence
referenced in the workflow (the similar chmod at lines 135-136) the same way.
Ensure the permission choice matches test needs (write for owner/group vs
owner-only).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 5ecaf1cf-3312-4518-89cd-b611d27c9460
📒 Files selected for processing (2)
.github/workflows/ci.ymlpackage.json
🚧 Files skipped from review as they are similar to previous changes (1)
- package.json
ecf0b52 to
91bd99e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 19: The test:oriole npm script currently restarts Oriole and immediately
runs the seeding script which does a single DB connect attempt (see
"test:oriole" in package.json and src/test/db/import-dummy-data.ts where it
calls the DB connect), causing flakey CI when the DB is not ready; fix by gating
seeding on DB readiness either by adding a small wait-for/retry loop before
invoking import-dummy-data.ts or by adding connect-with-retries inside
import-dummy-data.ts (implement exponential backoff or fixed retries around the
existing connect call), ensuring the seeder only proceeds once a successful
connection is confirmed and exits non-zero after max retries so failures fail
the job.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 5fef5869-dd32-4bb3-899c-3dbac4c0a9b8
📒 Files selected for processing (2)
.github/workflows/ci.ymlpackage.json
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
91bd99e to
1e4624f
Compare
What kind of change does this PR introduce?
CI Feature
What is the current behavior?
Only migrations were run with orioledb.
What is the new behavior?
All tests also run with orioledb.
Additional context
Will use for vector bucket compatibility check.
Coverage is only sent for postgres run.