Skip to content

Commit 92241aa

Browse files
docs(tasks): add OpenAPI YAML for tasks module API (#3389)
* docs(tasks): add OpenAPI YAML for tasks module API (#3386) 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. * fix(tasks): address Copilot review — swagger merge, OpenAPI accuracy - 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) * fix(swagger): guard YAML non-objects, seed reduce with {}, preserve error 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 * fix(swagger): address CodeRabbit review — Unauthorized empty body, multi-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 * test(swagger): add branch coverage for empty file list, YAML error, and 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 * test(swagger): fix async import in non-object YAML branch test Use async/await with dynamic import('fs') so the test works correctly in Jest's experimental VM modules ESM mode * fix(swagger): address CodeRabbit pass-2 — empty spec guard, partial update, 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)
1 parent 3e0081b commit 92241aa

6 files changed

Lines changed: 502 additions & 7 deletions

File tree

MIGRATIONS.md

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,23 @@ Breaking changes and upgrade notes for downstream projects.
44

55
---
66

7+
## OpenAPI Module Documentation (2026-04-04)
8+
9+
Modules can now ship their own OpenAPI YAML in `modules/{name}/doc/{name}.yml`. These files are auto-discovered via the `modules/*/doc/*.yml` glob, merged into the base spec from `modules/core/doc/index.yml`, and served at `/api/spec.json` (+ Scalar UI at `/api/docs`).
10+
11+
### What changed
12+
13+
- `modules/core/doc/index.yml` — added shared component schemas (`SuccessResponse`, `ErrorResponse`) and reusable responses (`Unauthorized`, `Forbidden`, `NotFound`, `UnprocessableEntity`)
14+
- `modules/tasks/doc/tasks.yml` — reference OpenAPI doc for the tasks module (all CRUD + stats endpoints)
15+
16+
### Action for downstream
17+
18+
1. Run `/update-stack` to pull the change
19+
2. No breaking change — existing modules without a `doc/` folder are unaffected
20+
3. To document a custom module, create `modules/{name}/doc/{name}.yml` with paths, schemas, and tags
21+
22+
---
23+
724
## Scalar replaces swagger-ui-express (2026-04-04)
825

926
`swagger-ui-express` has been removed. The API documentation UI is now powered by [Scalar](https://scalar.com/) via `@scalar/express-api-reference`.

lib/services/express.js

Lines changed: 45 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,57 @@ import analyticsMiddleware from '../middlewares/analytics.js';
2626

2727
/**
2828
* Initialize API documentation (Scalar UI + JSON spec endpoint)
29+
* @param {object} app - express application instance
30+
* @returns {void}
2931
*/
3032
const initSwagger = (app) => {
3133
if (config.swagger.enable) {
34+
if (!config.files.swagger || config.files.swagger.length === 0) {
35+
logger.warn('[swagger] no swagger files configured — skipping API docs');
36+
return;
37+
}
3238
// Merge all module OpenAPI YAML files into a single spec
33-
const contents = config.files.swagger.map((filePath) => YAML.load(fs.readFileSync(filePath).toString()));
34-
const spec = contents.reduce(_.merge);
39+
// Use mergeWith to concatenate arrays (tags, servers, security) instead of merging by index
40+
const contents = config.files.swagger
41+
.map((filePath) => {
42+
try {
43+
const doc = YAML.load(fs.readFileSync(filePath).toString());
44+
if (!doc || typeof doc !== 'object' || Array.isArray(doc)) {
45+
logger.warn(`[swagger] skipping ${filePath}: YAML did not parse to a plain object`);
46+
return null;
47+
}
48+
return doc;
49+
} catch (err) {
50+
throw new Error(`[swagger] failed to load ${filePath}: ${err.message}`, { cause: err });
51+
}
52+
})
53+
.filter(Boolean);
54+
const spec = contents.reduce(
55+
(acc, doc) =>
56+
_.mergeWith(acc, doc, (objValue, srcValue) => {
57+
if (Array.isArray(objValue)) return objValue.concat(srcValue);
58+
return undefined;
59+
}),
60+
{},
61+
);
3562

36-
// Serve the merged spec as JSON
37-
app.get('/api/spec.json', (req, res) => {
63+
if (Object.keys(spec).length === 0) {
64+
logger.warn('[swagger] all YAML files were skipped — no valid spec to serve');
65+
return;
66+
}
67+
68+
/**
69+
* Serve the merged OpenAPI spec as JSON.
70+
* @param {import('express').Request} _req - Incoming request (unused).
71+
* @param {import('express').Response} res - Express response.
72+
* @returns {void}
73+
*/
74+
const serveSpec = (_req, res) => {
3875
res.json(spec);
39-
});
76+
};
77+
78+
// Serve the merged spec as JSON
79+
app.get('/api/spec.json', serveSpec);
4080

4181
// Mount Scalar API reference UI
4282
app.use(

modules/core/doc/index.yml

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,3 +9,62 @@ components:
99
type: "http"
1010
scheme: "bearer"
1111
bearerFormat: "JWT"
12+
13+
schemas:
14+
SuccessResponse:
15+
type: object
16+
properties:
17+
type:
18+
type: string
19+
enum:
20+
- success
21+
message:
22+
type: string
23+
data:
24+
description: Response payload (type varies by endpoint)
25+
26+
ErrorResponse:
27+
type: object
28+
properties:
29+
type:
30+
type: string
31+
enum:
32+
- error
33+
message:
34+
type: string
35+
code:
36+
type: integer
37+
status:
38+
type: integer
39+
errorCode:
40+
type: string
41+
description:
42+
type: string
43+
error:
44+
type: string
45+
description: JSON-stringified error details included only in non-production environments.
46+
47+
responses:
48+
Unauthorized:
49+
description: Missing or invalid JWT token — Passport JWT middleware returns 401 with an empty body (no JSON envelope)
50+
51+
Forbidden:
52+
description: Insufficient permissions (CASL policy denied)
53+
content:
54+
application/json:
55+
schema:
56+
$ref: '#/components/schemas/ErrorResponse'
57+
58+
NotFound:
59+
description: Resource not found
60+
content:
61+
application/json:
62+
schema:
63+
$ref: '#/components/schemas/ErrorResponse'
64+
65+
UnprocessableEntity:
66+
description: Validation or business logic error
67+
content:
68+
application/json:
69+
schema:
70+
$ref: '#/components/schemas/ErrorResponse'

modules/core/tests/core.unit.tests.js

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,37 @@ describe('Core unit tests:', () => {
507507
);
508508
});
509509

510+
it('should merge multiple module YAML files and include paths from each', () => {
511+
config.swagger = { enable: true };
512+
config.files = {
513+
...config.files,
514+
swagger: [
515+
path.join(process.cwd(), 'modules/core/doc/index.yml'),
516+
path.join(process.cwd(), 'modules/tasks/doc/tasks.yml'),
517+
],
518+
};
519+
const mockGet = jest.fn();
520+
const mockUse = jest.fn();
521+
const mockApp = { get: mockGet, use: mockUse };
522+
expressService.initSwagger(mockApp);
523+
const handler = mockGet.mock.calls.find((c) => c[0] === '/api/spec.json')[1];
524+
const mockRes = { json: jest.fn() };
525+
handler({}, mockRes);
526+
expect(mockRes.json).toHaveBeenCalledWith(
527+
expect.objectContaining({
528+
openapi: '3.0.0',
529+
paths: expect.objectContaining({
530+
'/api/tasks': expect.any(Object),
531+
}),
532+
components: expect.objectContaining({
533+
schemas: expect.objectContaining({
534+
Task: expect.any(Object),
535+
}),
536+
}),
537+
}),
538+
);
539+
});
540+
510541
it('should not register routes when swagger is disabled', () => {
511542
config.swagger = { enable: false };
512543
const mockGet = jest.fn();
@@ -516,6 +547,62 @@ describe('Core unit tests:', () => {
516547
expect(mockGet).not.toHaveBeenCalled();
517548
expect(mockUse).not.toHaveBeenCalled();
518549
});
550+
551+
it('should warn and skip registration when swagger file list is empty', () => {
552+
config.swagger = { enable: true };
553+
config.files = { ...config.files, swagger: [] };
554+
const mockGet = jest.fn();
555+
const mockUse = jest.fn();
556+
const mockApp = { get: mockGet, use: mockUse };
557+
expressService.initSwagger(mockApp);
558+
expect(mockGet).not.toHaveBeenCalled();
559+
expect(mockUse).not.toHaveBeenCalled();
560+
});
561+
562+
it('should throw with file path context when a YAML file fails to load', () => {
563+
config.swagger = { enable: true };
564+
config.files = { ...config.files, swagger: ['/nonexistent/path/bad.yml'] };
565+
const mockApp = { get: jest.fn(), use: jest.fn() };
566+
expect(() => expressService.initSwagger(mockApp)).toThrow('[swagger] failed to load /nonexistent/path/bad.yml');
567+
});
568+
569+
it('should skip YAML files that do not parse to a plain object and still register routes from valid ones', async () => {
570+
// Write a temp YAML file that parses to a scalar string (not an object) — triggers the non-object guard
571+
const { default: fsMod } = await import('fs');
572+
const tmpFile = path.join('/tmp', `test-scalar-${Date.now()}.yml`);
573+
fsMod.writeFileSync(tmpFile, 'just a string value\n');
574+
try {
575+
const validFile = path.join(process.cwd(), 'modules/core/doc/index.yml');
576+
config.swagger = { enable: true };
577+
config.files = { ...config.files, swagger: [tmpFile, validFile] };
578+
const mockGet = jest.fn();
579+
const mockUse = jest.fn();
580+
const mockApp = { get: mockGet, use: mockUse };
581+
expressService.initSwagger(mockApp);
582+
expect(mockGet).toHaveBeenCalledWith('/api/spec.json', expect.any(Function));
583+
} finally {
584+
fsMod.unlinkSync(tmpFile);
585+
}
586+
});
587+
588+
it('should warn and skip registration when all YAML files produce an empty spec', async () => {
589+
// Write a temp YAML file that parses to a scalar — after filter(Boolean), contents is empty → spec is {}
590+
const { default: fsMod } = await import('fs');
591+
const tmpFile = path.join('/tmp', `test-scalar-only-${Date.now()}.yml`);
592+
fsMod.writeFileSync(tmpFile, 'just a string value\n');
593+
try {
594+
config.swagger = { enable: true };
595+
config.files = { ...config.files, swagger: [tmpFile] };
596+
const mockGet = jest.fn();
597+
const mockUse = jest.fn();
598+
const mockApp = { get: mockGet, use: mockUse };
599+
expressService.initSwagger(mockApp);
600+
expect(mockGet).not.toHaveBeenCalled();
601+
expect(mockUse).not.toHaveBeenCalled();
602+
} finally {
603+
fsMod.unlinkSync(tmpFile);
604+
}
605+
});
519606
});
520607

521608
describe('trust proxy', () => {

0 commit comments

Comments
 (0)