chore: sync docs, ADR, and workflow versions with CD fixes#119
Conversation
|
Ready to act? Review this PR in Change Stack to turn feedback into patch suggestions you can inspect and refine. Warning Review limit reached
More reviews will be available in 54 minutes and 22 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR modernizes the project's GitHub Actions setup by upgrading to newer action versions, documenting the versioning strategy, and updating project documentation to reflect new tooling practices. The changes establish a consistent major-version pinning policy across frontend and backend CI/CD workflows while adding comprehensive benchmark planning and changelog maintenance. ChangesGitHub Actions modernization and documentation
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Replace commit SHA pins with version tags in all workflow files - Bump actions/checkout to @v6 (node24), keep blocked actions at current versions - Add annotated tag convention to README CI/CD table - Create ADR 002 for CI/CD action version strategy - Create benchmark Phase 0 plan (fair scenario isolation design)
samir1498
force-pushed
the
chore/sync-docs-and-cd-fixes
branch
from
62a8265 to
7bea75f
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
benchmark/PLAN.md (1)
19-19: ⚡ Quick winConsider testing both ORM and raw query strategies.
The current rule mandates choosing either ORM or raw query builder for all stacks. While this ensures fairness within a single benchmark run, it may hide important insights. Consider adding a scenario variant that tests both strategies (ORM-based CRUD and raw query-based CRUD) to identify whether ORM overhead is significant. This would help attribute performance differences more precisely and provide actionable guidance for users choosing between ORM and raw queries.
🤖 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 `@benchmark/PLAN.md` at line 19, Update the benchmark plan to add a scenario variant that tests both ORM-based CRUD and raw query-based CRUD in addition to the existing rule "Same DB access strategy across stacks: all ORM or all raw query builder." Specifically, add a new test case description (referencing the existing sentence "Same DB access strategy across stacks: **all ORM or all raw query builder.** No mixing ORM in one and raw SQL in another.") that runs the benchmark twice per stack—once using the ORM implementations and once using raw query implementations—so results can be compared for ORM overhead and attribution of performance differences..github/workflows/ci-backend.yml (1)
17-17: ⚡ Quick winAdd
persist-credentials: falseto all checkout steps for consistency.All seven jobs in this workflow are missing
persist-credentials: falseon the checkout step, while other workflows (cd.yml, ci-backend-springboot.yml, ci-e2e.yml, ci.yml) consistently include this security setting.🔒 Add persist-credentials to all checkout steps
Example for the first job (repeat for all 7 jobs):
- uses: actions/checkout@v6 + with: + persist-credentials: false - uses: pnpm/action-setup@v4Also applies to: 35-35, 53-53, 73-73, 92-92, 111-111, 130-130
🤖 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/ci-backend.yml at line 17, Add the security option persist-credentials: false to every checkout step that uses actions/checkout@v6 in this workflow (all seven jobs) so the checkout steps do not leave GitHub token credentials in the workspace; locate each step with "uses: actions/checkout@v6" and add the persist-credentials: false input to the step configuration.Source: Linters/SAST tools
.github/workflows/cd-backend-springboot.yml (1)
20-22: ⚡ Quick winAdd
persist-credentials: falseto checkout step for consistency.The CD workflow for Spring Boot doesn't set
persist-credentials: false, while other workflows (cd.yml, ci-backend-springboot.yml, ci-e2e.yml, ci.yml) consistently include this setting. This is a security best practice to prevent the GitHub token from persisting in the workspace.🔒 Add persist-credentials setting
- uses: actions/checkout@v6 + with: + persist-credentials: false - uses: actions/setup-java@v5🤖 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/cd-backend-springboot.yml around lines 20 - 22, The checkout step using actions/checkout@v6 should include the security option persist-credentials: false; update the workflow's checkout step (the actions/checkout@v6 entry) to add a with block containing persist-credentials: false so the GitHub token is not persisted into the workspace, matching other workflows like cd.yml and ci-backend-springboot.yml.Source: Linters/SAST tools
🤖 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 `@benchmark/PLAN.md`:
- Around line 131-142: Add a single blank line immediately before the "Resource
usage:" table so the Markdown parser renders the table correctly; locate the
"Resource usage:" heading in PLAN.md and insert an empty line between that
heading and the table (the pipe-delimited table starting with "| Metric | NestJS
(Express) ...") to fix the formatting.
- Around line 119-130: Add a single blank line immediately before the Markdown
table that begins with the header "| Metric | NestJS (Express) | NestJS
(Fastify) | Spring JVM | Spring Native | Go |" so the table is separated from
the preceding paragraph; update the PLAN.md content where the table header
string appears to insert one newline above it to ensure correct Markdown
rendering.
- Line 89: Update PLAN.md so the PostgreSQL CPU allocation rule is consistent:
either change the hard rule phrase "Never run PostgreSQL on the same CPU
allocation as the backend" to the softer "Prefer PostgreSQL on separate CPU
allocation if possible" or alternately change the later line "Prefer PostgreSQL
on separate CPU allocation if possible" to the strict "Never run PostgreSQL on
the same CPU allocation as the backend"; pick one policy and apply it to both
occurrences (the exact phrases shown) so the document contains a single,
unambiguous rule.
In `@docs/adr/002-ci-cd-action-version-strategy.md`:
- Around line 14-16: Update ADR 002 wording to correct the Node.js deprecation
timeline and action compatibility: replace the statement that "runners will stop
supporting Node.js 20 on June 16, 2026" with that June 16, 2026 makes Node.js 24
the default while Node.js 20 remains temporarily available via
ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION and that full removal is expected in
fall 2026; update the compatibility claims about setup actions to note that
actions/setup-node@v4 supports node-version: 24 and that pnpm/action-setup@v4
supports Node.js 24 starting at v4.4.0 (mention the minimum tag/version rather
than implying they require Node.js 20); apply these wording fixes where the ADR
currently asserts the June 16 date and the action compatibility (the instances
you noted).
---
Nitpick comments:
In @.github/workflows/cd-backend-springboot.yml:
- Around line 20-22: The checkout step using actions/checkout@v6 should include
the security option persist-credentials: false; update the workflow's checkout
step (the actions/checkout@v6 entry) to add a with block containing
persist-credentials: false so the GitHub token is not persisted into the
workspace, matching other workflows like cd.yml and ci-backend-springboot.yml.
In @.github/workflows/ci-backend.yml:
- Line 17: Add the security option persist-credentials: false to every checkout
step that uses actions/checkout@v6 in this workflow (all seven jobs) so the
checkout steps do not leave GitHub token credentials in the workspace; locate
each step with "uses: actions/checkout@v6" and add the persist-credentials:
false input to the step configuration.
In `@benchmark/PLAN.md`:
- Line 19: Update the benchmark plan to add a scenario variant that tests both
ORM-based CRUD and raw query-based CRUD in addition to the existing rule "Same
DB access strategy across stacks: all ORM or all raw query builder."
Specifically, add a new test case description (referencing the existing sentence
"Same DB access strategy across stacks: **all ORM or all raw query builder.** No
mixing ORM in one and raw SQL in another.") that runs the benchmark twice per
stack—once using the ORM implementations and once using raw query
implementations—so results can be compared for ORM overhead and attribution of
performance differences.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e159cc1a-222a-4c5f-8a32-2467eba55e5b
📒 Files selected for processing (10)
.github/workflows/cd-backend-springboot.yml.github/workflows/cd.yml.github/workflows/ci-backend-springboot.yml.github/workflows/ci-backend.yml.github/workflows/ci-e2e.yml.github/workflows/ci.ymlREADME.mdbenchmark/PLAN.mddocs/adr/002-ci-cd-action-version-strategy.mdfrontend/CHANGELOG.md
| - Same seed data (identical rows, same IDs) | ||
| - Explicit identical pool size (e.g. 10 connections) | ||
| - Capture PostgreSQL CPU/RAM, active/idle connections, locks, slow queries, query count, query latency, pool wait, transaction duration | ||
| - Prefer PostgreSQL on separate CPU allocation if possible |
There was a problem hiding this comment.
Inconsistency with Hard Rules: PostgreSQL CPU allocation.
Line 21 states "Never run PostgreSQL on the same CPU allocation as the backend" (hard rule), but line 89 says "Prefer PostgreSQL on separate CPU allocation if possible" (optional). This inconsistency could lead to confusion during execution. Either enforce the hard rule consistently or relax line 21 to match line 89.
📝 Proposed fix to align the constraint
-Prefer PostgreSQL on separate CPU allocation if possible
+PostgreSQL must run on separate CPU allocation (as per Hard Rules)📝 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.
| - Prefer PostgreSQL on separate CPU allocation if possible | |
| - PostgreSQL must run on separate CPU allocation (as per Hard Rules) |
🤖 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 `@benchmark/PLAN.md` at line 89, Update PLAN.md so the PostgreSQL CPU
allocation rule is consistent: either change the hard rule phrase "Never run
PostgreSQL on the same CPU allocation as the backend" to the softer "Prefer
PostgreSQL on separate CPU allocation if possible" or alternately change the
later line "Prefer PostgreSQL on separate CPU allocation if possible" to the
strict "Never run PostgreSQL on the same CPU allocation as the backend"; pick
one policy and apply it to both occurrences (the exact phrases shown) so the
document contains a single, unambiguous rule.
| Per scenario: | ||
| | Metric | NestJS (Express) | NestJS (Fastify) | Spring JVM | Spring Native | Go | | ||
| |--------|------------------|------------------|------------|---------------|-----| | ||
| | avg | | | | | | | ||
| | med | | | | | | | ||
| | p90 | | | | | | | ||
| | p95 | | | | | | | ||
| | p99 | | | | | | | ||
| | max | | | | | | | ||
| | failed | | | | | | | ||
| | req/s | | | | | | | ||
|
|
There was a problem hiding this comment.
Add blank line before table for proper markdown formatting.
The table at line 120 should be preceded by a blank line to comply with markdown best practices and avoid rendering issues in some markdown parsers.
📝 Proposed fix
Per scenario:
+
| Metric | NestJS (Express) | NestJS (Fastify) | Spring JVM | Spring Native | Go |📝 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.
| Per scenario: | |
| | Metric | NestJS (Express) | NestJS (Fastify) | Spring JVM | Spring Native | Go | | |
| |--------|------------------|------------------|------------|---------------|-----| | |
| | avg | | | | | | | |
| | med | | | | | | | |
| | p90 | | | | | | | |
| | p95 | | | | | | | |
| | p99 | | | | | | | |
| | max | | | | | | | |
| | failed | | | | | | | |
| | req/s | | | | | | | |
| Per scenario: | |
| | Metric | NestJS (Express) | NestJS (Fastify) | Spring JVM | Spring Native | Go | | |
| |--------|------------------|------------------|------------|---------------|-----| | |
| | avg | | | | | | | |
| | med | | | | | | | |
| | p90 | | | | | | | |
| | p95 | | | | | | | |
| | p99 | | | | | | | |
| | max | | | | | | | |
| | failed | | | | | | | |
| | req/s | | | | | | | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 120-120: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 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 `@benchmark/PLAN.md` around lines 119 - 130, Add a single blank line
immediately before the Markdown table that begins with the header "| Metric |
NestJS (Express) | NestJS (Fastify) | Spring JVM | Spring Native | Go |" so the
table is separated from the preceding paragraph; update the PLAN.md content
where the table header string appears to insert one newline above it to ensure
correct Markdown rendering.
Source: Linters/SAST tools
| Resource usage: | ||
| | Metric | NestJS (Express) | NestJS (Fastify) | Spring JVM | Spring Native | Go | | ||
| |--------|------------------|------------------|------------|---------------|-----| | ||
| | CPU avg | | | | | | | ||
| | CPU max | | | | | | | ||
| | RAM avg | | | | | | | ||
| | RAM max | | | | | | | ||
| | GC/event-loop delay | | | | | | | ||
| | DB query count | | | | | | | ||
| | DB latency | | | | | | | ||
| | DB pool wait | | | | | | | ||
|
|
There was a problem hiding this comment.
Add blank line before table for proper markdown formatting.
The table at line 132 should be preceded by a blank line to comply with markdown best practices and avoid rendering issues in some markdown parsers.
📝 Proposed fix
Resource usage:
+
| Metric | NestJS (Express) | NestJS (Fastify) | Spring JVM | Spring Native | Go |📝 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.
| Resource usage: | |
| | Metric | NestJS (Express) | NestJS (Fastify) | Spring JVM | Spring Native | Go | | |
| |--------|------------------|------------------|------------|---------------|-----| | |
| | CPU avg | | | | | | | |
| | CPU max | | | | | | | |
| | RAM avg | | | | | | | |
| | RAM max | | | | | | | |
| | GC/event-loop delay | | | | | | | |
| | DB query count | | | | | | | |
| | DB latency | | | | | | | |
| | DB pool wait | | | | | | | |
| Resource usage: | |
| | Metric | NestJS (Express) | NestJS (Fastify) | Spring JVM | Spring Native | Go | | |
| |--------|------------------|------------------|------------|---------------|-----| | |
| | CPU avg | | | | | | | |
| | CPU max | | | | | | | |
| | RAM avg | | | | | | | |
| | RAM max | | | | | | | |
| | GC/event-loop delay | | | | | | | |
| | DB query count | | | | | | | |
| | DB latency | | | | | | | |
| | DB pool wait | | | | | | |
🧰 Tools
🪛 markdownlint-cli2 (0.22.1)
[warning] 132-132: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
🤖 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 `@benchmark/PLAN.md` around lines 131 - 142, Add a single blank line
immediately before the "Resource usage:" table so the Markdown parser renders
the table correctly; locate the "Resource usage:" heading in PLAN.md and insert
an empty line between that heading and the table (the pipe-delimited table
starting with "| Metric | NestJS (Express) ...") to fix the formatting.
Source: Linters/SAST tools
| Additionally, GitHub Actions runners will stop supporting the Node.js 20 | ||
| runtime on **June 16, 2026**, requiring actions to target Node.js 24 or | ||
| later. |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
When does GitHub Actions stop supporting Node.js 20 runtime?
💡 Result:
GitHub Actions is currently in the process of deprecating Node.js 20. Following the transition to Node.js 24 as the default runtime, Node.js 20 support will be removed in the fall of 2026 [1]. Key milestones for this transition include: - June 16, 2026: Runners will begin using Node.js 24 by default [1]. - Post-June 16, 2026: Users who require additional time can temporarily continue using Node.js 20 by setting the environment variable ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION=true in their workflows or on their runner machines [1]. - Fall 2026: Node.js 20 support will be fully removed, and this opt-out mechanism will no longer be available [1]. Node.js 20 itself reached its official end-of-life (EOL) in April 2026 [1]. GitHub strongly recommends that action maintainers and users update their actions to run on Node.js 24 to ensure continued compatibility and security [1].
Citations:
🌐 Web query:
Does actions/setup-node@v4 support Node.js 24 runtime?
💡 Result:
Yes—actions/setup-node@v4 can install Node.js 24. Evidence: the setup-node action’s current README documents that specifying node-version: 24 is supported (and node-version accepts major versions including “24”). [1] This is consistent with the action’s evolution that added support for Node 24 (noted as “Upgraded action from node20 to node24” in the repo history). [2] Caveat: older v4 code examples and docs snapshots may list only earlier major versions, but the action’s version selection mechanism is explicitly designed to pull requested Node versions from actions/node-versions when available. [3] Practical check: in your workflow, set node-version to 24 (example for latest major v6 shown in docs; same input exists in v4): uses: actions/setup-node@v4 with: node-version: 24
Citations:
- 1: https://github.com/actions/setup-node
- 2: https://github.com/actions/setup-node/tree/refs/heads/main
- 3: https://github.com/actions/setup-node/tree/5ef044f9d09786428e6e895be6be17937becee3a
🌐 Web query:
Does pnpm/action-setup@v4 support Node.js 24 runtime?
💡 Result:
Yes, pnpm/action-setup version 4 supports the Node.js 24 runtime, starting from version 4.4.0 [1]. While earlier iterations of the v4 series (such as v4.3.0) were primarily built on Node.js 20, they faced deprecation warnings as GitHub transitioned runners to Node.js 24 [2]. To address this, pnpm/action-setup v4.4.0 was released, which updated the action to run on Node.js 24 [1][3]. If you are using an older version of the v4 branch, it is recommended to update to at least v4.4.0 or the latest available version (v5 or higher) to ensure compatibility with the updated GitHub Actions runtime requirements [1][2].
Citations:
- 1: Bump pnpm/action-setup from 4 to 6 actions/setup-node#1528
- 2: Handle Node.js 20 actions deprecation on github pnpm/action-setup#209
- 3: pnpm/action-setup@fc06bc1
Fix Node.js 20 deprecation timing + action compatibility claims in ADR 002
- The ADR’s “stop supporting Node.js 20 on June 16, 2026” wording is off: June 16 is when Node.js 24 becomes the default runtime, with Node.js 20 remaining temporarily available (via
ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION) and full removal in fall 2026. actions/setup-node@v4supportsnode-version: 24;pnpm/action-setup@v4supports Node.js 24 starting with v4.4.0—so don’t state/imply these still require Node.js 20 without calling out the minimum tag/version.- Applies to lines 14-16 and also 29-32, 45-46.
🤖 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 `@docs/adr/002-ci-cd-action-version-strategy.md` around lines 14 - 16, Update
ADR 002 wording to correct the Node.js deprecation timeline and action
compatibility: replace the statement that "runners will stop supporting Node.js
20 on June 16, 2026" with that June 16, 2026 makes Node.js 24 the default while
Node.js 20 remains temporarily available via
ACTIONS_ALLOW_USE_UNSECURE_NODE_VERSION and that full removal is expected in
fall 2026; update the compatibility claims about setup actions to note that
actions/setup-node@v4 supports node-version: 24 and that pnpm/action-setup@v4
supports Node.js 24 starting at v4.4.0 (mention the minimum tag/version rather
than implying they require Node.js 20); apply these wording fixes where the ADR
currently asserts the June 16 date and the action compatibility (the instances
you noted).
1 participant
Summary
actions/checkoutto@v6(node24 runtime) where SHAs were usedChanges
ci.yml,ci-backend.yml,cd.yml(Spring Boot workflows already fixed)002-ci-cd-action-version-strategy.mddocumenting version tag preference, node24 migration strategy, and annotated tag requirementPLAN.mdwith bias-resistant design covering NestJS (Express + Fastify), Spring Boot (JVM + Native), and Go — 7 isolated scenarios, PostgreSQL isolation rules, >=3 repeats with variance reporting, and bias audit checklistfrontend/CHANGELOG.mdTest plan
Summary by CodeRabbit
Documentation
Chores