Skip to content

fix: reject generated @All contracts - #684

Merged
kang-heewon merged 1 commit into
trunkfrom
fix-668-codegen-all-routes
Jun 14, 2026
Merged

fix: reject generated @All contracts#684
kang-heewon merged 1 commit into
trunkfrom
fix-668-codegen-all-routes

Conversation

@kang-heewon

@kang-heewon kang-heewon commented Jun 14, 2026

Copy link
Copy Markdown
Member

Summary

OpenAPI and RPC generators now reject runtime-only @All routes with clear route-specific diagnostics, so generated public contracts no longer fail generically or emit invalid client request methods.

Fixes #668

변경 사항

  • Added an OpenAPI generator diagnostic for @All routes that includes controller, method, and path context.
  • Added RPC client route validation before writing generated files so method: 'ALL' is never emitted.
  • Added OpenAPI coverage with a real @All controller and RPC coverage for both direct RouteIR generation and extracted controller metadata.
  • Added a patch changeset for @croco/openapi-spec and @croco/rpc-codegen.

Verification

  • Reproduced the pre-fix RPC behavior: focused RPC tests failed because ALL routes were accepted and generated.
  • pnpm --filter @croco/openapi-spec exec vitest run src/tests/emitOpenAPI.spec.ts -t "@All"
  • pnpm --filter @croco/rpc-codegen exec vitest run src/tests/codegen.spec.ts -t "ALL routes"
  • pnpm --filter @croco/rpc-codegen exec vitest run src/tests/e2e.spec.ts -t "@All"
  • pnpm test --filter=@croco/openapi-spec
  • pnpm test --filter=@croco/rpc-codegen
  • pnpm typecheck --filter=@croco/openapi-spec --filter=@croco/rpc-codegen
  • pnpm build --filter=@croco/openapi-spec --filter=@croco/rpc-codegen
  • pnpm --filter @croco/openapi-spec lint
  • pnpm --filter @croco/rpc-codegen lint
  • pnpm check
  • pnpm typecheck
  • pnpm test
  • pnpm build
  • pre-commit: oxfmt, oxlint
  • pre-push: auto-changeset, test, typecheck

Self-review

  • Correctness/regression: generated contract surfaces now reject @All before producing invalid OpenAPI/RPC output, and tests cover both generators plus extracted RPC route metadata.
  • API/security/compatibility: runtime HTTP @All behavior is untouched; the public generator behavior changes only for routes that could not be represented as concrete generated contracts.
  • Maintainability/minimality: the diff is limited to local generator validation, focused diagnostics, tests, and a changeset; no new dependencies or shared package APIs were introduced.

Risk

Users with @All routes in generated-contract inputs must replace them with explicit concrete HTTP method decorators for OpenAPI/RPC generation.

Summary by CodeRabbit

릴리스 노트

  • 버그 수정

    • OpenAPI 및 RPC 코드 생성기가 런타임 전용 @All 라우트를 명확한 진단 메시지로 거부하도록 개선되었습니다.
    • 잘못된 공개 계약을 생성하는 대신 코드 생성 단계에서 유효하지 않은 라우트를 사전에 감지합니다.
  • 테스트

    • @All 라우트 거부 동작에 대한 테스트 케이스가 추가되었습니다.

@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

openapi-spectoHttpMethod 함수와 rpc-codegengenerateClientFiles@All 라우트 거부 로직을 추가했다. 두 패키지 모두 httpMethodALL인 라우트를 감지하면 컨트롤러·메서드·경로 정보를 포함한 명시적 예외를 발생시킨다. 각 패키지에 단위·e2e 테스트와 changeset이 함께 추가되었다.

Changes

@All 라우트 코드 생성 거부 처리

Layer / File(s) Summary
emitOpenAPI의 @All 라우트 거부 구현 및 테스트
packages/openapi-spec/src/libs/emitOpenAPI.ts, packages/openapi-spec/src/tests/emitOpenAPI.spec.ts
toHttpMethodRouteIR 전체를 인자로 받도록 변경되고, httpMethodall이면 formatRoute 헬퍼로 구성한 메시지로 예외를 발생시킨다. toRouteConfig 호출부도 toHttpMethod(route)로 갱신되며, 테스트에 @All 예외 검증 케이스가 추가된다.
generateClientFiles의 @All 라우트 거부 구현 및 테스트
packages/rpc-codegen/src/libs/generate.ts, packages/rpc-codegen/src/tests/codegen.spec.ts, packages/rpc-codegen/src/tests/e2e.spec.ts
generateClientFiles 진입 시 assertGeneratedClientRoutes를 호출해 httpMethod: "ALL" 라우트를 조기 거부한다. formatRoute 헬퍼가 에러 메시지에 라우트 식별 정보를 제공하며, 단위 테스트와 e2e 테스트(getAllControllerSource 포함)가 함께 추가된다.
Changeset 릴리즈 기록
.changeset/codegen-all-routes.md
@croco/openapi-spec@croco/rpc-codegen에 대해 @All 라우트 거부 동작을 패치 수준으로 문서화한다.

예상 코드 리뷰 노력

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed 제목 'fix: reject generated @All contracts'는 주요 변경 사항인 @All 라우트 거부 로직을 명확하게 요약합니다.
Linked Issues check ✅ Passed PR이 모든 #668 요구사항을 충족합니다: OpenAPI 및 RPC 생성기 모두에서 @All 라우트를 거부, 명확한 진단 메시지 포함, 포괄적인 테스트 커버리지 추가, 런타임 동작 보존.
Out of Scope Changes check ✅ Passed 모든 변경사항이 @All 라우트 거부 및 검증 로직과 관련된 범위 내 변경입니다: 검증 함수, 테스트, changeset 파일.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-668-codegen-all-routes

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions

Copy link
Copy Markdown

📊 Benchmark Results

❌ Some benchmarks failed

Benchmark p75 Threshold Baseline vs Baseline Status
CrocoApp benchmarks 3.2μs - - - ⚠️
EventBusConfig.start (10 handlers) 1.4μs - 9.0μs -84.6%
EventPublisher.publishNow single event 1.6μs - - - ⚠️
DefaultHandlerResolver.resolve × 10 0.1μs - 0.2μs -60.0%
Container.get singleton (cold) 0.9μs - 0.5μs +86.2%
Container.register × 50 components 10.5μs - 8.0μs +31.8%
Container.validate (50 components) 27.7μs - 25.0μs +10.8%
Container.get singleton (warm) 0.4μs - 0.3μs +30.3%
TelemetryRuntime benchmarks 2.0μs - 2.0μs -1.8%

Updated: 2026-06-14T11:49:05.574Z · Commit: 3f96f19

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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 @.changeset/codegen-all-routes.md:
- Line 6: The markdown file .changeset/codegen-all-routes.md is triggering a
markdownlint MD041 warning, which requires proper heading structure. Fix this by
either adding an appropriate heading (such as an H1 or H2) at the beginning or
before the content line that starts with "OpenAPI and RPC generators now reject
runtime-only", while maintaining the changeset format, or alternatively add a
linting configuration exception for this specific file to suppress the MD041
rule where the changeset structure does not allow for traditional markdown
headings.

In `@packages/openapi-spec/src/libs/emitOpenAPI.ts`:
- Around line 172-174: Both `packages/openapi-spec/src/libs/emitOpenAPI.ts`
(lines 172-174) and `packages/rpc-codegen/src/libs/generate.ts` (lines 37-39)
are throwing generic Error instances for `@All` route rejection instead of
Problem subclasses. Replace both generic Error throws with an appropriate
Problem subclass (following RFC 7807 Problem-based error handling) to maintain
consistent error handling across the generators. Use the same Problem subclass
in both locations to ensure uniform error contract compliance.
🪄 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: 23efd627-be1d-4d07-a795-e6cc2295d98c

📥 Commits

Reviewing files that changed from the base of the PR and between 0ee21dc and bc95986.

📒 Files selected for processing (6)
  • .changeset/codegen-all-routes.md
  • packages/openapi-spec/src/libs/emitOpenAPI.ts
  • packages/openapi-spec/src/tests/emitOpenAPI.spec.ts
  • packages/rpc-codegen/src/libs/generate.ts
  • packages/rpc-codegen/src/tests/codegen.spec.ts
  • packages/rpc-codegen/src/tests/e2e.spec.ts

Comment thread .changeset/codegen-all-routes.md
Comment thread packages/openapi-spec/src/libs/emitOpenAPI.ts
@kang-heewon
kang-heewon merged commit 8b28607 into trunk Jun 14, 2026
8 checks passed
@kang-heewon
kang-heewon deleted the fix-668-codegen-all-routes branch June 14, 2026 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[codegen] Handle @All routes before generating public contracts

1 participant