docs(api): wire app.title/description and domain into OpenAPI spec#3462
docs(api): wire app.title/description and domain into OpenAPI spec#3462PierreBrisorgueil merged 2 commits intomasterfrom
Conversation
The spec served at /api/spec.json was advertising empty info.title and localhost as the server URL for every downstream project, so Scalar rendered the reference with no title and its Try-It samples all pointed to http://localhost:3000 regardless of environment. The correct values already lived in config (app.title, app.description, domain) and were even wired into app.locals. Inject them into the spec in initSwagger right before the guides helper appends the markdown sidebar, so info.description remains the concatenation of config + guides. Drop the redundant servers: block from modules/core/doc/index.yml so the code remains the single source of truth. Closes #3461
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 48 minutes and 9 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe pull request wires application configuration values into the OpenAPI specification served at Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3462 +/- ##
==========================================
+ Coverage 85.81% 85.82% +0.01%
==========================================
Files 115 115
Lines 2911 2914 +3
Branches 805 807 +2
==========================================
+ Hits 2498 2501 +3
Misses 327 327
Partials 86 86 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR makes the runtime-served OpenAPI spec (/api/spec.json) reflect each downstream project’s configured identity (title/description) and public base URL (domain), rather than relying on hardcoded YAML values.
Changes:
- Inject
config.app.title,config.app.description, andconfig.domaininto the merged OpenAPI spec ininitSwagger(prior to merging markdown guides). - Remove the hardcoded
serversblock frommodules/core/doc/index.ymlto keep runtime config as the single source of truth. - Add/adjust unit + integration tests to assert the injected
info.title,info.description, andservers[0].urlbehavior (including fallback).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
lib/services/express.js |
Injects runtime info metadata and servers into the merged spec before guides are appended. |
modules/core/doc/index.yml |
Removes hardcoded servers entry to avoid conflicting with runtime config. |
modules/core/tests/core.unit.tests.js |
Adds unit coverage for metadata/server injection and domain fallback + guide append ordering. |
modules/core/tests/core.integration.tests.js |
Tightens assertions to require non-empty title/description and a populated servers array in the served spec. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@modules/core/tests/core.integration.tests.js`:
- Line 307: The current assertion only checks type for res.body.info.description
and would allow an empty string; update the test to assert the description is a
non-empty string by adding a second expectation that checks
res.body.info.description has length > 0 or matches a non-whitespace regex
(e.g., expect(res.body.info.description.length).toBeGreaterThan(0) or
expect(res.body.info.description).toMatch(/\S/)), keeping the original type
check for clarity.
In `@modules/core/tests/core.unit.tests.js`:
- Around line 645-685: The tests "should inject info.title, info.description and
servers from config" and "should fall back to localhost in servers when
config.domain is empty" rely on global config.files.guides and can fail if
guides are present; update both tests to explicitly isolate guides by setting
config.files to include guides: [] (e.g. config.files = { ...config.files,
swagger: [...], guides: [] }) before calling expressService.initSwagger and
restore as needed so the handler behavior is deterministic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 90c62bcc-e0bf-4c8c-946c-3ce1a672bef3
📒 Files selected for processing (4)
lib/services/express.jsmodules/core/doc/index.ymlmodules/core/tests/core.integration.tests.jsmodules/core/tests/core.unit.tests.js
💤 Files with no reviewable changes (1)
- modules/core/doc/index.yml
- integration: assert info.description is a non-empty string, not just typed - unit: explicitly set guides: [] in the two new injection tests so the description equality assertion stays deterministic regardless of any default config.files.guides
|
@coderabbitai Both points addressed in 241143e:
|
|
Tip For best results, initiate chat on the files or code changes.
All good! 🎉 (_/) 🧠 Learnings usedIf you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Summary
info.title,info.descriptionandservers[0].urlinto the merged OpenAPI spec fromconfig.app.title,config.app.descriptionandconfig.domainininitSwagger, right before the markdown guides helper appends its sidebar content. Each downstream project (trawl, pierreb, comes, montaine...) now advertises its own identity and public domain at/api/spec.json, so Scalar renders the reference with the right title and its Try-It samples hit the real API base URL.servers:block frommodules/core/doc/index.ymlso the code remains the single source of truth (the YAML was hardcoded tohttp://localhost:3000and competed with the config).info.descriptionfrom config first, then letguidesHelper.mergeGuidesIntoSpecconcatenate markdown guides after ([existing, ...sections].filter(Boolean).join('\n\n')).Closes #3461
Test plan
npm run lintcleannpm run test:coverageall 64 suites / 769 tests green, coverage thresholds respectedcore.unit.tests.jscover:info.title/info.descriptioninjection, customconfig.domain, empty-domain fallback tohttp://localhost:3000, guides still appended after config descriptioncore.integration.tests.jsnow asserts non-emptyinfo.title,info.descriptionandservers[0].urlmodules/homeapp.locals.titlerendering unaffected (unchanged code path)Summary by CodeRabbit
Release Notes
Improvements
Tests