feat(api-contract): adopt checked-in OpenAPI contract as source of truth - #21
Conversation
Document the contract-first direction for the current web/api read seam, including scope, non-goals, success criteria, and follow-up planning questions in both language trees.
Add a shared OpenAPI package, expose refreshed API documents from the API, replace the web client seam with generated contract types, and make hook-time routing skip generated contract output instead of formatting or linting it.
Route generated API contract files away from formatting hooks so lint-staged can skip them cleanly. Refresh the generated client to match the new routing behavior.
Use the current `summaryError` field name in both app READMEs so the published contract matches the OpenAPI source of truth.
📝 WalkthroughSummary by CodeRabbit
WalkthroughImplements a contract-first OpenAPI workflow: adds Changes
Sequence DiagramsequenceDiagram
participant Dev as Developer
participant API as apps/api (Nest app)
participant Script as openapi-refresh.ts
participant Contract as packages/api-contract
participant Orval as Orval
participant Web as apps/web
Dev->>API: Change controller/DTOs
Dev->>Script: Run pnpm contract:refresh
Script->>API: Boot test Nest app (override bootstraps)
Script->>Script: createOpenApiDocument(app)
Script->>Contract: Write openapi/openapi.yaml
Contract->>Orval: Run orval -> read openapi.yaml
Orval->>Contract: Emit generated client/types
Web->>Contract: Import generated client/types
Web->>Web: Replace fetch with getArticles/getArticleById
Dev->>Dev: Open PR with openapi YAML + generated artifacts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/en/brainstorms/2026-04-24-web-api-openapi-contract-first-requirements.md (1)
1-144:⚠️ Potential issue | 🟠 MajorAdd the Simplified Chinese counterpart for this brainstorm document.
This new requirements doc is added only under
docs/en/...; please add the matchingdocs/zh-Hans/brainstorms/...version in the same PR to keep documentation synchronized.As per coding guidelines: "All durable documentation must be kept in sync in Chinese (Simplified) and English."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/en/brainstorms/2026-04-24-web-api-openapi-contract-first-requirements.md` around lines 1 - 144, This PR adds an English brainstorm doc but omits the required Simplified Chinese counterpart; add a translated Simplified Chinese version of the same brainstorm (same date 2026-04-24 and the same slug/topic web-api-openapi-contract-first) into the repo's zh-Hans documentation tree, preserving the YAML frontmatter fields (date/topic) and all sections/structure (Requirements, Success Criteria, Scope, Key Decisions, References, Next Steps) so the two docs stay synchronized in the same PR; ensure the translated file uses the same filename/slug and mirrors headings and bullet content so reviewers can compare diffs easily.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/api/e2e/openapi-contract.e2e-spec.ts`:
- Around line 51-54: The test currently builds the contract path using
process.cwd(), which makes lookup brittle; change the path resolution in
apps/api/e2e/openapi-contract.e2e-spec.ts to anchor to the test file location
(e.g., use __dirname or equivalent) when calling resolve(...) so the path to
"../../packages/api-contract/openapi/openapi.yaml" is computed relative to the
test file instead of the current working directory.
- Around line 18-35: The parity test currently creates a partial
Test.createTestingModule with only ArticlesController/HealthController and
mocked ArticlesService/PrismaService, which can miss endpoints added to the real
app graph; change the setup to bootstrap the real AppModule (import AppModule
into Test.createTestingModule or use Test.createTestingModule({ imports:
[AppModule] })) and then override providers you need (use
moduleRef.overrideProvider(PrismaService).useValue(...) and
moduleRef.overrideProvider(ArticlesService).useValue(...)) so the generated
OpenAPI comes from the same module graph as the refresh flow while still
allowing mocks for PrismaService/ArticlesService.
In `@apps/api/src/openapi/openapi-document.ts`:
- Around line 1-36: The createOpenApiDocument function and its OpenAPI
composition are placed in a repo-wide tech-layer folder; move this logic into a
feature-scoped module (e.g., an api/infrastructure or docs feature folder) to
respect NestJS module boundaries: relocate the createOpenApiDocument function
and related DTO imports into the chosen feature folder, update any
imports/usages (e.g., where createOpenApiDocument is invoked during app
bootstrap) to the new path, and register/export it from the feature module so
consumers continue to call createOpenApiDocument (keep the function name and
SwaggerModule usage unchanged).
In `@apps/api/src/openapi/openapi-refresh.ts`:
- Around line 16-23: Don't bootstrap the full AppModule for OpenAPI refresh;
instead create and use a dedicated module (e.g., OpenApiRefreshModule) that only
imports the controllers/feature modules required for Swagger generation and use
that in Test.createTestingModule(...) in place of AppModule. Ensure
FeedBootstrapService is still overridden
(overrideProvider(FeedBootstrapService).useValue(...)) and do not call any
application init paths that trigger unrelated module-level side effects; wire
only the controllers/providers needed for document creation so contract refresh
is isolated from runtime initialization.
- Around line 42-43: The script's CLI entry (the require.main === module block
invoking refreshOpenApiContract) leaves promise rejections unhandled; update
that block to explicitly handle errors by awaiting or chaining
refreshOpenApiContract().catch(err => { processLogger/error output (or
console.error) with the error details; process.exit(1); }) so any failure logs
the error and exits with a non‑zero code instead of relying on global
unhandled-rejection behavior.
In
`@docs/en/brainstorms/2026-04-24-web-api-openapi-contract-first-requirements.md`:
- Around line 133-139: The bullet list items containing reference-style bracket
tokens like "[Affects R1][Technical]", "[Affects R10][Technical]", "[Affects
R10][Needs research]", and "[Affects R4][Technical]" are being parsed as
markdown reference links and causing MD052; fix by replacing or escaping those
bracketed labels (e.g., change to "Affects R1 — Technical" or escape the
brackets "\[Technical\]") so they render as plain text in the Outstanding
Questions list, updating the four affected list items accordingly.
In `@docs/en/plans/2026-04-24-001-feat-web-api-openapi-contract-first-plan.md`:
- Line 4: The frontmatter key "status: completed" conflicts with remaining
unchecked task items (plain "[ ]" checkboxes) in the plan body; update either
the status key or the checkbox states so they match: either change "status:
completed" to "in-progress" or "needs-review", or mark the remaining unchecked
task lines as checked ("[x]"); ensure consistency by making "status" reflect the
actual checkbox state and update any headings that reference completion
accordingly.
In
`@docs/zh-Hans/plans/2026-04-24-001-feat-web-api-openapi-contract-first-plan.md`:
- Around line 150-156: The zh-Hans plan introduces a "延后到实现" deferred section
that is not present in the English plan, causing bilingual documentation drift;
either add the same deferred bullet list (including the `packages/api-contract`
generated filenames/exports item, the API document helper reuse question, and
the web thin-adapter shape / `API_BASE_URL` and 404 handling note) to
docs/en/plans/2026-04-24-001-feat-web-api-openapi-contract-first-plan.md, or
remove the deferred section from this zh-Hans file so both durable plan docs
remain synchronized.
In `@lint-staged.config.mjs`:
- Around line 39-40: The ignore-prefix matching fails on Windows due to
backslashes from path.relative; update isIgnoredByLintRouting and the places
that call path.relative (where you compare against rootScopedIgnoredLintPrefixes
like "packages/api-contract/src/generated/") to normalize path separators before
matching: convert the computed relative path to a consistent form (e.g., replace
backslashes with forward slashes) and then use startsWith against
rootScopedIgnoredLintPrefixes so generated files are correctly ignored on all
platforms.
In `@packages/api-contract/src/index.ts`:
- Around line 41-46: fetchJson and getArticleById perform outbound fetches
without timeouts; add an explicit timeout by creating a dedicated
AbortController per request (e.g., create DEFAULT_FETCH_TIMEOUT_MS constant),
start a timer that calls controller.abort() after the timeout, and if the caller
passed init?.signal or getArticleById receives a signal, forward that signal to
abort the same controller (attach an abort listener that calls
controller.abort()); then pass controller.signal into fetch and ensure you clear
the timer and remove listeners after fetch completes to avoid leaks; apply this
same pattern in both fetchJson and getArticleById to merge caller signals with
the timeout.
- Around line 69-76: The function getArticleById currently requires a
pre-encoded encodedArticleId and constructs the path with a hard-coded string
via new URL(`/articles/${encodedArticleId}`, apiBaseUrl).toString(), which is
error-prone; change the parameter to articleId (raw), perform encoding inside
the function (e.g., encodeURIComponent(articleId)), and replace the hard-coded
path construction with the project's generated route helper for article detail
URLs (i.e., use the generated URL helper instead of new URL(...)). Ensure the
exported signature remains async getArticleById(apiBaseUrl: string, articleId:
string, init?: RequestInit) and that the fetch target uses the helper plus the
internally encoded ID.
---
Outside diff comments:
In
`@docs/en/brainstorms/2026-04-24-web-api-openapi-contract-first-requirements.md`:
- Around line 1-144: This PR adds an English brainstorm doc but omits the
required Simplified Chinese counterpart; add a translated Simplified Chinese
version of the same brainstorm (same date 2026-04-24 and the same slug/topic
web-api-openapi-contract-first) into the repo's zh-Hans documentation tree,
preserving the YAML frontmatter fields (date/topic) and all sections/structure
(Requirements, Success Criteria, Scope, Key Decisions, References, Next Steps)
so the two docs stay synchronized in the same PR; ensure the translated file
uses the same filename/slug and mirrors headings and bullet content so reviewers
can compare diffs easily.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1d3134d7-96d2-4884-bcc5-cd39c82cf7a5
⛔ Files ignored due to path filters (2)
packages/api-contract/src/generated/api-client.tsis excluded by!**/generated/**pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (32)
.prettierignoreAGENTS.mdapps/api/README.mdapps/api/e2e/openapi-contract.e2e-spec.tsapps/api/package.jsonapps/api/src/articles/articles.controller.tsapps/api/src/articles/dto/article-detail-item.dto.tsapps/api/src/articles/dto/article-list-item.dto.tsapps/api/src/health/dto/health-response.dto.tsapps/api/src/health/health.controller.tsapps/api/src/openapi/openapi-document.tsapps/api/src/openapi/openapi-refresh.tsapps/web/README.mdapps/web/app/next-config.spec.tsapps/web/jest.config.tsapps/web/next.config.tsapps/web/package.jsonapps/web/src/widgets/article-reader/api/articles-api.tsdocs/en/brainstorms/2026-04-24-web-api-openapi-contract-first-requirements.mddocs/en/plans/2026-04-24-001-feat-web-api-openapi-contract-first-plan.mddocs/zh-Hans/brainstorms/2026-04-24-web-api-openapi-contract-first-requirements.mddocs/zh-Hans/plans/2026-04-24-001-feat-web-api-openapi-contract-first-plan.mdlint-staged.config.mjspackage.jsonpackages/api-contract/openapi/openapi.yamlpackages/api-contract/orval.config.tspackages/api-contract/package.jsonpackages/api-contract/src/index.tspackages/api-contract/tsconfig.build.jsonpackages/api-contract/tsconfig.jsonpackages/eslint-config/base.jsturbo.json
| import { type INestApplication } from "@nestjs/common"; | ||
| import { DocumentBuilder, SwaggerModule } from "@nestjs/swagger"; | ||
|
|
||
| import { | ||
| ArticleDetailItemDto, | ||
| ArticleSummaryErrorDto, | ||
| } from "../articles/dto/article-detail-item.dto"; | ||
| import { ArticleListItemDto } from "../articles/dto/article-list-item.dto"; | ||
| import { | ||
| HealthLiveChecksDto, | ||
| HealthLiveResponseDto, | ||
| HealthReadyChecksDto, | ||
| HealthReadyResponseDto, | ||
| } from "../health/dto/health-response.dto"; | ||
|
|
||
| export function createOpenApiDocument(app: INestApplication) { | ||
| const config = new DocumentBuilder() | ||
| .setTitle("RSSift API") | ||
| .setDescription("OpenAPI contract for the RSSift API surface") | ||
| .setVersion("0.1.0") | ||
| .build(); | ||
|
|
||
| return SwaggerModule.createDocument(app, config, { | ||
| deepScanRoutes: true, | ||
| extraModels: [ | ||
| ArticleDetailItemDto, | ||
| ArticleListItemDto, | ||
| ArticleSummaryErrorDto, | ||
| HealthLiveChecksDto, | ||
| HealthLiveResponseDto, | ||
| HealthReadyChecksDto, | ||
| HealthReadyResponseDto, | ||
| ], | ||
| operationIdFactory: (_controllerKey, methodKey) => methodKey, | ||
| }); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Move this OpenAPI composition out of a repo-wide tech-layer folder.
apps/api/src/openapi/* centralizes cross-feature concerns in a global technical folder. This should live in a feature-scoped module/folder boundary to stay aligned with backend module architecture rules.
As per coding guidelines: "In apps/api NestJS, avoid repo-wide tech-layer folders unless deeper scoped rule overrides; prefer self-contained feature folders."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/api/src/openapi/openapi-document.ts` around lines 1 - 36, The
createOpenApiDocument function and its OpenAPI composition are placed in a
repo-wide tech-layer folder; move this logic into a feature-scoped module (e.g.,
an api/infrastructure or docs feature folder) to respect NestJS module
boundaries: relocate the createOpenApiDocument function and related DTO imports
into the chosen feature folder, update any imports/usages (e.g., where
createOpenApiDocument is invoked during app bootstrap) to the new path, and
register/export it from the feature module so consumers continue to call
createOpenApiDocument (keep the function name and SwaggerModule usage
unchanged).
Generated client artifacts should stay local and be ignored by git. This keeps the repo clean while preserving the source generator workflow.
Refresh the checked-in OpenAPI artifact from a real app module, keep the web adapter and generated client aligned on article IDs, and add fetch cleanup plus a default timeout for contract helpers. Normalize staged paths for lint-staged so the contract refresh flow stays reliable on Windows too.
Narrow the generated contract lint scope, keep the OpenAPI refresh\nmodule clean, and align repo guidance with the current workflow.\n\nAlso relax linting for the generated API client that is intentionally\nchecked in under packages/api-contract.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@AGENTS.md`:
- Around line 6-7: The terse policy fragments like "No break workspace layout",
"No add…", and "No make…" in AGENTS.md are ambiguous; replace them with clear,
consistent policy sentences (e.g., use "Do not break workspace layout", "Do not
add X that..." or "Avoid making changes that...") across the referenced lines
(including the instances flagged at lines ~6-7, 13, 44-46, 57-58) so
contributors and automation can interpret them unambiguously; update each clause
to specify the prohibited action and its scope (what, where, and why) and ensure
uniform phrasing and tone throughout the document.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 97ef4170-a59a-4077-90da-cff1c3f983fe
📒 Files selected for processing (3)
AGENTS.mdapps/api/src/openapi/openapi-refresh.module.tspackages/eslint-config/base.js
| Keep monorepo shape. | ||
| No break workspace layout, package boundaries, shared config, task graph, repo conventions. |
There was a problem hiding this comment.
Clarify policy sentences to avoid ambiguous enforcement language.
Several updated lines use terse grammar (“No break…”, “No add…”, “No make…”), which weakens interpretation consistency for contributors and automation-facing guidance.
✍️ Proposed wording cleanup
-Keep monorepo shape.
-No break workspace layout, package boundaries, shared config, task graph, repo conventions.
+Keep the monorepo shape intact.
+Do not break workspace layout, package boundaries, shared config, task graph, or repo conventions.
-No move package responsibility to repo root.
+Do not move package responsibility to the repo root.
-If none fits perfect, use closest existing category.
-No add new category unless both language trees expand on purpose.
+If none fits perfectly, use the closest existing category.
+Do not add a new category unless both language trees are intentionally expanded.
- - In `apps/web`, prefer `src/app`, `src/widgets`, `src/features`, `src/entities`, `src/shared` for business structure. If page-scoped business slice needed, keep under `src/app` or `src/widgets` with route-aligned name. No add `src/pages`.
- - Keep root `app/` thin: route entrypoints, top-level providers, metadata wiring, route handlers, minimal bridge code only. No make root `app/` main home for reusable business slices.
+ - In `apps/web`, prefer `src/app`, `src/widgets`, `src/features`, `src/entities`, and `src/shared` for business structure. If a page-scoped business slice is needed, keep it under `src/app` or `src/widgets` with a route-aligned name. Do not add `src/pages`.
+ - Keep root `app/` thin: route entrypoints, top-level providers, metadata wiring, route handlers, and minimal bridge code only. Do not make root `app/` the main home for reusable business slices.Also applies to: 13-13, 44-46, 57-58
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@AGENTS.md` around lines 6 - 7, The terse policy fragments like "No break
workspace layout", "No add…", and "No make…" in AGENTS.md are ambiguous; replace
them with clear, consistent policy sentences (e.g., use "Do not break workspace
layout", "Do not add X that..." or "Avoid making changes that...") across the
referenced lines (including the instances flagged at lines ~6-7, 13, 44-46,
57-58) so contributors and automation can interpret them unambiguously; update
each clause to specify the prohibited action and its scope (what, where, and
why) and ensure uniform phrasing and tone throughout the document.
Adopt a checked-in OpenAPI contract as the single source of truth for the API
surface, then make the web app consume generated contract types instead of
handwritten DTO-like shapes.
Changes
apps/api, plus a refresh task that writes thechecked-in YAML contract.
committed contract file.
apps/webto depend on@repo/api-contractand removed thehandwritten seam shapes in favor of generated contract types.
cached task paths.
summaryErrorfield name.This keeps the API surface explicit, lets web consume the same schema the API
publishes, and narrows the places where contract drift can appear.