feat: CI job to detect breaking api v2 changes#24028
Conversation
WalkthroughMoved Swagger generation into Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 |
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/api/v2/src/swagger/copy-swagger-module.ts (2)
18-25: Fail fast on copy errors and missing source directoryCurrently errors are only logged; the script exits 0 and downstream build/start proceeds without the plugin, causing hard‑to‑debug failures.
- Check that sourceDir exists before copying.
- Rethrow or exit non‑zero on failure.
Apply this diff:
if (!(await fs.pathExists(targetDir))) { try { await fs.ensureDir(nodeModulesNestjs); - await fs.copy(sourceDir, targetDir); + if (!(await fs.pathExists(sourceDir))) { + console.error(`[copy-swagger-module] Source not found: ${sourceDir}`); + throw new Error("Missing @nestjs/swagger in monorepo root node_modules"); + } + await fs.copy(sourceDir, targetDir); } catch (error) { - console.error("Failed to copy @nestjs/swagger:", error); + console.error("Failed to copy @nestjs/swagger:", error); + throw error; } }
28-28: Handle promise rejection so the script fails the pipeline when copy failsThe async call is not awaited, and errors are swallowed. Ensure a non‑zero exit code on failure.
Apply this diff:
-copyNestSwagger(); +copyNestSwagger().catch((error) => { + console.error("Failed to copy @nestjs/swagger:", error); + process.exit(1); +});
🧹 Nitpick comments (7)
.github/workflows/api-v2-breaking-changes.yml (1)
23-26: DB services vs env mismatch — point DATABASE_URLs to local Postgres.You start a Postgres service on 5432 but point all DB URLs to a secret (likely remote). Either remove the service or target it. Using the local service isolates CI and avoids external dependencies.
Apply this diff (align all URLs with the service credentials declared below):
- DATABASE_URL: ${{ secrets.CI_DATABASE_URL }} - DATABASE_READ_URL: ${{ secrets.CI_DATABASE_URL }} - DATABASE_WRITE_URL: ${{ secrets.CI_DATABASE_URL }} - DATABASE_DIRECT_URL: ${{ secrets.CI_DATABASE_URL }} + DATABASE_URL: postgresql://postgres:postgres@localhost:5432/calendso + DATABASE_READ_URL: postgresql://postgres:postgres@localhost:5432/calendso + DATABASE_WRITE_URL: postgresql://postgres:postgres@localhost:5432/calendso + DATABASE_DIRECT_URL: postgresql://postgres:postgres@localhost:5432/calendsoIf you intentionally need the external DB, drop the Postgres service to speed up CI.
.github/workflows/all-checks.yml (1)
89-91: Including api-v2-breaking-changes in required is good; consider path-based gating later.This will enforce the check in the aggregate. If runtime becomes a concern, consider passing an input from a paths-filter job to the reusable workflow to no-op when v2 specs aren’t impacted.
apps/api/v2/src/swagger/copy-swagger-module.ts (1)
11-12: Verify path resolution for both ts-node and compiled (dist) executionThe new upward traversal looks correct for ts-node (src/... context). If this ever runs from dist, five levels up would resolve to the dist root, not repo root. Please confirm this script is only ever executed via ts-node as per package.json.
If you need robustness across both contexts, resolve based on process.cwd() (workspace root) or detect both candidates.
apps/api/v2/src/main.ts (1)
29-34: Propagate startup failuresAfter logging, rethrow to trigger the top-level run().catch and exit(1). Otherwise the process may linger without a listener.
Apply this diff:
} catch (error) { console.error(error); logger.error("Application crashed", { error, }); + throw error; }apps/api/v2/src/swagger/generate-swagger-script.ts (1)
17-23: Initialize the app before generating the documentCalling app.init() ensures the DI container is fully initialized before Swagger scans metadata. This avoids subtle missing‑binding issues.
Apply this diff:
try { - bootstrap(app); - await generateSwaggerForApp(app); + bootstrap(app); + await app.init(); + await generateSwaggerForApp(app);apps/api/v2/src/swagger/generate-swagger.ts (2)
15-15: Include TRACE in HTTP methodsMinor completeness nit.
Apply this diff:
-const HttpMethods: (keyof PathItemObject)[] = ["get", "post", "put", "delete", "patch", "options", "head"]; +const HttpMethods: (keyof PathItemObject)[] = ["get", "post", "put", "delete", "patch", "options", "head", "trace"];
43-72: Sorting drops untagged paths and can misclassify multi-tag paths
- Untagged endpoints are omitted entirely.
- The last seen method decides the group for a path, which is brittle when methods have different first tags.
Apply this diff to preserve all paths deterministically and keep untagged last:
-function groupAndSortPathsByFirstTag(paths: PathsObject): PathsObject { - const groupedPaths: { [key: string]: PathsObject } = {}; - - Object.keys(paths).forEach((pathKey) => { - const pathItem = paths[pathKey]; - - HttpMethods.forEach((method) => { - const operation = pathItem[method]; - - if (isOperationObject(operation) && operation.tags && operation.tags.length > 0) { - const firstTag = operation.tags[0]; - - if (!groupedPaths[firstTag]) { - groupedPaths[firstTag] = {}; - } - - groupedPaths[firstTag][pathKey] = pathItem; - } - }); - }); - - const sortedTags = Object.keys(groupedPaths).sort(customTagSort); - const sortedPaths: PathsObject = {}; - - sortedTags.forEach((tag) => { - Object.assign(sortedPaths, groupedPaths[tag]); - }); - - return sortedPaths; -} +function groupAndSortPathsByFirstTag(paths: PathsObject): PathsObject { + const groupedPaths: { [key: string]: PathsObject } = {}; + const UNTAGGED = "__zzz_untagged__"; + + Object.keys(paths).forEach((pathKey) => { + const pathItem = paths[pathKey]; + const firstTag = + HttpMethods.map((m) => pathItem[m]) + .filter(isOperationObject) + .map((op) => op.tags?.[0]) + .find(Boolean) || UNTAGGED; + + if (!groupedPaths[firstTag]) { + groupedPaths[firstTag] = {}; + } + groupedPaths[firstTag][pathKey] = pathItem; + }); + + const sortedTags = Object.keys(groupedPaths) + .filter((t) => t !== UNTAGGED) + .sort(customTagSort) + .concat([UNTAGGED]); + + const sortedPaths: PathsObject = {}; + sortedTags.forEach((tag) => Object.assign(sortedPaths, groupedPaths[tag])); + return sortedPaths; +}
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (9)
.github/workflows/all-checks.yml(1 hunks).github/workflows/api-v2-breaking-changes.yml(1 hunks).github/workflows/pr.yml(3 hunks).prettierignore(0 hunks)apps/api/v2/package.json(2 hunks)apps/api/v2/src/main.ts(2 hunks)apps/api/v2/src/swagger/copy-swagger-module.ts(1 hunks)apps/api/v2/src/swagger/generate-swagger-script.ts(1 hunks)apps/api/v2/src/swagger/generate-swagger.ts(1 hunks)
💤 Files with no reviewable changes (1)
- .prettierignore
🧰 Additional context used
📓 Path-based instructions (3)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
apps/api/v2/src/swagger/generate-swagger-script.tsapps/api/v2/src/swagger/copy-swagger-module.tsapps/api/v2/src/main.tsapps/api/v2/src/swagger/generate-swagger.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
apps/api/v2/src/swagger/generate-swagger-script.tsapps/api/v2/src/swagger/copy-swagger-module.tsapps/api/v2/src/main.tsapps/api/v2/src/swagger/generate-swagger.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
apps/api/v2/src/swagger/generate-swagger-script.tsapps/api/v2/src/swagger/copy-swagger-module.tsapps/api/v2/src/main.tsapps/api/v2/src/swagger/generate-swagger.ts
🧠 Learnings (1)
📚 Learning: 2025-09-15T12:58:12.826Z
Learnt from: CR
PR: calcom/cal.com#0
File: AGENTS.md:0-0
Timestamp: 2025-09-15T12:58:12.826Z
Learning: Use `yarn build` to build all packages
Applied to files:
apps/api/v2/package.json
🧬 Code graph analysis (3)
apps/api/v2/src/swagger/generate-swagger-script.ts (3)
apps/api/v2/src/main.ts (1)
createNestApp(37-42)apps/api/v2/src/app.ts (1)
bootstrap(27-89)apps/api/v2/src/swagger/generate-swagger.ts (1)
generateSwaggerForApp(17-41)
apps/api/v2/src/main.ts (2)
apps/api/v2/src/app.ts (1)
bootstrap(27-89)apps/api/v2/src/swagger/generate-swagger.ts (1)
generateSwaggerForApp(17-41)
apps/api/v2/src/swagger/generate-swagger.ts (1)
apps/api/v2/src/env.ts (1)
getEnv(39-54)
🪛 actionlint (1.7.7)
.github/workflows/pr.yml
209-209: error while parsing reusable workflow "./.github/workflows/api-v2-breaking-changes.yml": "workflow_call" event trigger is not found in "on:" at line:4, column:3
(expression)
.github/workflows/api-v2-breaking-changes.yml
44-44: label "buildjet-4vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
.github/workflows/all-checks.yml
86-86: error while parsing reusable workflow "./.github/workflows/api-v2-breaking-changes.yml": "workflow_call" event trigger is not found in "on:" at line:4, column:3
(workflow-call)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (9)
.github/workflows/api-v2-breaking-changes.yml (1)
44-44: Runner label verification.
buildjet-4vcpu-ubuntu-2204is unknown to actionlint; confirm it’s a valid self-hosted/partner runner in this org. If not, switch toubuntu-22.04orubuntu-latest-8-cores..github/workflows/pr.yml (2)
221-221: Merge reports dependency updated — OK.Including
api-v2-breaking-changeshere keeps reports consistent with other gating checks.If you want
merge-reportsto run only for e2e flows (as currently gated), this dependency is fine.
247-268: Required aggregate now includes api-v2-breaking-changes — OK.This ensures the PR’s aggregate check fails if the breaking-change job fails. With the earlier fix (removing dependency on
build-api-v2), it should not be skipped.apps/api/v2/package.json (2)
19-19: LGTM: dev:build now builds platform libraries tooThis reduces flakiness from missing types/artifacts in local and CI builds.
33-33: LGTM: updated ts-node path for copy-swagger-moduleThe new source path reflects the file relocation under src/swagger.
apps/api/v2/src/main.ts (2)
14-17: LGTM: top-level error handlerGood explicit exit on fatal bootstrap errors.
37-41: LGTM: createNestApp extractionClean separation; keeps logger wiring in one place.
apps/api/v2/src/swagger/generate-swagger-script.ts (1)
7-15: LGTM: controlled exit codes and cleanupThe flow cleanly exits with 0/1 and closes the app in finally.
apps/api/v2/src/swagger/generate-swagger.ts (1)
34-41: LGTM: conditional Swagger UI mountingGood guard on DOCS_URL for hosted docs scenarios.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
.github/workflows/api-v2-breaking-changes.yml (2)
41-42: Pin Docker images (avoid mutable tags)Use fixed versions (and ideally digests) to ensure reproducible builds; avoid redis:latest.
- image: postgres:13 + image: postgres:13-alpine … - image: redis:latest + image: redis:7-alpineConsider pinning to immutable digests if possible.
Also applies to: 57-58
23-23: Right-size Node heap--max-old-space-size=29000 likely exceeds runner RAM and is unnecessary for Swagger generation; reduce (e.g., 4096) or omit.
- NODE_OPTIONS: --max-old-space-size=29000 + NODE_OPTIONS: --max-old-space-size=4096
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/api-v2-breaking-changes.yml(1 hunks)
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/api-v2-breaking-changes.yml
36-36: label "buildjet-4vcpu-ubuntu-2204" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🔇 Additional comments (4)
.github/workflows/api-v2-breaking-changes.yml (4)
36-36: Verify runner labelbuildjet-4vcpu-ubuntu-2204 isn’t a standard GitHub-hosted label; ensure your org has this runner (or switch to ubuntu-22.04).
Suggested fallback:
- runs-on: buildjet-4vcpu-ubuntu-2204 + runs-on: ubuntu-22.04
85-85: Pin the oasdiff action (don’t use @main)Use a released tag or commit SHA.
- uses: oasdiff/oasdiff-action/breaking@main + uses: oasdiff/oasdiff-action/breaking@v1
3-5: Thanks for adding workflow_callThis enables reuse from other workflows.
87-87: Fix raw.githubusercontent base URLrefs/heads/main will 404 on raw.githubusercontent.com; use the branch name directly.
- base: https://raw.githubusercontent.com/calcom/cal.com/refs/heads/main/docs/api-reference/v2/openapi.json + base: https://raw.githubusercontent.com/calcom/cal.com/main/docs/api-reference/v2/openapi.json
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/pr.yml (1)
221-221: Avoid coupling merge-reports to api-v2-breaking-changes.
merge-reportsaggregates E2E artifacts; the breaking-changes job doesn’t produce those and can unnecessarily block it. Consider removing it fromneeds.- needs: [changes, check-label, e2e, e2e-embed, e2e-embed-react, e2e-app-store, e2e-atoms, api-v2-breaking-changes] + needs: [changes, check-label, e2e, e2e-embed, e2e-embed-react, e2e-app-store, e2e-atoms]
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
.github/workflows/pr.yml(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: calcom/cal.com#0
File: .cursor/rules/review.mdc:0-0
Timestamp: 2025-07-28T11:50:23.946Z
Learning: When changes to API v2 or v1 are made, ensure there are no breaking changes on existing endpoints; new functionality must be added as a newly versioned endpoint while keeping the old one functional
🪛 actionlint (1.7.7)
.github/workflows/pr.yml
208-208: property "check-label" is not defined in object type {changes: {outputs: {commit-sha: string; has-files-requiring-all-checks: string}; result: string}}
(expression)
🔇 Additional comments (2)
.github/workflows/pr.yml (2)
205-211: Ensure the called workflow supports workflow_call.The reusable workflow must declare
on: workflow_callor this job won’t load.
267-268: Including api-v2-breaking-changes in required is good.This enforces blocking on breaking API changes. Once the job’s
ifis corrected (see above), this should behave as intended.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.github/workflows/e2e-api-v2.yml(1 hunks)apps/api/v2/package.json(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/v2/package.json
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: supalarry
PR: calcom/cal.com#24028
File: .github/workflows/api-v2-breaking-changes.yml:9-31
Timestamp: 2025-09-24T10:33:39.693Z
Learning: The Cal.com API v2 swagger generation process requires starting up the entire API v2 application with full database connectivity, Redis, and all API keys to introspect the application and generate the complete OpenAPI specification. This means the generate-swagger step needs all production-like environment variables to function properly.
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Install dependencies / Yarn install & cache
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (1)
.github/workflows/e2e-api-v2.yml (1)
79-82: Ensure Swagger generation has required services and run earlier to fail fast
- Verified: "generate-swagger" exists in apps/api/v2/package.json and the generator writes to "../../../docs/api-reference/v2/openapi.json" (apps/api/v2/src/swagger/generate-swagger.ts).
- Action: Ensure this workflow job provides DB, Redis, and required API keys/secrets (or add them to the job) so swagger generation can boot the app.
- Suggestion: Move the Generate Swagger step before E2E tests to fail fast on spec generation errors and save CI time.
| - name: Check breaking changes | ||
| uses: oasdiff/oasdiff-action/breaking@main | ||
| with: | ||
| base: https://raw.githubusercontent.com/calcom/cal.com/refs/heads/main/docs/api-reference/v2/openapi.json | ||
| revision: docs/api-reference/v2/openapi.json | ||
| fail-on: WARN | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
🧩 Analysis chain
Pin the action version and stabilize the base spec reference
- Security/reliability: Avoid using a mutable action ref. Pin to a release or SHA instead of @main.
- Flakiness: Referencing the moving head of main via a remote URL can cause nondeterministic results during PR runs. Prefer checking out main in a separate path and diffing a local file.
Apply this minimal change to stop tracking a moving branch:
- - name: Check breaking changes
- uses: oasdiff/oasdiff-action/breaking@main
+ - name: Check breaking changes
+ uses: oasdiff/oasdiff-action/breaking@<pinned-commit-sha>
with:
- base: https://raw.githubusercontent.com/calcom/cal.com/refs/heads/main/docs/api-reference/v2/openapi.json
+ base: https://raw.githubusercontent.com/calcom/cal.com/main/docs/api-reference/v2/openapi.json
revision: docs/api-reference/v2/openapi.json
fail-on: WARNOptionally, make the base deterministic by checking out main locally (example step to insert before “Check breaking changes”):
- name: Checkout main for base spec
uses: actions/checkout@v4
with:
ref: main
path: base-main
# Then use:
# base: base-main/docs/api-reference/v2/openapi.json🌐 Web query:
What is the latest stable tag for oasdiff/oasdiff-action and the recommended way to pin it (release tag vs commit SHA)?
💡 Result:
Latest stable release tag: v0.0.21. [1]
Recommended pinning: pin to the release's full commit SHA for immutability (or use the release tag if you want auto-updates); GitHub recommends using a commit SHA to ensure stability/security. [2]
Example:
- by tag: uses: oasdiff/oasdiff-action@v0.0.21
- immutable pin (recommended): uses: oasdiff/oasdiff-action@<full-commit-SHA-for-v0.0.21>
Sources:
[1] oasdiff/oasdiff-action releases.
[2] GitHub Actions docs — pin actions to SHAs.
Pin oasdiff action and make base spec deterministic
Pin the action to the latest stable release (v0.0.21) — preferably the release's full commit SHA for immutability — and stop fetching the base spec from a moving remote URL; checkout main and diff a local file.
File: .github/workflows/e2e-api-v2.yml (lines 83–89)
- - name: Check breaking changes
- uses: oasdiff/oasdiff-action/breaking@main
+ - name: Check breaking changes
+ uses: oasdiff/oasdiff-action/breaking@v0.0.21 # or @<full-commit-SHA-for-v0.0.21> (recommended)
with:
- base: https://raw.githubusercontent.com/calcom/cal.com/refs/heads/main/docs/api-reference/v2/openapi.json
+ base: base-main/docs/api-reference/v2/openapi.json
revision: docs/api-reference/v2/openapi.json
fail-on: WARNOptional step to add before the check step:
- name: Checkout main for base spec
uses: actions/checkout@v4
with:
ref: main
path: base-main🤖 Prompt for AI Agents
In .github/workflows/e2e-api-v2.yml around lines 83 to 89, the workflow uses the
oasdiff action via the floating ref "main" and pulls the base spec from a remote
URL; pin the action to a stable release (e.g.,
oasdiff/oasdiff-action/breaking@v0.0.21 or the action's full commit SHA) and
stop using the remote base URL by checking out the main branch locally first
(use actions/checkout@v4 with ref: main into a directory like base-main) and
then point the action's base input to the checked-out local file (e.g.,
base-main/docs/api-reference/v2/openapi.json) so the comparison is deterministic
and immutable.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
E2E results are ready! |
Linear CAL-6017