Skip to content

fix: validate DI provider graphs before startup - #1041

Merged
kang-heewon merged 2 commits into
trunkfrom
fix/977-di-graph-check
Jun 20, 2026
Merged

fix: validate DI provider graphs before startup#1041
kang-heewon merged 2 commits into
trunkfrom
fix/977-di-graph-check

Conversation

@kang-heewon

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

Copy link
Copy Markdown
Member

Closes #977.

Summary

  • Exposes deterministic DI graph manifests from @croco/framework-context, including missing provider, circular dependency, scope mismatch, and TypeDI fallback diagnostics.
  • Exposes deterministic module graph manifests from @croco/framework-module, including pre-start module cycle and provider visibility diagnostics.
  • Adds croco di check so CI can validate DI/module manifest JSON without importing or running app code.
  • Updates public API snapshots, generated framework-context API docs, tests, and changesets for the new public surface.

Validation

  • pnpm --filter @croco/framework-context exec vitest run src/tests/DependencyGraphManifest.spec.ts
  • pnpm --filter @croco/framework-module exec vitest run src/tests/ModuleGraphManifest.spec.ts
  • pnpm --filter @croco/cli exec vitest run src/tests/diCheck.spec.ts
  • pnpm test --filter=@croco/framework-context
  • pnpm test --filter=@croco/framework-module
  • pnpm --filter @croco/cli test
  • pnpm --filter @croco/framework-context typecheck
  • pnpm --filter @croco/framework-module typecheck
  • pnpm --filter @croco/cli typecheck
  • pnpm --filter @croco/framework-context lint
  • pnpm --filter @croco/framework-module lint
  • pnpm --filter @croco/cli lint
  • pnpm public-api:check
  • pnpm package-manifests:check
  • pnpm changeset-required:check -- --base origin/trunk --head HEAD
  • pnpm check
  • pnpm typecheck
  • pnpm create-croco-app:smoke
  • pre-push pnpm test
  • pre-push pnpm typecheck

Review notes

  • croco di check reads an explicit manifest file and reports its diagnostics; it does not load controllers, providers, modules, or user application code.
  • TypeDI fallback providers are surfaced as explicit framework-context/di-unknown-provider errors so static verification does not hide unverifiable runtime state.

Summary by CodeRabbit

릴리스 노트

  • New Features

    • CLI에 di check 명령 추가로 의존성 주입 및 모듈 그래프 검증 가능
    • 매니페스트 생성 기능을 통해 의존성 및 모듈 그래프를 정적으로 분석 및 내보내기
    • JSON 및 사람이 읽을 수 있는 형식의 진단 리포트 지원
  • Documentation

    • 의존성 그래프 매니페스트 및 진단 관련 API 문서 추가
    • 모듈 그래프 타입 및 구조 문서화
  • Tests

    • CLI 및 매니페스트 생성 기능에 대한 테스트 스위트 추가

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown

Review Change Stack

Warning

Review limit reached

@kang-heewon, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 40 minutes and 16 seconds. 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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

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, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 57eb08e2-a93e-430e-94b8-b094d78487ae

📥 Commits

Reviewing files that changed from the base of the PR and between e44250d and c0cebeb.

📒 Files selected for processing (11)
  • packages/cli/src/commands/diCheck.ts
  • packages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphDiagnostic.md
  • packages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphManifest.md
  • packages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphProvider.md
  • packages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyResolutionStep.md
  • packages/framework-context/src/libs/Container.ts
  • packages/framework-context/src/libs/types.ts
  • packages/framework-context/src/tests/DependencyGraphManifest.spec.ts
  • packages/framework-module/src/ModuleRegistry.ts
  • packages/framework-module/src/tests/ModuleGraphManifest.spec.ts
  • packages/framework-module/src/types.ts
📝 Walkthrough

Walkthrough

framework-contextContainerframework-moduleModuleRegistry에 결정적 DI/모듈 그래프 매니페스트 생성 API를 추가한다. missing provider, circular dependency, scope mismatch, module visibility 위반을 앱 시작 전에 진단하며, croco di check CLI 명령으로 CI에서 매니페스트를 검증할 수 있다.

Changes

DI/모듈 그래프 매니페스트 및 croco di check CLI

Layer / File(s) Summary
DI/모듈 그래프 타입 계약 정의
packages/framework-context/src/libs/types.ts, packages/framework-module/src/types.ts
DependencySourceLocation, DependencyGraphManifest/Diagnostic/DiagnosticCode/Provider, DependencyGraphManifestVersion/Status 타입과 ModuleGraphManifest/Diagnostic/DiagnosticCode/Module/Provider, ModuleGraphManifestVersion/Status 타입을 각각 신규 추가한다.
Container.createDependencyGraphManifest 구현
packages/framework-context/src/libs/Container.ts, packages/framework-context/src/index.ts
componentSourceLocations 맵을 추가해 register/remove/reset 시 소스 위치를 관리하고, createDependencyGraphManifest 정적 메서드가 root 토큰별 trace를 생성한 뒤 providers와 missing/circular/scope-mismatch/unknown 진단을 포함한 매니페스트를 반환한다. captureSourceLocation 스택 파싱 및 normalizeSourceFile 경로 정규화 유틸도 함께 추가된다. 공개 API에 DependencyGraph* 타입을 재-export한다.
DependencyGraphManifest 단위 테스트
packages/framework-context/src/tests/DependencyGraphManifest.spec.ts
missing-provider, circular-dependency, scope-mismatch, typedi-unknown-provider 시나리오에서 createDependencyGraphManifest가 올바른 status, code, path, sourceLocation을 반환하는지 검증한다.
ModuleRegistry.createModuleGraphManifest 구현
packages/framework-module/src/ModuleRegistry.ts, packages/framework-module/src/index.ts
createModuleGraphManifest/stringifyModuleGraphManifest를 추가하고, provider visibility 검증을 getClassProviderVisibilityFailures 실패 목록 수집 방식으로 리팩터링한다. canAccessToken/isKnownToken 계열 함수가 states 맵을 인자로 받도록 변경하고, 순환 의존성과 provider-not-visible 진단 생성 헬퍼를 추가한다. CrocoModule.createGraphManifest() 정적 메서드와 ModuleGraph* 타입 재-export를 추가한다.
ModuleGraphManifest 단위 테스트
packages/framework-module/src/tests/ModuleGraphManifest.spec.ts
클래스 기반 비공개 provider와 Token/@Inject 기반 의존성에서 provider-not-visible 진단이 모듈 startup 이전에 생성되고, status/code/token/path가 올바른지 검증한다.
CLI croco di check 명령 구현 및 라우팅
packages/cli/src/commands/di.ts, packages/cli/src/commands/diCheck.ts, packages/cli/src/commands/root.ts, packages/cli/src/index.ts
di 명령과 check 서브커맨드를 정의하고 root에 lazy 등록한다. runDiCheck가 인자 파싱, JSON 매니페스트 읽기, DiCheckReport 생성, --json/--out 분기 출력, 사람이 읽는 진단 포맷, exit code 결정을 처리한다. di/diCheckDiCheckDiagnostic/DiCheckIo/DiCheckReport 타입을 패키지 엔트리에서 재-export한다.
diCheck CLI 단위 테스트
packages/cli/src/tests/diCheck.spec.ts
failing manifest 진단 출력/exit code, ready manifest 성공 출력, --json/--out JSON 파일 생성, parseDiCheckArgs 플래그 파싱을 검증하는 Vitest 테스트와 createIo 스텁 헬퍼를 포함한다.
API 문서 및 공개 API 스냅샷 업데이트
packages/docs/src/content/docs/api/framework-context/src/..., public-api-surface.snapshot.json, .changeset/di-graph-manifest-check.md
Container.createDependencyGraphManifest 메서드 문서와 DependencyGraph* 타입 문서 페이지를 추가하고, 공개 API 스냅샷에 cli/framework-context/framework-module의 신규 export를 반영한다.

Sequence Diagram(s)

sequenceDiagram
  participant User as 사용자/CI
  participant croco_di_check as croco di check (runDiCheck)
  participant Container as Container.createDependencyGraphManifest
  participant ModuleRegistry as createModuleGraphManifest
  participant DiCheckReport as createDiCheckReport

  rect rgba(70, 130, 180, 0.5)
    note over User, ModuleRegistry: 매니페스트 생성 단계 (앱 빌드/스크립트)
    User->>Container: createDependencyGraphManifest(options?)
    Container->>Container: root 토큰별 trace 생성 및 captureSourceLocation
    Container-->>User: DependencyGraphManifest (JSON 파일로 저장)

    User->>ModuleRegistry: createModuleGraphManifest(rootModules?)
    ModuleRegistry->>ModuleRegistry: provider visibility 검사 및 순환 의존성 포착
    ModuleRegistry-->>User: ModuleGraphManifest (JSON 파일로 저장)
  end

  rect rgba(60, 179, 113, 0.5)
    note over User, DiCheckReport: CI 검증 단계 (croco di check)
    User->>croco_di_check: --manifest di-graph.json [--json] [--out report.json]
    croco_di_check->>croco_di_check: JSON 파일 읽기 및 파싱
    croco_di_check->>DiCheckReport: normalizeDiagnostic 및 status 계산
    DiCheckReport-->>croco_di_check: DiCheckReport (passed/failed)
    alt failed
      croco_di_check-->>User: 포맷된 진단 출력 + exit code 1
    else passed
      croco_di_check-->>User: 성공 메시지 + exit code 0
    end
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 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 PR 제목 'fix: validate DI provider graphs before startup'은 변경사항의 주요 목표인 DI 그래프 검증 기능 추가를 명확하게 요약하고 있으며, 제공된 raw_summary의 여러 파일 변경사항(DI 매니페스트 생성, CLI check 명령 추가 등)과 일치합니다.
Linked Issues check ✅ Passed 이 PR은 #977의 모든 수용 기준을 충족합니다: (1) missing provider, circular dependency, scope mismatch 진단을 DependencyGraphManifest에 포함 [Container.ts], (2) 소스 위치 정보 추가 [types.ts], (3) croco di check 명령 구현 [di.ts, diCheck.ts], (4) 모듈 그래프 매니페스트 추가 [ModuleRegistry.ts], (5) TypeDI 폴백을 di-unknown-provider로 명시화 [Container.ts].
Out of Scope Changes check ✅ Passed 모든 변경사항이 #977 범위 내에 있습니다: DI/모듈 그래프 매니페스트 생성, 진단 노출, CLI check 명령 추가, 및 관련 테스트와 문서 업데이트만 포함되어 있으며, package import boundary/layer policy (#931)와 같은 범위 외 변경은 없습니다.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/977-di-graph-check

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

github-actions Bot commented Jun 20, 2026

Copy link
Copy Markdown

📊 Benchmark Results

❌ Some benchmarks failed

Gate failures

  • CrocoApp constructor: p75 6.8μs exceeds baseline 3.4μs by more than 20%
  • CrocoApp lambdaHandler (10 controllers): p75 231.9μs exceeds baseline 33.3μs by more than 20%
  • Lambda cold-start simulation: p75 330.0μs exceeds baseline 70.2μs by more than 20%
  • Lambda cold-start with headers: p75 300.9μs exceeds baseline 66.7μs by more than 20%
  • Lambda cold-start with binary body: p75 266.2μs exceeds baseline 63.2μs by more than 20%
  • Lambda cold-start with query params: p75 239.6μs exceeds baseline 63.9μs by more than 20%
  • Lambda cold-start with authorizer context: p75 240.1μs exceeds baseline 59.8μs by more than 20%
  • Lambda cold-start realistic scenario: p75 239.0μs exceeds baseline 60.2μs by more than 20%
  • EventBusConfig.start (10 handlers): p75 1.4μs exceeds baseline 0.9μs by more than 20%
  • EventPublisher.publishNow single event: p75 1.6μs exceeds baseline 1.1μs by more than 20%
  • DefaultHandlerResolver.resolve × 10: p75 0.1μs exceeds baseline 0.0μs by more than 20%
  • Container.get singleton (cold): p75 73.4μs exceeds baseline 0.6μs by more than 20%
  • Container.register × 50 components: p75 3.2ms exceeds baseline 12.1μs by more than 20%
  • Container.validate (50 components): p75 3.3ms exceeds baseline 29.7μs by more than 20%
  • Container.get singleton (warm): p75 1.6μs exceeds baseline 0.3μs by more than 20%
  • lambdaPreset config creation: p75 1.5μs exceeds baseline 1.0μs by more than 20%
Benchmark p75 Threshold Baseline vs Baseline Status Notes
CrocoApp constructor 6.8μs 30.0ms 3.4μs +101.0% -
CrocoApp lambdaHandler (10 controllers) 231.9μs 50.0ms 33.3μs +596.5% -
Lambda cold-start simulation 330.0μs 80.0ms 70.2μs +370.3% -
Lambda cold-start with headers 300.9μs 80.0ms 66.7μs +350.8% -
Lambda cold-start with binary body 266.2μs 80.0ms 63.2μs +321.5% -
Lambda cold-start with query params 239.6μs 80.0ms 63.9μs +275.1% -
Lambda cold-start with authorizer context 240.1μs 80.0ms 59.8μs +301.6% -
Lambda cold-start realistic scenario 239.0μs 80.0ms 60.2μs +297.3% -
EventBusConfig.start (10 handlers) 1.4μs 10.0ms 0.9μs +62.6% -
EventPublisher.publishNow single event 1.6μs 2.0ms 1.1μs +51.7% -
DefaultHandlerResolver.resolve × 10 0.1μs 5.0ms 0.0μs +114.3% -
Container.get singleton (cold) 73.4μs 5.0ms 0.6μs +11648.5% -
Container.register × 50 components 3.2ms 10.0ms 12.1μs +26619.7% -
Container.validate (50 components) 3.3ms 20.0ms 29.7μs +11049.0% -
Container.get singleton (warm) 1.6μs 500.0μs 0.3μs +449.0% -
TelemetryRuntime.init (lambda preset) 1.1ms 200.0ms 69.3ms -98.4% -
lambdaPreset config creation 1.5μs 2.0ms 1.0μs +51.3% -

Updated: 2026-06-20T14:52:40.174Z · Commit: 968b31b

@kang-heewon

Copy link
Copy Markdown
Member Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 20, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@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: 4

🤖 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/commands/diCheck.ts`:
- Around line 195-213: The function normalizeDiagnostic contains duplicate calls
to readOptionalString for the "token" and "moduleName" keys. In both cases,
readOptionalString is called once in the ternary condition and again when
constructing the object value. Extract the results of readOptionalString(record,
"token") and readOptionalString(record, "moduleName") into separate variables
before the return statement, then use these variables in the conditional checks
and object spread operations to eliminate the redundant function calls.

In `@packages/framework-context/src/libs/Container.ts`:
- Around line 293-337: The createGraphProviders method uses the token label
string as the Map key for providers aggregation and dependency lookups, which
can cause collisions when different tokens share the same label (e.g.,
same-named classes). Replace the use of step.token as the Map key with a unique
tokenId identifier instead, ensuring both the providers Map operations and the
dependencyOf lookups use this tokenId consistently. This prevents provider
information and source locations from being incorrectly merged or associated
with the wrong tokens.
- Around line 438-444: The regular expression pattern used in the match
statement on line 438 only accepts Unix-style absolute paths starting with
forward slashes or file:// URIs, and fails to match Windows-style paths with
drive letters like C:\. Update the regex pattern to also capture Windows
absolute paths that start with a drive letter followed by a colon, in addition
to the existing Unix path patterns. This will ensure that sourceLocation parsing
works correctly on both Windows and Unix environments.

In `@packages/framework-module/src/ModuleRegistry.ts`:
- Around line 236-238: The diagnostic code string in the diagnostics.push call
at the ModuleRegistry.ts location does not match the code defined in the
ModuleCircularDependencyProblem in problems.ts. Update the code value from
"framework-module/module-circular-dependency" to
"framework-module/circular-dependency" to ensure consistency between the
diagnostic record and the Problem definition, which is required for proper
downstream diagnostic aggregation and filtering.
🪄 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: 151e8c65-440f-4891-b636-514f97d905da

📥 Commits

Reviewing files that changed from the base of the PR and between 40d0aec and e44250d.

📒 Files selected for processing (23)
  • .changeset/di-graph-manifest-check.md
  • packages/cli/src/commands/di.ts
  • packages/cli/src/commands/diCheck.ts
  • packages/cli/src/commands/root.ts
  • packages/cli/src/index.ts
  • packages/cli/src/tests/diCheck.spec.ts
  • packages/docs/src/content/docs/api/framework-context/src/classes/Container.md
  • packages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphDiagnostic.md
  • packages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphDiagnosticCode.md
  • packages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphManifest.md
  • packages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphManifestStatus.md
  • packages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphManifestVersion.md
  • packages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencyGraphProvider.md
  • packages/docs/src/content/docs/api/framework-context/src/type-aliases/DependencySourceLocation.md
  • packages/framework-context/src/index.ts
  • packages/framework-context/src/libs/Container.ts
  • packages/framework-context/src/libs/types.ts
  • packages/framework-context/src/tests/DependencyGraphManifest.spec.ts
  • packages/framework-module/src/ModuleRegistry.ts
  • packages/framework-module/src/index.ts
  • packages/framework-module/src/tests/ModuleGraphManifest.spec.ts
  • packages/framework-module/src/types.ts
  • public-api-surface.snapshot.json

Comment thread packages/cli/src/commands/diCheck.ts
Comment thread packages/framework-context/src/libs/Container.ts
Comment thread packages/framework-context/src/libs/Container.ts Outdated
Comment thread packages/framework-module/src/ModuleRegistry.ts
@kang-heewon
kang-heewon merged commit e12e825 into trunk Jun 20, 2026
8 checks passed
@kang-heewon
kang-heewon deleted the fix/977-di-graph-check branch June 20, 2026 15:14
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.

[framework-context] DI graph manifest와 check 명령으로 provider 오류를 사전 검증한다

1 participant