fix: enforce strict generated contract graphs - #1217
Merged
Conversation
|
Warning Review limit reached
Next review available in: 53 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (64)
📝 WalkthroughWalkthrough이 PR은 생성된 앱 경로의 ContractGraph를 strict하게 검증하도록 바꾸고, openapi-spec/rpc-codegen CLI에 fail-on-diagnostics와 strict/compatibility 분기 처리를 추가했다. 생성 앱 템플릿의 컨트롤러와 라우트 계약, 문서, 스모크 테스트, 문제 레지스트리도 함께 갱신됐다. ChangesContract Graph 코어 및 CLI Strict 모드
생성 앱 템플릿의 라우트 계약 전환
Problem Registry Route Projection 처리
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Developer
participant CLI as croco-openapi-spec / croco-rpc-codegen
participant Graph as ContractGraph
participant Output as OpenAPI / RPC artifacts
Developer->>CLI: run with strict / compatibility flags
CLI->>Graph: build and validate graph
Graph-->>CLI: diagnostics or errors
alt blocking diagnostics present
CLI-->>Developer: exit 1, print diagnostics
else no blocking diagnostics
CLI->>Output: generate artifacts
Output-->>Developer: files written
end
sequenceDiagram
participant Template as create-croco-app template
participant Controller as generated controller
participant Script as contract script
participant Smoke as generated smoke test
Template->>Controller: emit route contracts and problem responses
Template->>Script: add --fail-on-diagnostics
Smoke->>Script: run strict generation
Script-->>Smoke: fail on diagnostics or emit artifacts
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8dd8a25c79
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
📊 Benchmark Results✅ All benchmarks passed
Updated: 2026-07-05T09:01:07.604Z · Commit: aa26891 |
kang-heewon
force-pushed
the
fix/1184-strict-contract-graph
branch
from
July 4, 2026 16:43
8dd8a25 to
78aa981
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/openapi-spec/src/libs/cli.ts (1)
101-149: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
parseStrictProblems/parseStrictSchemas/reportContractGraph로직이 rpc-codegen CLI와 완전히 중복됩니다.
packages/rpc-codegen/src/libs/cli.ts의 145-166, 213-233 라인과 이 파일의 로직이 문구(에러 메시지 문자열)를 제외하고 사실상 동일합니다. strict/compatibility 판정 및 diagnostics 차단 로직은 이번 PR의 핵심 게이트인데, 두 패키지 모두 이미@croco/protocols-core를 참조하고 있으므로 공통 헬퍼로 추출하면 향후 두 구현이 서로 다르게 발전(drift)하는 위험을 없앨 수 있습니다.♻️ 제안: 공통 헬퍼를 protocols-core로 추출
+// packages/protocols-core/src/libs/cliStrictMode.ts +export function parseStrictModeFlag( + args: readonly string[], + strictFlag: string, + compatibilityFlag: string, +): boolean | null { + const strict = args.includes(strictFlag); + const compatibility = args.includes(compatibilityFlag); + if (strict && compatibility) { + return null; + } + return !compatibility; +} + +export function resolveBlockingDiagnostics( + graph: ContractGraph, + failOnDiagnostics: boolean, +): readonly ContractDiagnostic[] { + return failOnDiagnostics ? graph.diagnostics : getContractGraphErrors(graph); +}각 CLI에서는 다음과 같이 호출:
-function parseStrictProblems(args: readonly string[]): boolean | null { - const strictProblems = args.includes("--strict-problems"); - const compatibilityProblems = args.includes("--compatibility-problems"); - if (strictProblems && compatibilityProblems) { - return null; - } - return !compatibilityProblems; -} +const strictProblems = parseStrictModeFlag(args, "--strict-problems", "--compatibility-problems");Also applies to: 217-237
🤖 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 `@packages/openapi-spec/src/libs/cli.ts` around lines 101 - 149, The strict/compatibility parsing and diagnostics gating logic in the CLI is duplicated with rpc-codegen, so extract the shared behavior into a common helper in `@croco/protocols-core` and have the CLI use that instead. Refactor the parseStrictProblems and parseStrictSchemas flow, along with the reportContractGraph-related diagnostics check, to call the shared helper so both packages stay in sync while preserving the existing CLI-specific error messages.
🤖 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
`@packages/create-croco-app/templates/ai-saas/apps/api-server/src/controllers/aiSchemas.ts`:
- Around line 1-12: The import block in aiSchemas should follow the project’s
ordering rules: external packages first, then internal `@croco/`* packages, then
relative imports, with type imports separated when applicable. Reorder the
imports in this file so zod is grouped with external dependencies before
`@croco/problems-core` and `@croco/protocols-rest`, while keeping the aiProblems
relative import last.
In
`@packages/create-croco-app/templates/spa-be-split/apps/api-server/src/controllers/userSchemas.ts`:
- Around line 1-4: The import order in the userSchemas module is out of
guideline order because the external package import for zod is placed after
internal `@croco/`* imports. Reorder the imports in userSchemas so they follow
external packages first, then `@croco/`* packages, then relative paths, and keep
any type-only imports in their own section if applicable. Use the existing
import block in userSchemas as the target to fix.
In `@packages/create-croco-app/templates/spa-be-split/README.md.hbs`:
- Around line 48-49: The new README paragraph in the template is still written
in English, breaking the document’s Korean language consistency. Translate the
added ContractGraph/OpenAPI/RPC explanation into Korean in the README template,
keeping the same meaning about strict schema checks, `--fail-on-diagnostics`,
and the `--compatibility-*` migration-only opt-outs.
---
Outside diff comments:
In `@packages/openapi-spec/src/libs/cli.ts`:
- Around line 101-149: The strict/compatibility parsing and diagnostics gating
logic in the CLI is duplicated with rpc-codegen, so extract the shared behavior
into a common helper in `@croco/protocols-core` and have the CLI use that instead.
Refactor the parseStrictProblems and parseStrictSchemas flow, along with the
reportContractGraph-related diagnostics check, to call the shared helper so both
packages stay in sync while preserving the existing CLI-specific error messages.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 27668cf4-df7c-4cc7-8d35-e65a66142710
⛔ Files ignored due to path filters (1)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**
📒 Files selected for processing (41)
.changeset/strict-generated-contract-graphs.mddocs/problem-code-registry.jsondocs/release/contract-first-gates.mdpackages/cli/src/commands/generateUsageDashboard.tspackages/cli/src/tests/generateUsageDashboard.spec.tspackages/cli/src/tests/integration/e2e.spec.tspackages/create-croco-app/src/tests/templates-build.spec.tspackages/create-croco-app/templates/admin-console/README.md.hbspackages/create-croco-app/templates/admin-console/apps/api-server/src/controllers/AdminController.tspackages/create-croco-app/templates/admin-console/apps/api-server/src/controllers/adminSchemas.tspackages/create-croco-app/templates/admin-console/package.json.hbspackages/create-croco-app/templates/ai-saas/README.md.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/src/controllers/AiController.tspackages/create-croco-app/templates/ai-saas/apps/api-server/src/controllers/aiSchemas.tspackages/create-croco-app/templates/ai-saas/package.json.hbspackages/create-croco-app/templates/saas/README.md.hbspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/JobsController.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/OperationsController.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/SaasController.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/schemas.tspackages/create-croco-app/templates/saas/package.json.hbspackages/create-croco-app/templates/spa-be-split/README.md.hbspackages/create-croco-app/templates/spa-be-split/apps/api-server/src/controllers/UserController.tspackages/create-croco-app/templates/spa-be-split/apps/api-server/src/controllers/userSchemas.tspackages/create-croco-app/templates/spa-be-split/package.json.hbspackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/openapi-spec/src/libs/cli.tspackages/openapi-spec/src/tests/Cli.spec.tspackages/protocols-core/src/libs/ContractGraph.tspackages/protocols-core/src/libs/RouteIR.tspackages/protocols-core/src/libs/extractRouteIR.tspackages/protocols-core/src/tests/ContractGraph.spec.tspackages/protocols-core/src/tests/extractRouteIR.spec.tspackages/rpc-codegen/src/libs/cli.tspackages/rpc-codegen/src/tests/Cli.spec.tspackages/rpc-codegen/src/tests/ContractCheckCli.spec.tspackages/rpc-codegen/src/tests/codegen.spec.tsscripts/create-croco-app-generated-smoke.mtsscripts/problem-registry.mtsscripts/static-misuse-raw-error-allowlist.jsonscripts/tests/problem-registry.spec.ts
💤 Files with no reviewable changes (1)
- scripts/static-misuse-raw-error-allowlist.json
kang-heewon
force-pushed
the
fix/1184-strict-contract-graph
branch
2 times, most recently
from
July 4, 2026 17:50
925b8a0 to
4a51127
Compare
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 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 `@packages/cli/src/tests/generateUsageDashboard.spec.ts`:
- Around line 73-75: The tests for generateUsageDashboard only assert that
certain strings exist, so they do not verify the real route produced by
combining `@Controller` with `@Get`(usageDashboardSnapshotRoute) and the contract
path. Update the generateUsageDashboard.spec.ts assertions to validate the
resolved controller route behavior for generateUsageDashboard.ts, using the
unique symbols usageDashboardSnapshotRoute and controllerContent, so the test
catches duplicate or conflicting route configurations instead of just matching
substrings.
In
`@packages/create-croco-app/templates/admin-console/apps/api-server/src/controllers/AdminController.ts`:
- Around line 25-62: The controller handlers are missing explicit RouteResponse
return types, so the route response contract is no longer checked at compile
time. Update AdminController methods like snapshot, listUsers, getUser,
createUser, and listOperations to return Promise<RouteResponse<typeof ...Route>>
and import RouteResponse alongside RouteBody/RouteParam/RouteQueryParam. Keep
the same pattern used in UserController so each handler’s service result is
validated against its route definition.
In
`@packages/create-croco-app/templates/saas/apps/api-server/src/controllers/schemas.ts`:
- Around line 1-3: The import ordering in the schema module is out of guideline:
`zod` should be grouped before the internal `@croco/*` imports. Update the
import block in `schemas.ts` (and mirror the same ordering pattern in
`userSchemas.ts` where applicable) so external packages like `zod` come first,
followed by `@croco/problems-core` and `@croco/protocols-rest`, keeping imports
organized consistently with the project import rules.
In `@packages/openapi-spec/src/libs/cli.ts`:
- Around line 62-77: There is duplicated blocking-diagnostics selection logic in
runCli and reportContractGraph, both combining getContractGraphErrors with
failOnDiagnostics to decide which diagnostics are blocking. Extract that shared
computation into a common helper and have both call sites use it, so the
blockingDiagnostics selection and related message logic live in one place. Use
the existing runCli, reportContractGraph, getContractGraphErrors, and
failOnDiagnostics symbols to centralize the behavior without changing the CLI
output.
In `@packages/protocols-core/src/libs/ContractGraph.ts`:
- Around line 798-805: The duplicate diagnostics issue in
validateRouteContractProblemResponses comes from iterating
route.routeContract.problemResponses directly when problemResponsesDeclared is
true, which allows repeated code values to generate repeated missing/mismatch
results. Update the ContractGraph route contract validation path to deduplicate
problem responses by code even for declared contracts, either by applying the
same code-based unique filtering used by getRouteContractProblemResponses or by
reusing that deduped helper before building diagnostics.
In `@packages/rpc-codegen/src/libs/cli.ts`:
- Around line 54-69: The blocking-diagnostics calculation and message selection
are duplicated between runCli and reportContractGraph, so consolidate this logic
into a shared helper used by both paths. Extract the error vs diagnostic
selection and the corresponding stdout message into a single function or
utility, then call it from runCli and reportContractGraph so --check and normal
generation always use the same blocking criteria and wording.
- Around line 145-166: The strict/compatibility parsing logic in
parseStrictProblems, parseStrictSchemas, and reportContractGraph is duplicated
across the CLI helpers, so extract the shared strict/compatibility handling into
a common cli-kit utility and reuse it from the CLI modules. Keep the existing
semantics intact for default strict behavior and the opt-out flags, and make
sure both packages import the same helper instead of maintaining separate
copies.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d05a8e30-427f-4e08-a3bf-a5bc439b8f81
⛔ Files ignored due to path filters (1)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**
📒 Files selected for processing (43)
.changeset/strict-generated-contract-graphs.mddocs/problem-code-registry.jsondocs/release/contract-first-gates.mdpackages/cli/src/commands/generateUsageDashboard.tspackages/cli/src/tests/generateUsageDashboard.spec.tspackages/cli/src/tests/integration/e2e.spec.tspackages/create-croco-app/src/tests/templates-build.spec.tspackages/create-croco-app/templates/admin-console/README.md.hbspackages/create-croco-app/templates/admin-console/apps/api-server/src/controllers/AdminController.tspackages/create-croco-app/templates/admin-console/apps/api-server/src/controllers/adminSchemas.tspackages/create-croco-app/templates/admin-console/package.json.hbspackages/create-croco-app/templates/ai-saas/README.md.hbspackages/create-croco-app/templates/ai-saas/apps/api-server/src/controllers/AiController.tspackages/create-croco-app/templates/ai-saas/apps/api-server/src/controllers/aiSchemas.tspackages/create-croco-app/templates/ai-saas/package.json.hbspackages/create-croco-app/templates/saas/README.md.hbspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/JobsController.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/OperationsController.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/SaasController.tspackages/create-croco-app/templates/saas/apps/api-server/src/controllers/schemas.tspackages/create-croco-app/templates/saas/package.json.hbspackages/create-croco-app/templates/spa-be-split/README.md.hbspackages/create-croco-app/templates/spa-be-split/apps/api-server/src/controllers/UserController.tspackages/create-croco-app/templates/spa-be-split/apps/api-server/src/controllers/userSchemas.tspackages/create-croco-app/templates/spa-be-split/package.json.hbspackages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.mdpackages/docs/src/content/docs/api/protocols-core/src/type-aliases/RouteContractIR.mdpackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/openapi-spec/src/libs/cli.tspackages/openapi-spec/src/tests/Cli.spec.tspackages/protocols-core/src/libs/ContractGraph.tspackages/protocols-core/src/libs/RouteIR.tspackages/protocols-core/src/libs/extractRouteIR.tspackages/protocols-core/src/tests/ContractGraph.spec.tspackages/protocols-core/src/tests/extractRouteIR.spec.tspackages/rpc-codegen/src/libs/cli.tspackages/rpc-codegen/src/tests/Cli.spec.tspackages/rpc-codegen/src/tests/ContractCheckCli.spec.tspackages/rpc-codegen/src/tests/codegen.spec.tsscripts/create-croco-app-generated-smoke.mtsscripts/problem-registry.mtsscripts/static-misuse-raw-error-allowlist.jsonscripts/tests/problem-registry.spec.ts
kang-heewon
force-pushed
the
fix/1184-strict-contract-graph
branch
2 times, most recently
from
July 5, 2026 07:53
2e0f673 to
1270ad9
Compare
kang-heewon
force-pushed
the
fix/1184-strict-contract-graph
branch
from
July 5, 2026 08:53
1270ad9 to
ea214a0
Compare
This was referenced Jul 16, 2026
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
1 participant
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.
Fixes #1184.
Summary
--compatibility-*opt-outs.--fail-on-diagnosticsand wires generated app contract scripts to fail on strict warnings/errors before OpenAPI or RPC artifacts are written.neverfailure unions, and keeps extra Problem metadata outside the route contract as a ContractGraph error.Verification
COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm --filter @croco/openapi-spec exec vitest run src/tests/Cli.spec.ts --reporter=verbose- passed.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm --filter @croco/rpc-codegen exec vitest run src/tests/Cli.spec.ts src/tests/ContractCheckCli.spec.ts src/tests/codegen.spec.ts --reporter=verbose- passed.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm --filter @croco/cli exec vitest run src/tests/generateUsageDashboard.spec.ts --reporter=verbose- passed.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm --filter @croco/cli test:e2e -- --runInBand- passed, 9 integration tests.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm --filter @croco/protocols-core exec vitest run src/tests/extractRouteIR.spec.ts src/tests/ContractGraph.spec.ts --reporter=verbose- passed, 66 tests.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm exec vitest run scripts/tests/create-croco-app-generated-smoke.spec.ts --config vitest.config.ts --reporter=verbose- passed.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm exec vitest run scripts/tests/problem-registry.spec.ts --config vitest.config.ts --reporter=verbose- passed, 15 tests.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm --filter create-croco-app exec vitest run src/tests/templates-build.spec.ts --reporter=verbose- passed, 11 tests.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm --filter create-croco-app test- passed, 93 tests.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm static-misuse:check- passed.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm problem-registry:write- passed, 412 codes from 412 discoveries.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 CROCO_GENERATED_SMOKE_CASES=goal-saas-api,admin-console-starter,saas-golden-path,saas-cloudflare-profile,saas-lambda-profile,ai-saas-golden-path corepack pnpm create-croco-app:smoke- passed.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm create-croco-app:smoke- passed, all generated app smoke cases.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm check- passed.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm release-docs:check- passed.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm changeset-required:check -- --base origin/trunk --head HEAD- passed.git diff --check HEAD^ HEAD- passed.auto-changeset,test, andtypecheckpassed on the pushed branch after the final amend; cached rerun reported 225/225 test tasks and 224/224 typecheck tasks.Self-review gates
Notes
Summary by CodeRabbit