Skip to content

fix: enforce events-inmemory runtime dependency contract - #789

Merged
kang-heewon merged 2 commits into
trunkfrom
fix/563-events-inmemory-otel-dependency
Jun 15, 2026
Merged

fix: enforce events-inmemory runtime dependency contract#789
kang-heewon merged 2 commits into
trunkfrom
fix/563-events-inmemory-otel-dependency

Conversation

@kang-heewon

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

Copy link
Copy Markdown
Member

Fixes #563.

Summary

@croco/events-inmemory now has issue-specific regression coverage for its direct OpenTelemetry runtime import contract. The fixture mirrors the in-memory event bus importing context, trace, and SpanStatusCode from @opentelemetry/api while only declaring @croco/telemetry-api, and proves the package manifest checker fails that transitive-dependency leakage.

The same coverage confirms declared runtime dependencies, runtime peer dependencies, optional dynamic imports, built-in modules, and type-only imports remain accepted, while runtime imports satisfied only by @types/*, dev-only dependencies, or no declaration fail manifest validation.

Verification

  • pnpm exec vitest run scripts/tests/normalize-packages.spec.ts --reporter verbose - passed, 11 tests.
  • pnpm package-manifests:check - passed, 97 checked / 2 private skipped.
  • pnpm changeset-required:check -- --base origin/trunk --head HEAD - passed, no publishable package behavior changes detected.
  • git diff --check origin/trunk...HEAD - passed.
  • pnpm check - passed.
  • pnpm --filter @croco/events-inmemory build - passed.
  • pnpm --filter @croco/events-inmemory typecheck - passed.
  • pnpm --filter @croco/events-inmemory test - passed, 35 tests.
  • pnpm package-entrypoints:smoke - passed, 96 public packages; @croco/events-inmemory resolved through CJS, ESM, and TypeScript consumers.
  • Pre-push hook passed after the latest push: auto-changeset found no publishable package changes, full cached pnpm test passed 197/197 tasks, and full cached pnpm typecheck passed 196/196 tasks.
  • GitHub Actions on head 14b61e6 passed: validate, benchmark, changes, and WIP. Docs checks were skipped by path filters.

Self-review

  • Correctness/regression: PASS. packages/events-inmemory/package.json declares @opentelemetry/api as ^1.9.0, package-entrypoint smoke proves a clean consumer can resolve @croco/events-inmemory, and the new manifest fixture proves the issue shape fails when OTel runtime values are only available transitively.
  • API/security/compatibility/release: PASS. This is test coverage only: no public API, package manifest, lockfile, export map, secret handling, or runtime behavior changed. The changeset gate confirms no publishable package behavior change.
  • Maintainability/minimality: PASS. The diff stays inside the existing package manifest regression suite, reuses the local fixture helper, and avoids duplicating the runtime import scanner already present on trunk after fix: enforce direct runtime dependency declarations #778. The pre-rebase all-import approach had a type-only/runtime edge; resolving against trunk kept the existing runtime classifier and narrowed this PR to [events-inmemory] Runtime OpenTelemetry API import is undeclared #563 evidence.

Risk

Low. The PR only strengthens regression coverage for an existing manifest validation path.

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

normalize-packages.spec.tspublishablePackage() 헬퍼 함수와 신규 테스트 케이스가 추가되었다. 신규 테스트는 --check 모드에서 프로덕션 소스 런타임 import를 기준으로 dependencies, peerDependencies, optionalDependencies 선언이 누락된 경우 실패 및 오류 메시지 출력 여부를 검증한다.

Changes

normalize-packages --check 모드 의존성 누락 감지 테스트

Layer / File(s) Summary
publishablePackage 헬퍼 및 --check 모드 테스트 케이스
scripts/tests/normalize-packages.spec.ts
publishablePackage()가 기본 publishable 패키지 매니페스트를 생성하고, 신규 테스트가 type-only/peer/optional/동적 import/Node built-in 등 다양한 시나리오에서 --check 모드의 의존성 선언 누락 감지 및 오류 메시지 출력을 검증한다.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • croco-dev/framework#634: normalize-packages.mjs 기반 package-manifests:check 게이트를 추가·강화하는 PR으로, 본 PR의 --check 모드 실패/오류 메시지 검증 테스트와 동일한 매니페스트 계약 강제 흐름에 연결된다.
🚥 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
Title check ✅ Passed PR 제목이 주요 변경사항인 @croco/events-inmemory 패키지의 런타임 의존성 계약 검증 테스트 추가와 직접적으로 관련되어 있습니다.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ 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/563-events-inmemory-otel-dependency

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 15, 2026

Copy link
Copy Markdown

📊 Benchmark Results

❌ Some benchmarks failed

Gate failures

  • CrocoApp constructor: p75 3.1μs exceeds baseline 2.5μs by more than 20%
  • CrocoApp lambdaHandler (10 controllers): p75 36.0μs exceeds baseline 30.0μs by more than 20%
  • Lambda cold-start simulation: p75 195.8μs exceeds baseline 60.0μs by more than 20%
  • Lambda cold-start with headers: p75 129.8μs exceeds baseline 60.0μs by more than 20%
  • Lambda cold-start with binary body: p75 115.0μs exceeds baseline 60.0μs by more than 20%
  • Lambda cold-start with query params: p75 103.6μs exceeds baseline 60.0μs by more than 20%
  • Lambda cold-start with authorizer context: p75 107.3μs exceeds baseline 60.0μs by more than 20%
  • Lambda cold-start realistic scenario: p75 107.2μs exceeds baseline 60.0μs by more than 20%
  • Container.get singleton (cold): p75 1.0μs exceeds baseline 0.7μs by more than 20%
  • Container.get singleton (warm): p75 0.4μs exceeds baseline 0.3μs by more than 20%
Benchmark p75 Threshold Baseline vs Baseline Status Notes
CrocoApp constructor 3.1μs 30.0ms 2.5μs +25.4% -
CrocoApp lambdaHandler (10 controllers) 36.0μs 50.0ms 30.0μs +20.1% -
Lambda cold-start simulation 195.8μs 80.0ms 60.0μs +226.4% -
Lambda cold-start with headers 129.8μs 80.0ms 60.0μs +116.3% -
Lambda cold-start with binary body 115.0μs 80.0ms 60.0μs +91.6% -
Lambda cold-start with query params 103.6μs 80.0ms 60.0μs +72.6% -
Lambda cold-start with authorizer context 107.3μs 80.0ms 60.0μs +78.9% -
Lambda cold-start realistic scenario 107.2μs 80.0ms 60.0μs +78.6% -
EventBusConfig.start (10 handlers) 1.3μs 10.0ms 9.0μs -85.7% -
EventPublisher.publishNow single event 1.6μs 2.0ms 1.5μs +4.2% -
DefaultHandlerResolver.resolve × 10 0.1μs 5.0ms 0.2μs -64.5% -
Container.get singleton (cold) 1.0μs 5.0ms 0.7μs +37.4% -
Container.register × 50 components 11.9μs 10.0ms 10.0μs +18.7% -
Container.validate (50 components) 29.4μs 20.0ms 25.0μs +17.7% -
Container.get singleton (warm) 0.4μs 500.0μs 0.3μs +40.3% -
TelemetryRuntime.init (lambda preset) 1.1ms 200.0ms 60.0ms -98.2% -
lambdaPreset config creation 1.5μs 2.0ms 2.0μs -25.8% -

Updated: 2026-06-15T06:55:02.277Z · Commit: cb0060b

@kang-heewon
kang-heewon force-pushed the fix/563-events-inmemory-otel-dependency branch from 84a6a1b to 7fc85b0 Compare June 15, 2026 06:37
coderabbitai[bot]
coderabbitai Bot previously requested changes Jun 15, 2026

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

🤖 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 `@scripts/tests/normalize-packages.spec.ts`:
- Around line 563-575: The peer-declared test case uses only import type syntax
in the sourceContent for the package at line 573, which does not create a
runtime import path. To properly test runtime peer dependency validation, change
the import type statement to a regular import statement (without the type
keyword) so that the test actually exercises the runtime import scenario that
the peerDependencies declaration should allow.
🪄 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: 28e67c14-c8ef-4829-a4b9-202ad0b13ccc

📥 Commits

Reviewing files that changed from the base of the PR and between 40cb9f1 and 7fc85b0.

📒 Files selected for processing (1)
  • scripts/tests/normalize-packages.spec.ts

Comment thread scripts/tests/normalize-packages.spec.ts
@kang-heewon
kang-heewon merged commit 2f7d887 into trunk Jun 15, 2026
8 checks passed
@kang-heewon
kang-heewon deleted the fix/563-events-inmemory-otel-dependency branch June 15, 2026 08:47
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.

[events-inmemory] Runtime OpenTelemetry API import is undeclared

1 participant