feat(public): docs content contract (/api/public/docs) + decommission Redoc#3878
Merged
Merged
Conversation
Add a public, unauthenticated docs module serving:
- GET /api/public/docs -> { categories: [{ id, label, order,
guides: [{ slug, title, persona, order, summary }] }] }
- GET /api/public/docs/:slug.md -> raw text/markdown (unknown slug -> 404)
Guides are discovered from the existing config.files.guides glob
(modules/*/doc/guides/*.md) and grouped via the config.docs.guideSections
primitive (prefix-range sections, optional per-section persona; persona
defaults to ['all']). Guides outside every range fall back to their
capitalised module name so none is ever dropped. Rate-limited (limiters.api),
~5min in-process TTL cache.
Ship two neutral sample guides (00-welcome, 01-quickstart) under
modules/home/doc/guides and a neutral docs.guideSections default so the
endpoint returns a working tree out of the box.
Remove the redoc-express dependency and the /api/docs Redoc UI mount, its theme + custom-CSS blocks, and the config.docs.redocTheme merge. Rename initSwagger -> initApiSpec; the OpenAPI JSON spec endpoint (/api/spec.json) is now the documentation surface and keeps the YAML merge, config-injected info/servers/x-logo, and the markdown guides merge into info.description. The config.swagger.enable + non-dev env gate is unchanged (now gates spec-serving only). Tests: drop the Redoc-UI assertions, strengthen the spec assertions, and add coverage asserting /api/docs is no longer mounted.
|
Warning Review limit reached
More reviews will be available in 37 minutes. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ 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: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (11)
WalkthroughRemoves the ChangesRedoc Decommission →
Public Docs Content Contract
Sequence DiagramsequenceDiagram
participant Client
participant PublicDocsRoute as GET /api/public/docs
participant PublicDocsController
participant PublicDocsService
participant DocsTreeHelper as publicDocs.tree.js
participant Filesystem as doc/guides/*.md
Client->>PublicDocsRoute: GET /api/public/docs (unauthenticated)
PublicDocsRoute->>PublicDocsController: tree(req, res)
PublicDocsController->>PublicDocsService: getTree()
PublicDocsService->>PublicDocsService: check TTL cache
alt cache miss
PublicDocsService->>DocsTreeHelper: loadGuideEntries(guideFiles)
DocsTreeHelper->>Filesystem: readFileSync each .md
Filesystem-->>DocsTreeHelper: markdown content
DocsTreeHelper-->>PublicDocsService: sorted guide entries
PublicDocsService->>DocsTreeHelper: buildDocsTree(entries, guideSections)
DocsTreeHelper-->>PublicDocsService: { categories }
PublicDocsService->>PublicDocsService: stamp expiresAt, build slug index
end
PublicDocsService-->>PublicDocsController: { categories }
PublicDocsController->>Client: 200 + Cache-Control + success envelope
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
🚥 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 |
There was a problem hiding this comment.
Pull request overview
Introduces a new public module that exposes a generic, config-driven docs content contract (tree + raw markdown) for downstream docs UIs, and removes the bundled Redoc UI while preserving the merged OpenAPI spec endpoint.
Changes:
- Added
GET /api/public/docs(structured categories/guides contract) andGET /api/public/docs/:slug.md(raw markdown) with rate limiting + short in-process caching. - Decommissioned Redoc (
/api/docs) and renamed docs init frominitSwagger→initApiSpec, keeping/api/spec.json(still env-gated). - Added 2 sample markdown guides under
modules/home/doc/guides/and updated unit/integration tests accordingly.
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Removes redoc-express dependency. |
| package-lock.json | Lockfile updates reflecting redoc-express removal. |
| lib/services/express.js | Removes /api/docs Redoc wiring; keeps /api/spec.json and renames initializer to initApiSpec. |
| lib/helpers/guides.js | Updates wording from Redoc-specific to generic OpenAPI viewer language. |
| lib/services/tests/express.docs.unit.tests.js | Updates docs tests to validate /api/spec.json behavior and absence of /api/docs. |
| lib/services/tests/express.docsEnvGate.unit.tests.js | Updates env-gate tests for initApiSpec and ensures /api/docs is never mounted. |
| modules/public/helpers/publicDocs.tree.js | Adds markdown parsing + tree builder for the public docs contract. |
| modules/public/services/publicDocs.service.js | Adds TTL cache + guide tree/slug lookup service reading from config.files.guides. |
| modules/public/controllers/publicDocs.controller.js | Adds controllers that wrap the service into the standard response envelope and serve raw markdown. |
| modules/public/routes/publicDocs.route.js | Registers unauthenticated, rate-limited public docs routes. |
| modules/public/tests/publicDocs.tree.unit.tests.js | Unit tests for slug/prefix/module/title/summary parsing and tree assembly. |
| modules/public/tests/publicDocs.service.unit.tests.js | Unit tests for caching + slug lookup with mocked helpers/config. |
| modules/public/tests/publicDocs.service.fallback.unit.tests.js | Unit tests for defensive behavior when config keys are absent. |
| modules/public/tests/publicDocs.controller.unit.tests.js | Unit tests for controller response shaping and error paths. |
| modules/public/tests/publicDocs.integration.tests.js | End-to-end tests verifying the real wire contract and raw markdown behavior. |
| modules/home/doc/guides/00-welcome.md | Adds sample “Welcome” guide. |
| modules/home/doc/guides/01-quickstart.md | Adds sample “Quickstart” guide. |
| modules/core/tests/core.unit.tests.js | Updates core express-service tests for initApiSpec and /api/docs removal. |
| modules/core/tests/core.integration.tests.js | Updates integration coverage to assert /api/spec.json exists and /api/docs is not a Redoc UI. |
| modules/audit/middlewares/audit.middleware.js | Removes /api/docs from audit skip prefixes. |
| modules/audit/tests/audit.middleware.unit.tests.js | Updates audit middleware test for updated skip prefix list. |
| config/defaults/development.config.js | Adds docs.guideSections config used for guide grouping. |
…dpoints M1: replace silent last-win Map construction in publicDocs.service compute() with an explicit loop that logger.warn()s on duplicate slugs. M2: add comment in load() explaining why no inflight guard is needed (compute() is fully synchronous — no stampede risk on the event loop). M3: set Cache-Control: public, max-age=300 on both the tree and raw-markdown 200 responses in publicDocs.controller (matches the 5-min in-process TTL); add controller unit tests asserting the header is present on 200 and absent on 404. L4: document NN-kebab-name.md filename convention in slugFromPath. L5: add comment in express.docs.unit.tests.js second describe block noting that every new test must stub loadGuides inline (no module-level mock). nit: trim docs.guideSections block comment in development.config.js to one line pointing at the helper JSDoc.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3878 +/- ##
==========================================
+ Coverage 92.50% 92.67% +0.17%
==========================================
Files 165 169 +4
Lines 5403 5529 +126
Branches 1740 1775 +35
==========================================
+ Hits 4998 5124 +126
Misses 325 325
Partials 80 80
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report in Codecov by Harness.
🚀 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: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
lib/services/tests/express.docs.unit.tests.js (1)
191-215:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winAdd complete JSDoc tags to the modified helper functions.
buildMockAppandcallSpecRouteare modified helper functions, but their JSDoc blocks don’t include full@param/@returnstags required by the repository rule.As per coding guidelines:
**/*.js: Every new or modified function must have a JSDoc header with@paramfor each argument and@returnsfor non-void returns.🤖 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 `@lib/services/tests/express.docs.unit.tests.js` around lines 191 - 215, Add complete JSDoc tags to both modified helper functions. For buildMockApp, add a `@returns` tag documenting the returned mock app object with the routes property. For callSpecRoute, add a `@param` tag documenting the app parameter and a `@returns` tag documenting the returned spec value. Ensure both functions have their JSDoc blocks updated to include these required tags per the repository coding guidelines for modified functions.Source: Coding guidelines
🤖 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 `@modules/core/tests/core.integration.tests.js`:
- Around line 316-323: The test for the decommissioned Redoc endpoint is
incorrectly asserting a 200 status code when the intended contract for this PR
is that /api/docs should return 404 (Not Found). Change the expect(200)
assertion to expect(404) in the "should NOT serve a dedicated Redoc UI on
/api/docs (decommissioned)" test to properly validate the decommission contract.
Additionally, ensure the application routing is configured to explicitly return
a 404 response for the /api/docs path rather than falling through to a catch-all
handler, so the test assertion aligns with the actual expected behavior.
In `@modules/public/controllers/publicDocs.controller.js`:
- Around line 8-16: The JSDoc comments for async controller handlers incorrectly
document `@returns` as {void} instead of {Promise<void>}. Update the `@returns` type
annotation in the JSDoc comment blocks to accurately reflect that these are
async functions returning a Promise. Specifically, at lines 8-16 in the
publicDocs.controller.js file, change `@returns` {void} to `@returns`
{Promise<void>} for the GET /api/public/docs handler. Apply the same fix at
lines 27-34 for any other async handler in the same file that has the same
incorrect `@returns` documentation.
In `@modules/public/services/publicDocs.service.js`:
- Around line 30-38: The functions guideFiles() and guideSections() currently
return the live config arrays directly, which violates their JSDoc contract and
risks mutations leaking into global config state. Clone both arrays before
returning them to ensure defensive copies are provided to downstream consumers.
Use a shallow clone approach (such as spreading with the spread operator or
calling .slice()) for the returned arrays in both functions.
In `@modules/public/tests/publicDocs.controller.unit.tests.js`:
- Around line 20-27: The named function `mockResponse` is missing a JSDoc
header, which is required by repository guidelines. Add a JSDoc block above the
`mockResponse` function that includes a description of its purpose and a
`@returns` tag documenting that it returns a mock response object with chained
methods (status, json, send, set).
In `@modules/public/tests/publicDocs.tree.unit.tests.js`:
- Line 85: The named standalone helper function `write` is missing JSDoc
documentation. Add JSDoc annotations above the `write` function declaration that
includes `@param` tags for both the `name` and `content` parameters (describing
what each parameter represents and its expected type) and a `@returns` tag
describing what the function returns. This ensures compliance with the
repository JSDoc rule for named helper functions in test files.
---
Outside diff comments:
In `@lib/services/tests/express.docs.unit.tests.js`:
- Around line 191-215: Add complete JSDoc tags to both modified helper
functions. For buildMockApp, add a `@returns` tag documenting the returned mock
app object with the routes property. For callSpecRoute, add a `@param` tag
documenting the app parameter and a `@returns` tag documenting the returned spec
value. Ensure both functions have their JSDoc blocks updated to include these
required tags per the repository coding guidelines for modified functions.
🪄 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: 8bd1c260-2f8a-425c-b106-c4a67f3c39c2
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (21)
config/defaults/development.config.jslib/helpers/guides.jslib/services/express.jslib/services/tests/express.docs.unit.tests.jslib/services/tests/express.docsEnvGate.unit.tests.jsmodules/audit/middlewares/audit.middleware.jsmodules/audit/tests/audit.middleware.unit.tests.jsmodules/core/tests/core.integration.tests.jsmodules/core/tests/core.unit.tests.jsmodules/home/doc/guides/00-welcome.mdmodules/home/doc/guides/01-quickstart.mdmodules/public/controllers/publicDocs.controller.jsmodules/public/helpers/publicDocs.tree.jsmodules/public/routes/publicDocs.route.jsmodules/public/services/publicDocs.service.jsmodules/public/tests/publicDocs.controller.unit.tests.jsmodules/public/tests/publicDocs.integration.tests.jsmodules/public/tests/publicDocs.service.fallback.unit.tests.jsmodules/public/tests/publicDocs.service.unit.tests.jsmodules/public/tests/publicDocs.tree.unit.tests.jspackage.json
💤 Files with no reviewable changes (1)
- package.json
…copies + deterministic ordering
F1: git mv all publicDocs.* files to public.docs.{type}.js (dot-separated, module-prefixed,
routes plural); update every import and reference including test mocks and config comment.
F2/F3: guideFiles() returns [...config.files.guides], guideSections() returns .map(s=>({...s}))
so callers cannot mutate config state.
F4: [...filePaths].sort() before .map() so fallback index for unprefixed guides is
platform-stable (glob order is not guaranteed).
F5: sort tiebreaker a.slug.localeCompare(b.slug) for equal numeric prefixes.
F6: remove try/catch in beforeAll — let bootstrap() throw with its real stack trace.
F7: update stale Redoc/reference-sidebar comments in public.docs.tree.js and
development.config.js (guideSections groups public docs contract only).
Contributor
Author
|
Addressed review findings (Copilot + CodeRabbit):
|
Contributor
Author
|
@coderabbitai review — all prior findings addressed (naming convention, defensive copies, deterministic ordering, Cache-Control, the /api/docs decom assertion, JSDoc). Please re-review. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Listing modules/public structure ==="
find modules/public -type f | sort
echo ""
echo "=== modules/public/controllers ==="
find modules/public/controllers -type f | sort
echo ""
echo "=== modules/public/services ==="
find modules/public/services -type f | sort
echo ""
echo "=== modules/public/helpers ==="
find modules/public/helpers -type f | sort
echo ""
echo "=== modules/public/routes ==="
find modules/public/routes -type f | sort
echo ""
echo "=== modules/public/tests ==="
find modules/public/tests -type f | sortLength of output: 1755 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Controller file ==="
cat modules/public/controllers/public.docs.controller.js 2>/dev/null || find modules/public/controllers -name "*.js" -exec echo "File: {}" \; -exec cat {} \;
echo ""
echo "=== Service file ==="
find modules/public/services -name "*.js" -exec echo "File: {}" \; -exec cat {} \;
echo ""
echo "=== Helper/tree file ==="
find modules/public/helpers -name "*.js" -exec echo "File: {}" \; -exec cat {} \;
echo ""
echo "=== Routes file ==="
find modules/public/routes -name "*.js" -exec echo "File: {}" \; -exec cat {} \;Length of output: 19459 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== Test files ==="
find modules/public/tests -name "*.js" -exec echo "File: {}" \; -exec cat {} \;Length of output: 23369 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== lib/services/express.js ==="
cat lib/services/express.js
echo ""
echo "=== lib/helpers/guides.js ==="
cat lib/helpers/guides.js
echo ""
echo "=== config/defaults/development.config.js ==="
cat config/defaults/development.config.jsLength of output: 25683 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== modules/core/tests/core.integration.tests.js (relevant section) ==="
grep -n "api/docs\|spec.json\|Redoc\|OpenAPI\|decommission\|catch-all\|landing" modules/core/tests/core.integration.tests.js -A 5 -B 2 || true
echo ""
echo "=== modules/audit/middlewares/audit.middleware.js ==="
cat modules/audit/middlewares/audit.middleware.js
echo ""
echo "=== modules/home/doc/guides/ ==="
ls modules/home/doc/guides/ 2>/dev/null || echo "not found"
find . -path "*/home/doc/guides/*" -type f 2>/dev/null | sort
echo ""
echo "=== package.json (dependencies section) ==="
python3 -c "
import json
with open('package.json') as f:
pkg = json.load(f)
deps = pkg.get('dependencies', {})
for k in sorted(deps.keys()):
print(f'{k}: {deps[k]}')
" 2>/dev/null || grep -A 3 '"redoc\|posthog\|resend' package.json || trueLength of output: 8716
Review SummaryNaming convention ✅ Defensive copies ✅ Deterministic ordering ✅
Cache-Control ✅ JSDoc ✅ Additional checks that passed:
One minor note (no action required): The implementation is clean, well-structured, and the contract is unambiguous. Happy to approve. [approve] |
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.
What
modules/public:GET /api/public/docs→{ categories:[{ id, label, order, guides:[{ slug, title, persona, order, summary }] }] }, grouped viaconfig.docs.guideSections(neutral defaults; persona defaults to['all']).GET /api/public/docs/:slug.md→ rawtext/markdown; unknown slug → 404. Unauthenticated, rate-limited (limiters.api), ~5min in-process cache.doc/guides/*.mdvialib/helpers/guides.js; ships 2 generic sample guides (modules/home/doc/guides/) so it works out of the box.lib/services/express.js): removedredoc-express+ the/api/docsUI route + theme/customCss;initSwagger→initApiSpec. Kept the merged OpenAPI spec at/api/spec.json+ the guides-into-spec merge + theswagger.enable/env gate. Droppedredoc-expressfrom deps./api/docsaudit skip-prefix.Why
A downstream consumer can now build its own docs UI from the contract + render
/api/spec.jsonitself (in-theme), instead of the bundled Redoc page. Spec-first.Tests
New
modules/publicunit + integration (the integration test asserts the real wire shape against the real sample guides, not a mock). Updatedexpress.docs/env-gate/core tests (/api/docs404,/api/spec.jsonpreserved). Full unit + integration green locally; coverage on the new module 100%/98.9%.Closes #3877
Summary by CodeRabbit
New Features
Chores
redoc-expressdependency.