fix: enforce spine entrypoint policy - #1216
Conversation
|
Warning Review limit reached
Next review available in: 5 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 selected for processing (6)
📝 WalkthroughWalkthrough스파인 패키지(1.0)의 엔트리포인트 정책이 도입되어, 로컬 root의 Changes스파인 엔트리포인트 정책 구현
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant CLI as normalize-packages CLI
participant Catalog as docs/package-catalog.json
participant Validator as validatePackage
participant SpinePolicy as validateSpineEntrypointPolicy
participant DistPolicy as validateDirectDistEntrypoints
participant Smoke as package-entrypoint-smoke
CLI->>Catalog: spine.packages 목록 읽기
Catalog-->>CLI: 스파인 패키지명 집합
CLI->>Validator: context(spinePackageNames)와 함께 검증 호출
Validator->>SpinePolicy: 스파인 root main/types == SRC_INDEX 검증
Validator->>DistPolicy: direct-dist root/publishConfig parity 검증
SpinePolicy-->>Validator: 위반 목록
DistPolicy-->>Validator: 위반 목록
Validator-->>CLI: 종합 위반 결과
Smoke->>Catalog: DIRECT_DIST_ENTRYPOINT_PACKAGES 조회
Smoke->>Smoke: fieldMatchesPath로 root/publishConfig 불일치 진단
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 |
📊 Benchmark Results✅ All benchmarks passed
Updated: 2026-07-05T14:35:35.710Z · Commit: ace090e |
20eb40a to
af50b2e
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af50b2e50f
ℹ️ 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".
154b0b0 to
d370fa0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d370fa051f
ℹ️ 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".
e18c6aa to
be5bf00
Compare
There was a problem hiding this comment.
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 `@packages/problems-core/src/libs/ProblemCategoryMapper.ts`:
- Around line 3-13: `ProblemClassConstructor` is a local constructor type that
should be aligned with the shared `Constructor<T>` utility pattern used by the
framework. Update `ProblemCategoryMapper` to reuse the existing generic
constructor type instead of defining a bespoke `ProblemClassConstructor`, while
preserving the current `require`-based workaround and the `ProblemModule` shape
so the mapping logic remains unchanged.
In `@scripts/normalize-packages.mjs`:
- Around line 928-938: `validateRootPublishFieldParity` is duplicating the same
dotted-path `publishConfig` lookup and `JSON.stringify` comparison logic already
used by `pushRootPublishFieldDiagnostic` in
`scripts/package-entrypoint-smoke.mts`. Extract that comparison algorithm into a
shared helper in `scripts/package-manifest-contracts.mjs`, then update both
`validateRootPublishFieldParity` and `pushRootPublishFieldDiagnostic` to import
and use it so the policy stays consistent and avoids drift.
🪄 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: a2be5f92-b900-48a7-9544-5dc8df07632f
⛔ Files ignored due to path filters (1)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**
📒 Files selected for processing (17)
.changeset/spine-entrypoint-policy.mddocs/problem-code-registry.jsondocs/release/croco-1.0-spine.mddocs/release/package-entrypoint-contract.mdpackages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.mdpackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/problems-core/package.jsonpackages/problems-core/src/libs/Problem.tspackages/problems-core/src/libs/ProblemCategoryMapper.tspackages/rpc-codegen/package.jsonpackages/telemetry-api/package.jsonscripts/normalize-packages.mjsscripts/package-entrypoint-smoke.mtsscripts/package-manifest-contracts.mjsscripts/static-misuse-raw-error-allowlist.jsonscripts/tests/normalize-packages.spec.tsscripts/tests/package-entrypoint-smoke.spec.ts
💤 Files with no reviewable changes (1)
- packages/problems-core/package.json
be5bf00 to
fb3e7cf
Compare
fb3e7cf to
1b7baa7
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)
scripts/normalize-packages.mjs (1)
604-611: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
validateSpineEntrypointPolicy의ENTRYPOINT_EXEMPTIONS체크는 도달 불가능한 코드입니다.
validatePackage는 Line 604-606에서ENTRYPOINT_EXEMPTIONS.has(pkg.name)이면 이미 조기 반환하므로,validateSpineEntrypointPolicy(Line 630에서 호출)에는 해당 패키지가 절대 도달하지 않습니다. Line 912의ENTRYPOINT_EXEMPTIONS.has(pkg.name) ||조건은 죽은 코드입니다.♻️ 제안: 죽은 조건 제거
- if (ENTRYPOINT_EXEMPTIONS.has(pkg.name) || DIRECT_DIST_ENTRYPOINT_PACKAGES.has(pkg.name)) { + if (DIRECT_DIST_ENTRYPOINT_PACKAGES.has(pkg.name)) { return; }Also applies to: 907-914
🤖 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/normalize-packages.mjs` around lines 604 - 611, Remove the unreachable ENTRYPOINT_EXEMPTIONS handling from validateSpineEntrypointPolicy and its caller path, since validatePackage already returns early for exempt packages. Update the logic around validatePackage, validateSpineEntrypointPolicy, and the ENTRYPOINT_EXEMPTIONS check near the package entrypoint validation so the exemption is handled only once and the dead condition is removed.
🤖 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 491-516: The direct-dist normalization logic is split across two
separate `if (options.directDistRoot)` blocks in `normalize-packages.mjs`; merge
them into a single conditional so all directDistRoot handling in the package
export/main/types/module setup lives together. Keep the existing
`pkg.publishConfig.exports`, `pkg.main`, `pkg.types`, `pkg.exports`, and
`rootImportTargetFor` logic intact, but move it into one
`options.directDistRoot` branch and leave the non-direct path unchanged.
- Around line 372-376: `readSpinePackageNames` currently returns an empty Set
when `CATALOG_METADATA_PATH` is missing, which lets `package-manifests:check`
skip spine validation entirely. Change this path in
`scripts/normalize-packages.mjs` so a missing `docs/package-catalog.json` is
recorded as a violation through the existing `violations` flow (or otherwise
fails the check explicitly) instead of silently succeeding, while keeping the
rest of the `readSpinePackageNames` logic unchanged.
In `@scripts/package-entrypoint-smoke.mts`:
- Around line 326-371: The diagnostic format in pushRootPublishFieldDiagnostic
is inconsistent with the normalize-packages validation messages. Update
directDistRootPublishFaceDiagnostics and/or pushRootPublishFieldDiagnostic so
the emitted text matches the normalize-packages.mjs convention used by
normalize-packages.spec.ts, specifically removing the extra “root ” wording
while keeping the packageName and field comparison context intact.
---
Outside diff comments:
In `@scripts/normalize-packages.mjs`:
- Around line 604-611: Remove the unreachable ENTRYPOINT_EXEMPTIONS handling
from validateSpineEntrypointPolicy and its caller path, since validatePackage
already returns early for exempt packages. Update the logic around
validatePackage, validateSpineEntrypointPolicy, and the ENTRYPOINT_EXEMPTIONS
check near the package entrypoint validation so the exemption is handled only
once and the dead condition is removed.
🪄 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: 90e0c996-3075-48f6-8e2b-041981a3e585
⛔ Files ignored due to path filters (1)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**
📒 Files selected for processing (17)
.changeset/spine-entrypoint-policy.mddocs/problem-code-registry.jsondocs/release/croco-1.0-spine.mddocs/release/package-entrypoint-contract.mdpackages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.mdpackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/problems-core/package.jsonpackages/problems-core/src/libs/Problem.tspackages/problems-core/src/libs/ProblemCategoryMapper.tspackages/rpc-codegen/package.jsonpackages/telemetry-api/package.jsonscripts/normalize-packages.mjsscripts/package-entrypoint-smoke.mtsscripts/package-manifest-contracts.mjsscripts/static-misuse-raw-error-allowlist.jsonscripts/tests/normalize-packages.spec.tsscripts/tests/package-entrypoint-smoke.spec.ts
💤 Files with no reviewable changes (1)
- packages/problems-core/package.json
37d643e to
9c8e639
Compare
2a8652c to
462c492
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@docs/release/package-entrypoint-contract.md`:
- Around line 56-61: Document the missing `exports["."].development`
normalization policy for direct-dist exception packages: in the
`package-entrypoint-contract` guidance around
`DIRECT_DIST_ENTRYPOINT_EXCEPTIONS`, explicitly state when the `development`
condition is allowed, what dist entrypoint requirements it must satisfy, and how
`check` versus `write` mode handles it. Reference the existing
`normalize-packages` behavior in `scripts/tests/normalize-packages.spec.ts` and
make the rule consistent with the root `main`, `types`, `exports`, and
`publishConfig` alignment requirements already described.
In `@scripts/normalize-packages.mjs`:
- Around line 519-544: The export-condition merge in
directDistPublishedRootExportFor can place types after import/require, which may
break TypeScript resolution. Update directDistPublishedRootExportFor so that
when it backfills from publishedRootExportFor(pkg), the resulting object
preserves types as the first condition (or otherwise guarantees it is ordered
before runtime conditions). Apply the same ordering rule in
publishedRootExportFor if it also builds the default exports["."] object, and
use the existing directDistPublishedRootExportFor and publishedRootExportFor
helpers as the key locations for the fix.
In `@scripts/package-manifest-contracts.mjs`:
- Around line 45-57: `fieldMatchesPath` is using `JSON.stringify` equality,
which makes the root/publish field parity check sensitive to object key order
and can create false mismatches for semantically identical manifests. Update the
comparison in `fieldMatchesPath` to use a structural deep-equality check that
ignores property order for objects while still handling arrays and primitives
correctly, and keep the existing path traversal behavior when reading
`publishFieldPath`.
🪄 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: f07c0e7e-fbcd-4cff-90b0-104abd41bc3c
⛔ Files ignored due to path filters (1)
packages/problems-core/src/generated/problem-code-registry.tsis excluded by!**/generated/**
📒 Files selected for processing (16)
.changeset/spine-entrypoint-policy.mddocs/problem-code-registry.jsondocs/release/croco-1.0-spine.mddocs/release/package-entrypoint-contract.mdpackages/docs/src/content/docs/api/problems-core/src/variables/CROCO_PROBLEM_CODE_REGISTRY.mdpackages/docs/src/content/docs/en/reference/problem-recovery-cookbook.mdpackages/problems-core/package.jsonpackages/problems-core/src/libs/Problem.tspackages/problems-core/src/libs/ProblemCategoryMapper.tspackages/rpc-codegen/package.jsonpackages/telemetry-api/package.jsonscripts/normalize-packages.mjsscripts/package-entrypoint-smoke.mtsscripts/package-manifest-contracts.mjsscripts/tests/normalize-packages.spec.tsscripts/tests/package-entrypoint-smoke.spec.ts
💤 Files with no reviewable changes (1)
- packages/problems-core/package.json
Addressed all actionable comments in follow-up commit 38eb752; CodeRabbit cannot re-run because prepaid credits are exhausted.
Fixes #1181.
Summary
@croco/rpc-codegenCommonJS require target.@croco/problems-core,@croco/rpc-codegen, and@croco/telemetry-apimanifests and includes a patch changeset for those packages.Verification
COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm exec vitest run scripts/tests/normalize-packages.spec.ts scripts/tests/package-entrypoint-smoke.spec.ts --config vitest.config.ts- passed, 30 tests.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm package-manifests:check- passed.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm --filter @croco/problems-core typecheck- passed.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm --filter @croco/rpc-codegen exec vitest run src/tests/e2e.spec.ts -t "generates client types from a decorated controller"- passed.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm --filter @croco/cli build- passed.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm --filter @croco/architecture-policy... build && COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm architecture-policy:check- passed.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm static-misuse:check- passed.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm check- passed.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm build- passed, 113 tasks.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm package-entrypoints:smoke- passed, 109 packages checked.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm typecheck- passed, 224 tasks.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm test- passed, 225 tasks.COREPACK_ENABLE_DOWNLOAD_PROMPT=0 corepack pnpm changeset-required:check -- --base origin/trunk --head HEAD- passed.test- passed, 225 tasks.typecheck- passed, 224 tasks.Self-review gates
Review and QA
@croco/rpc-codegenCommonJS preservation; those changes are included.@croco/problems-coresource-root/CommonJS runtime mismatch and static misuse allowlist drift; fixed with an explicit direct-dist exception and updated metadata.Notes
--no-verifyafter the successful hook evidence above.Summary by CodeRabbit
New Features
Bug Fixes
Documentation