-
-
Notifications
You must be signed in to change notification settings - Fork 3
feat: add name and version to GET /__health endpoint
#238
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe health check endpoint Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant APIv1 Router
participant package.json
Client->>APIv1 Router: GET /api/v1/__health
APIv1 Router->>package.json: Read name, version
APIv1 Router-->>Client: JSON { status, timestamp, name, version }
Possibly related PRs
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
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 (5)
src/httpServer/routers/apiV1.js (1)
2-2: Consider centralisingpackage.jsonmetadata import
Multiple modules/tests nowrequire('../../../package.json'). Duplicating the relative-path dance is brittle (any move of this file breaks the path) and complicates bundling.
Create a tiny helper, e.g.src/utils/pkg.js, that exports{ name, version }once and reuse it everywhere.-const pkg = require('../../../package.json') +const { name, version } = require('../../utils/pkg')__tests__/httpServer/apiV1.test.js (2)
3-3: Mirror helper usage in tests once refactor is applied
If you adopt the suggestedutils/pkghelper, import it here too to keep test paths resilient:-const pkg = require('../../package.json') +const { name, version } = require('../../src/utils/pkg')
39-40: Assertion order / explicitness nitpick
To avoid a false positive where the endpoint returns the right keys but wrong values, assert both fields together:-expect(response.body).toHaveProperty('version', pkg.version) -expect(response.body).toHaveProperty('name', pkg.name) +expect(response.body).toMatchObject({ version: pkg.version, name: pkg.name })Purely stylistic; feel free to ignore.
src/httpServer/swagger/api-v1.yml (2)
31-36: Example values drift risk
example: 1.0.0andvisionboardare now hard-coded. As soon as the real package version changes, the example becomes outdated.
Either reference the real value during the CI build (e.g. through a templating step) or use a placeholder likex.y.zto avoid stale docs.
37-42: Required list already enforces presence – consider property ordering
additionalProperties: falseplusrequiredis good.
Minor: keep therequiredarray alphabetically sorted (name,status,timestamp,version) to reduce merge conflicts and improve diff readability.🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 42-42: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
__tests__/httpServer/apiV1.test.js(2 hunks)src/httpServer/routers/apiV1.js(1 hunks)src/httpServer/swagger/api-v1.yml(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/httpServer/routers/apiV1.js (1)
__tests__/httpServer/apiV1.test.js (2)
pkg(3-3)require(2-2)
__tests__/httpServer/apiV1.test.js (1)
src/httpServer/routers/apiV1.js (3)
pkg(2-2)require(1-1)require(3-3)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Playwright Tests
🔇 Additional comments (1)
src/httpServer/routers/apiV1.js (1)
8-10: Publicly exposing version may aid reconnaissance
Returning the exactpackage.jsonversion helps users, but also gives attackers an easy fingerprint of the running code.
If external consumers don’t strictly need it, consider hiding the patch component (e.g.1.2.x) or gating the full value behind authentication.
Related #216
Summary by CodeRabbit