docs(tasks): add OpenAPI YAML for tasks module API#3389
Merged
PierreBrisorgueil merged 7 commits intomasterfrom Apr 5, 2026
Merged
docs(tasks): add OpenAPI YAML for tasks module API#3389PierreBrisorgueil merged 7 commits intomasterfrom
PierreBrisorgueil merged 7 commits intomasterfrom
Conversation
|
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 15 minutes and 4 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 (4)
WalkthroughThis PR implements a module-based OpenAPI documentation system with auto-discovery and aggregation. It adds a Changes
Sequence DiagramsequenceDiagram
participant InitSwagger as initSwagger()
participant ModuleLoader as Module YAML Loader
participant Validator as Spec Validator
participant Merger as Spec Merger
participant Server as Express Server
participant Client as API Client
InitSwagger->>ModuleLoader: Load modules/{name}/doc/{name}.yml<br/>+ modules/core/doc/index.yml
ModuleLoader->>Validator: Parse YAML for each module
Validator->>Validator: Verify is plain object
Note over Validator: Skip invalid,<br/>throw on read failure
Validator->>Merger: Pass validated specs
Merger->>Merger: mergeWith customizer:<br/>concatenate arrays
Merger->>Server: Register /api/spec.json route<br/>with merged spec
Server->>Client: GET /api/spec.json
Client->>Client: Receive merged OpenAPI JSON<br/>(paths, components, tags, etc.)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
🚥 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 |
Contributor
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Pull request overview
This PR introduces first-class OpenAPI documentation for the tasks module and switches the API docs UI from Swagger UI to Scalar, with a merged OpenAPI spec served from the backend.
Changes:
- Add
modules/tasks/doc/tasks.ymldocumenting tasks CRUD + stats endpoints. - Extend
modules/core/doc/index.ymlwith sharedSuccessResponse/ErrorResponseschemas and common error responses. - Update Express Swagger initialization to serve
/api/spec.jsonand mount Scalar at/api/docs(and update related tests/config/migrations).
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds Scalar Express API reference dependency; removes swagger-ui-express. |
| package-lock.json | Lockfile updates for Scalar dependency swap. |
| lib/services/express.js | Replaces Swagger UI wiring with merged spec JSON + Scalar UI mounting. |
| config/defaults/development.config.js | Removes obsolete Swagger UI options from defaults. |
| modules/core/doc/index.yml | Adds shared schemas/responses for OpenAPI reuse. |
| modules/tasks/doc/tasks.yml | New OpenAPI YAML for tasks endpoints and schemas. |
| modules/core/tests/core.unit.tests.js | Adds unit coverage for initSwagger route registration/spec handler. |
| modules/audit/tests/audit.middleware.unit.tests.js | Updates audit ignore prefix to match new /api/docs route. |
| MIGRATIONS.md | Documents new OpenAPI module doc pattern and Scalar migration notes. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3389 +/- ##
==========================================
+ Coverage 85.06% 85.16% +0.09%
==========================================
Files 112 112
Lines 2860 2878 +18
Branches 781 787 +6
==========================================
+ Hits 2433 2451 +18
Misses 337 337
Partials 90 90 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/services/express.js`:
- Around line 27-39: The initSwagger function needs a JSDoc header and the
inline route handler should be extracted to a named function to satisfy repo
rules; add a JSDoc block above initSwagger with a one-line description, an
`@param` {Express.Application} app describing the Express app argument, and an
`@returns` {void} (or omit if void but include `@returns` per guideline), then move
the anonymous handler into a named function (e.g., serveSpec or getApiSpec) and
use that identifier in app.get('/api/spec.json', serveSpec) so both initSwagger
and the route handler are documented and readable; reference initSwagger and the
new serveSpec/getApiSpec function in your changes.
In `@modules/core/doc/index.yml`:
- Around line 45-50: The shared OpenAPI response "Unauthorized" currently
references the ErrorResponse schema but Passport-protected JWT endpoints return
401 with an empty body; update the docs by adding a new response definition
(e.g., "PassportUnauthorized") that omits the content/schema entirely, and
replace references to "Unauthorized" with "PassportUnauthorized" on
Passport-protected routes (or document those endpoints to use the new response)
so the spec accurately reflects the empty 401 responses returned by Passport.js
JWT routes.
In `@modules/core/tests/core.unit.tests.js`:
- Around line 483-507: The unit tests in core.unit.tests.js only load
modules/core/doc/index.yml so they don't verify that initSwagger's
merge/discovery pulls in other module specs (e.g., modules/tasks/doc/tasks.yml);
update the unit test that registers /api/spec.json to include at least one
additional module YAML (add modules/tasks/doc/tasks.yml to config.files.swagger)
and assert that the merged spec contains an expected path or operation from that
tasks.yml, and add an integration test that boots the real express app, issues
an HTTP GET to /api/spec.json and asserts the merged spec includes both openapi:
'3.0.0' and the task-specific path; refer to expressService.initSwagger and the
registered /api/spec.json handler when locating code to modify.
🪄 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: e5c5ed9d-0214-412d-95d2-42ff6d5cab70
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (8)
MIGRATIONS.mdconfig/defaults/development.config.jslib/services/express.jsmodules/audit/tests/audit.middleware.unit.tests.jsmodules/core/doc/index.ymlmodules/core/tests/core.unit.tests.jsmodules/tasks/doc/tasks.ymlpackage.json
💤 Files with no reviewable changes (1)
- config/defaults/development.config.js
Document all task endpoints (CRUD + stats) with request/response schemas, auth requirements, and path parameters. Add shared SuccessResponse and ErrorResponse schemas to core index for reuse across modules.
- initSwagger: use _.mergeWith to concatenate arrays (not index-merge) - initSwagger: guard empty file list, add filePath context on YAML errors - initSwagger: add JSDoc @param/@returns - tasks.yml: mark OrganizationId header deprecated with correct description - tasks.yml: add 422 response to GET /api/tasks - tasks.yml: update PUT description — both fields required, not partial - tasks.yml: document full DELETE response (id + acknowledged + deletedCount) - index.yml: add optional error field to ErrorResponse (non-prod only)
…rror cause
- Filter YAML.load results to plain objects (warn + skip null/array/primitive)
- Pass initial {} to reduce so single-file configs merge correctly
- Preserve original parse error as Error cause for better stack traces
…lti-module merge test - index.yml: Unauthorized response updated to reflect Passport JWT returns empty body (no JSON envelope) on 401 — matches actual runtime behavior - core.unit.tests.js: add test for multi-module YAML merge, asserting merged spec contains tasks paths and Task schema from modules/tasks/doc/tasks.yml
PierreBrisorgueil
force-pushed
the
feat/tasks-openapi-docs
branch
from
317ed60 to
252e913
Compare
…nd non-object YAML guard - Empty file list guard: verify initSwagger warns and skips route registration - YAML parse error: verify throw includes failing file path in message - Non-object YAML: verify scalar YAML files are skipped (filter(Boolean)) while valid files still produce a merged spec
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@lib/services/express.js`:
- Around line 54-61: After merging YAML docs into spec with contents.reduce, add
a guard that detects an empty/invalid merged spec (e.g., _.isEmpty(spec) or
Object.keys(spec).length === 0) and short-circuits the route registration path:
log or throw a descriptive error and return early (similar to the existing empty
file list check) so routes are not registered against an empty {}; update the
code around the spec variable computed by the reduce to perform this check
before any further processing or registration.
In `@modules/core/tests/core.unit.tests.js`:
- Around line 569-584: The test currently performs an asynchronous dynamic
import of fs inside a synchronous test callback so assertions run after test
completion; make the test function async and await the import('fs') promise
before writing the temp file and calling expressService.initSwagger, then run
the assertions and cleanup (unlinkSync) synchronously after awaiting; target the
mocha/jest test named "should skip YAML files..." and update its callback to
async and add await before import('fs') and any other async ops around tmpFile,
expressService.initSwagger, mockGet and mockUse usage.
In `@modules/tasks/doc/tasks.yml`:
- Around line 123-138: Update the PUT operation description to reflect that
TaskUpdate is a partial/optional schema (TaskUpdate and Zod's Task.partial()),
not requiring both title and description; change the sentence that claims "Both
`title` and `description` are required — omitting a field will overwrite it with
`undefined`" to state that fields are optional and omitted fields are left
unchanged (partial update), and ensure the wording mentions the CASL ownership
enforcement still applies (refer to the put operation and the TaskUpdate
schema).
- Around line 74-76: The POST create-task response is documented as '200' but
should be '201 Created' per REST conventions; update the tasks POST endpoint to
return 201 by changing the OpenAPI response entry from '200' to '201' and modify
the controller call that uses responses.success() to either call a new/existing
responses.created() helper or pass an explicit 201 status to
responses.success(); if you add a new helper, implement it in
lib/helpers/responses.js (alongside responses.success) to set status 201 and
return the same payload shape so the spec and implementation match.
🪄 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: 9cbf1bdc-b7e6-4489-90ce-21bb445d0c43
📒 Files selected for processing (5)
MIGRATIONS.mdlib/services/express.jsmodules/core/doc/index.ymlmodules/core/tests/core.unit.tests.jsmodules/tasks/doc/tasks.yml
Use async/await with dynamic import('fs') so the test works correctly
in Jest's experimental VM modules ESM mode
…pdate, PUT description
- express.js: add guard after reduce — warn and skip route registration if all
YAML files were filtered and spec is empty ({})
- tasks.service.js: fix update to only assign defined fields (partial update),
aligning with Zod TaskUpdate = Task.partial()
- tasks.yml: update PUT description to reflect actual partial-update behavior
- core.unit.tests.js: add test for empty-spec guard (all YAMLs skipped → no routes)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
modules/tasks/doc/tasks.ymldocumenting all 6 task endpoints (list, create, get, update, delete, stats) with full request/response schemas, auth requirements, and parametersSuccessResponse,ErrorResponseschemas and reusable error responses (Unauthorized,Forbidden,NotFound,UnprocessableEntity) tomodules/core/doc/index.ymlCloses #3386
Test plan
/api/docs— verify tasks endpoints render in Scalar UI/api/spec.json— verify tasks paths and schemas are present in merged specSummary by CodeRabbit
Release Notes
New Features
/api/spec.jsonendpoint and interactive documentation UI at/api/docsDocumentation