fix: harden package entrypoint and validation gates - #634
Conversation
|
Warning Review limit reached
More reviews will be available in 19 minutes and 49 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 We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (26)
📝 WalkthroughWalkthrough
Changes패키지 엔트리포인트 계약 표준화
Sequence Diagram(s)sequenceDiagram
rect rgba(173, 216, 230, 0.5)
Note over CI,normalize-packages.mjs: CI Validate / Release 파이프라인
end
participant CI as CI/Release Workflow
participant normalize-packages.mjs as normalize-packages.mjs
participant smoke as package-entrypoint-smoke.mts
participant tsc as tsc (NodeNext)
participant node as node runtime
CI->>normalize-packages.mjs: pnpm package-manifests:check (--check)
normalize-packages.mjs-->>CI: 위반 목록 출력 또는 OK
CI->>CI: pnpm build
CI->>smoke: pnpm package-entrypoints:smoke
smoke->>smoke: 임시 node_modules 구성 + dist symlink
smoke->>node: cjs.cjs 실행 (Container/Context 확인)
smoke->>node: esm.mjs 실행 (Container/Context 확인)
smoke->>tsc: types.ts 타입 컴파일 (noEmit)
tsc-->>smoke: 성공 / 실패
smoke-->>CI: 결과 반환
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
e54f752 to
4112be8
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/tests/normalize-packages.spec.ts (1)
1-166:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTS 문자열 리터럴 규칙(싱글 쿼트) 위반이 파일 전반에 있습니다.
Line 1-6 import 구문과 테스트/헬퍼 문자열들 전반을 싱글 쿼트로 통일해 주세요.
As per coding guidelines, 'Use single quotes for string literals'.🤖 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 `@scripts/tests/normalize-packages.spec.ts` around lines 1 - 166, Replace all double quotes with single quotes throughout the entire file to comply with TypeScript string literal conventions. This includes all import statements (lines 1-6), all test description strings and expect assertion messages in the describe and it blocks, object property values, file paths in function calls like writePackage and runScript, and all other string literals throughout the test file and helper functions.Source: Coding guidelines
🤖 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/normalize-packages.mjs`:
- Around line 345-347: The hardcoded POSIX path comparison in the condition
checking relativePath === "packages/impersonation-core/package.json" fails on
Windows because path.relative() returns backslash separators on that platform,
causing the condition to evaluate to false and skip the validation for the
impersonation-core package. Replace this path-based string comparison with a
package name-based comparison by checking if pkg.name equals
"impersonation-core" instead, which will work consistently across all platforms
regardless of path separator differences.
In `@scripts/package-entrypoint-smoke.mts`:
- Around line 99-110: The spawnSync call starting at line 99 lacks a timeout
setting which can cause the Node.js event loop to block indefinitely if a child
process hangs. Additionally, the error handling block at lines 105-110 only
checks result.status but ignores result.error, which means spawn execution
failures like missing executable or permission errors are not captured. Add a
timeout option (in milliseconds) to the spawnSync options object, and modify the
error handling to check if result.error exists and include it in the thrown
Error message along with the existing stdout and stderr information.
---
Outside diff comments:
In `@scripts/tests/normalize-packages.spec.ts`:
- Around line 1-166: Replace all double quotes with single quotes throughout the
entire file to comply with TypeScript string literal conventions. This includes
all import statements (lines 1-6), all test description strings and expect
assertion messages in the describe and it blocks, object property values, file
paths in function calls like writePackage and runScript, and all other string
literals throughout the test file and helper functions.
🪄 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: 1b8c6365-a110-4db7-8651-557af28ad849
📒 Files selected for processing (18)
.changeset/package-entrypoint-contracts.md.github/workflows/ci.yml.github/workflows/release.ymldocs/release/package-entrypoint-contract.mdpackage.jsonpackages/access-drizzle/package.jsonpackages/auth-drizzle/package.jsonpackages/cli/package.jsonpackages/cli/tsup.config.tspackages/impersonation-core/package.jsonpackages/llm-core/package.jsonpackages/llm-metering/package.jsonpackages/search-core/package.jsonpackages/search-drizzle/package.jsonpackages/search-meilisearch/package.jsonscripts/normalize-packages.mjsscripts/package-entrypoint-smoke.mtsscripts/tests/normalize-packages.spec.ts
📊 Benchmark Results❌ Some benchmarks failed
Updated: 2026-06-14T07:18:53.735Z · Commit: 3c1c906 |
4112be8 to
a38b199
Compare
Addressed in a38b199: the path comparison now uses package name, spawnSync has timeout/error reporting, and the normalize package test uses single quotes with formatter hook/check alignment.
a38b199 to
2f92b43
Compare
Summary
Public package publish manifests now have a checked entrypoint contract, and the validation path now ignores generated formatter targets so release and CI gates fail on actionable source drift instead of generated artifacts.
Fixes #618
변경 사항
publishConfigis the authoritative dist-based npm contract.pnpm package-manifests:checknow fails manifest drift, andpnpm package-manifests:writepreserves package versions while normalizing the contract.oxfmtchecks through the shared formatter ignore path.oxfmthook now uses the same ignore paths aspnpm checkandpnpm format.package-entrypoint-smokenow bounds child process execution and reports spawn errors explicitly.Container.remove()now unregisters constructor component metadata so validation and later resolution honor removed components.Verification
pnpm install --frozen-lockfilepnpm checkpnpm exec vitest run scripts/tests/normalize-packages.spec.tspnpm --filter @croco/framework-context testpnpm --filter @croco/framework-context typecheckpnpm --filter @croco/rpc-codegen testpnpm buildpnpm typecheckpnpm package-entrypoints:smokepnpm testpnpm packmanifest check for@croco/framework-context