fix: validate presentation preset runtime profiles - #1014
Conversation
|
Warning Review limit reached
More reviews will be available in 33 minutes and 47 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 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 configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (18)
📝 WalkthroughWalkthrough
Changespresentation-preset 런타임 프로필 계약 및 카탈로그 검증
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 |
📊 Benchmark Results❌ Some benchmarks failed Gate failures
Updated: 2026-06-19T19:04:24.514Z · Commit: 142868d |
60a8b82 to
42613a4
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/presentation-preset/src/output-contract-validator.ts (1)
287-299:⚠️ Potential issue | 🟠 Major | ⚡ Quick win교차 검증에서
nullartifact/entry를 만나면 런타임 예외가 발생합니다.Line 287의
artifact.path와 Line 291~298의entry.*접근이 레코드 검증 없이 실행됩니다. 앞단에서 shape 에러를 수집해도 여기서 TypeError로 중단될 수 있습니다.제안 수정안
- const artifactPaths = new Set(contract.artifacts.map((artifact) => artifact.path)); + const artifactPaths = new Set<string>(); + for (const artifact of contract.artifacts) { + if (isRecord(artifact) && isNonEmptyString(artifact.path)) { + artifactPaths.add(artifact.path); + } + } const referencedPaths = new Set<string>(); for (const entry of contract.entries) { + if (!isRecord(entry)) { + continue; + } if (isNonEmptyString(entry.main)) { referencedPaths.add(entry.main); }🤖 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 `@packages/presentation-preset/src/output-contract-validator.ts` around lines 287 - 299, The code in output-contract-validator.ts directly accesses artifact.path and entry properties (main, cjs, types) without null or undefined checks, which can cause runtime TypeErrors during cross-validation. Add defensive guards or optional chaining operators before accessing artifact.path in the artifactPaths Set construction and before accessing entry.main, entry.cjs, and entry.types in the referencedPaths Set construction loop. This ensures the validator gracefully handles cases where these objects or properties might be null or undefined rather than throwing TypeErrors.
🤖 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/presentation-preset/src/__tests__/output-contract-validator.spec.ts`:
- Around line 124-324: Add regression test cases to the "Generated runtime
profile catalog" describe block to ensure the OutputContractValidator handles
non-object and null inputs gracefully without throwing exceptions. Specifically,
add test cases that verify when validateGeneratedRuntimeProfile is called with
null values mixed into the profiles array, or null values in the entries or
artifacts arrays, the report.passed should be false and appropriate error
messages should be present in the results. These tests should verify the
validator returns a failed report rather than throwing an exception, ensuring
stability when handling invalid inputs.
In `@packages/presentation-preset/src/output-contract-validator.ts`:
- Around line 29-31: The constants artifactFormats, artifactTypes, and
presentationRuntimes on lines 29-31 are using camelCase naming convention but
should follow SCREAMING_SNAKE_CASE as per the coding guidelines for constant
names. Rename these three constants to ARTIFACT_FORMATS, ARTIFACT_TYPES, and
PRESENTATION_RUNTIMES respectively, and update all references to these constants
throughout the file. Additionally, check lines 469-479 for similar naming
convention violations and apply the same SCREAMING_SNAKE_CASE fix to any other
constants found in that range.
- Around line 89-115: The validator may crash with a TypeError if the profiles
array contains null or primitive values instead of valid objects. When iterating
through profiles in the loop, add a guard check before accessing profile.name
and profile.runtime to ensure the profile is a valid object. Specifically,
before the checks at the profile.name conditional and the
isPresentationRuntime() call, verify that profile is actually an object (not
null or a primitive value) and skip processing that element or push an
appropriate error result if it is invalid.
In `@packages/presentation-preset/tsup.config.ts`:
- Around line 5-16: The constant runtimeProfilesOutputPath on line 5 does not
follow the SCREAMING_SNAKE_CASE naming convention required for constant values.
Rename runtimeProfilesOutputPath to RUNTIME_PROFILES_OUTPUT_PATH and update all
references to this constant throughout the file, specifically in the
copyFileSync call within the onSuccess callback where it is currently used.
In `@scripts/package-docs-check.mts`:
- Around line 924-949: Add validation to check that if target.runtime is
defined, it must be an object. Currently, the code only validates internal
fields when target.runtime is already a record, but it doesn't catch cases where
target.runtime is defined but not a record (e.g., string, number). Before the
existing isRecord(target.runtime) check, add a validation block that calls
addRuntimeProfileViolation if target.runtime is defined but not a record, using
a message like "Generated runtime profile '${profileName}' runtime must be an
object when provided". This ensures consistency with the upstream
OutputContractValidator behavior.
In `@scripts/package-entrypoint-smoke.mts`:
- Around line 540-542: The JSON skip condition for types (checking if condition
equals "types" and isJsonTargetPath returns true) is currently only applied to
the string value case. When value is an object with a types property, the target
is extracted around lines 555-560 but the same JSON skip check is not applied to
the extracted target. Add the same condition check (condition === "types" &&
isJsonTargetPath) after extracting the target from the object to ensure JSON
paths are skipped for types targets in both the string and object cases.
---
Outside diff comments:
In `@packages/presentation-preset/src/output-contract-validator.ts`:
- Around line 287-299: The code in output-contract-validator.ts directly
accesses artifact.path and entry properties (main, cjs, types) without null or
undefined checks, which can cause runtime TypeErrors during cross-validation.
Add defensive guards or optional chaining operators before accessing
artifact.path in the artifactPaths Set construction and before accessing
entry.main, entry.cjs, and entry.types in the referencedPaths Set construction
loop. This ensures the validator gracefully handles cases where these objects or
properties might be null or undefined rather than throwing TypeErrors.
🪄 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: d3888092-047a-4cb8-b424-425f36f706b9
📒 Files selected for processing (18)
.changeset/presentation-preset-runtime-profiles.mdREADME.mddocs/package-docs-baseline.jsondocs/package-docs-report.mdpackages/docs/src/content/docs/en/reference/presentation-runtime-support.mdpackages/presentation-preset/README.mdpackages/presentation-preset/package.jsonpackages/presentation-preset/runtime-profiles.jsonpackages/presentation-preset/src/__tests__/output-contract-validator.spec.tspackages/presentation-preset/src/index.tspackages/presentation-preset/src/output-contract-validator.tspackages/presentation-preset/src/output-contract.tspackages/presentation-preset/tsup.config.tspackages/presentation-preset/vitest.config.tspublic-api-surface.snapshot.jsonscripts/package-docs-check.mtsscripts/package-entrypoint-smoke.mtsscripts/tests/package-docs-check.spec.ts
💤 Files with no reviewable changes (1)
- docs/package-docs-baseline.json
42613a4 to
bec2758
Compare
Fixes #949.
Summary
@croco/presentation-presetnow has a generated runtime profile catalog backing the package catalog claims fornode,lambda,cloudflare-workers, andbrowser.docs:catalog:checknow fails whendocs/package-catalog.jsonclaims a presentation-preset runtime without generated profile evidence../runtime-profiles.json, copied into the publisheddist, reflected in README/docs/catalog outputs, and released with a patch changeset.Verification
pnpm --filter @croco/presentation-preset test- passed, 30 tests.pnpm --filter @croco/presentation-preset typecheck- passed.pnpm --filter @croco/presentation-preset build- passed.pnpm --filter @croco/presentation-preset pack --pack-destination /tmp/croco-presentation-pack-check- passed; tarball containsdist/runtime-profiles.json.pnpm package-entrypoints:smoke- passed; CJS and ESM consumers resolved@croco/presentation-preset/runtime-profiles.json.pnpm exec vitest run scripts/tests/package-docs-check.spec.ts- passed, 11 tests.pnpm docs:catalog:check- passed.pnpm check- passed.pnpm create-croco-app:smoke- passed; all generated app smoke cases passed on the rebased commit.pnpm changeset-required:check -- --base origin/trunk --head HEAD- passed.git diff --check HEAD- passed.oxlintandoxfmtwhere applicable.pnpm testpassed with 209/209 Turbo tasks, and fullpnpm typecheckpassed with 208/208 Turbo tasks.Self-review gates
Independent review
An independent review found two actionable issues before PR creation: the published package would not include the runtime profile JSON, and the package validator was less strict than the docs catalog gate for malformed target metadata. The final patch copies
runtime-profiles.jsonintodist, exposes./runtime-profiles.json, verifies the tarball contents, hardens package validation, and adds regression coverage for malformed env/runtime metadata.Risk
Low-medium. This introduces stricter runtime-claim validation for presentation-preset catalog metadata and extends the repository entrypoint smoke harness for JSON subpath exports. Existing generated app flows continue to pass, while unsupported or unevidenced runtime claims now fail deterministically.
Summary by CodeRabbit
릴리스 노트
새로운 기능
문서
테스트