Added route settings parser, serializer and validation#28990
Open
vershwal wants to merge 1 commit into
Open
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThis PR adds a new route settings parser module using zod for validation and js-yaml for serialization. It defines TypeScript types for routes (channel/template), collections, taxonomies, and data entries. Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
|
| Command | Status | Duration | Result |
|---|---|---|---|
nx run @tryghost/admin:build |
✅ Succeeded | 6s | View ↗ |
nx run ghost:build:assets |
✅ Succeeded | 2s | View ↗ |
nx run ghost:build:tsc |
✅ Succeeded | 6s | View ↗ |
💡 Verify your cache is correct by running tasks in a sandbox. Read docs ↗
☁️ Nx Cloud last updated this comment at 2026-07-01 12:02:53 UTC
![coderabbitai[bot]](https://cdn.statically.io/img/avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
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 `@ghost/core/core/server/services/route-settings/validate-route-settings.ts`:
- Around line 145-146: `validateRoute()` currently enforces leading/trailing
slashes but still allows deprecated `:slug` segments on route paths. Update the
route path validation flow in `validate-route-settings` to call
`rejectColonNotation()` for `route.path`, alongside the existing
`requireLeadingSlash()` and `requireTrailingSlash()` checks, so routes are
rejected consistently with collections and taxonomies.
- Around line 49-59: The shortform validation in validateRouteSettings currently
only checks a prefix via the entry match, which allows malformed route values to
slip through. Update the validation in validateRouteSettings so the pattern is
anchored to the full string while still allowing valid slug separators, and keep
the existing ValidationError path and message shape for invalid entries. Make
sure the check around entry and the subsequent resource extraction still work
correctly with the stricter full-string validation.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6a7a803f-57ae-4e68-98fe-cb20e36343ec
📒 Files selected for processing (4)
ghost/core/core/server/services/route-settings/route-settings-parser.tsghost/core/core/server/services/route-settings/validate-route-settings.tsghost/core/test/unit/server/services/route-settings/route-settings-parser.test.tsghost/core/test/unit/server/services/route-settings/validate-route-settings.test.ts
vershwal
force-pushed
the
princi-hkg-1826-route-settings-parser-serializer-validation
branch
from
375a138 to
d3ccf79
Compare
allouis
requested changes
Jul 1, 2026
vershwal
force-pushed
the
princi-hkg-1826-route-settings-parser-serializer-validation
branch
from
d3ccf79 to
461aed4
Compare
Contributor
There was a problem hiding this comment.
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 `@ghost/core/core/server/services/route-settings/route-settings-parser.ts`:
- Line 119: The RESERVED_DATA_KEYS list in route-settings-parser.ts is missing
browse-entry wrapper fields, so data: {fields: ...} and data: {page: ...} are
not rejected consistently. Update RESERVED_DATA_KEYS to include both fields and
page, and make the same change wherever that constant or equivalent reserved-key
check is duplicated so the parsing logic in the route-settings parser treats
them like filter, limit, and order.
- Around line 419-420: The RSS serialization logic in route-settings-parser
should only write the rss field when it is explicitly disabled, since rss
defaults to enabled. Update the channel/collection mapping code around the
existing rss checks in the relevant parser methods so that entry.rss is emitted
only when the source value is false, and omit it for undefined or true. Apply
the same change in both affected blocks in route-settings-parser to keep channel
routes and collections consistent.
- Line 319: The route settings parser is coercing any falsy root value into an
empty object, which hides invalid non-object YAML roots. Update the logic in
route-settings-parser’s parsing flow around the raw-to-obj assignment so only
null or undefined become an empty settings object, and preserve false, 0, and
empty string so the schema validation can reject them. Refer to the parsing code
that sets obj from raw in the route-settings-parser implementation.
- Around line 185-229: RouteObjectSchema currently accepts fields that
RouteObjectSchema.transform later drops for the wrong route type, so tighten
validation to reject unsupported fields instead of silently discarding them.
Update the schema/transform logic around RouteObjectSchema to make filter,
order, limit, and rss valid only for channel routes, and content_type valid only
for template routes, so invalid combinations fail validation before persistence.
Keep the existing type-specific handling in the channel/template branches, but
ensure the schema enforces those constraints up front.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f51ef84e-c33d-401a-aad0-600d077615a6
📒 Files selected for processing (3)
ghost/core/core/server/services/route-settings/route-settings-parser.tsghost/core/test/unit/server/services/route-settings/route-settings-parser.test.tsghost/core/test/unit/server/services/route-settings/validate-route-settings.test.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- ghost/core/test/unit/server/services/route-settings/validate-route-settings.test.ts
- ghost/core/test/unit/server/services/route-settings/route-settings-parser.test.ts
ref https://linear.app/ghost/issue/HKG-1826 - replaced hand-rolled parsing and validation with Zod schemas so raw YAML input is validated andtransformed in a single pass, eliminating unsafe `as` casts on the unknown input boundary - deleted validate-route-settings.ts; all validation rules now live in the Zod schemas inside route-settings-parser.ts - rewrote validate-route-settings tests to exercise parseRouteSettings directly with raw YAML-shaped objects - added roundtrip tests (model→yaml→model and yaml→model→yaml) to lock serializer fidelity
vershwal
force-pushed
the
princi-hkg-1826-route-settings-parser-serializer-validation
branch
from
461aed4 to
435c828
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ref https://linear.app/ghost/issue/HKG-1826
Why
The existing
validate.jstangles 5 concerns into a single pass — validation,{slug}to:slugconversion, data expansion to{query, router}, template normalization, and defaults. It also importsRESOURCE_CONFIGfrom the frontend, creating a cross-boundary dependency that blocks moving route settings to the store layer.This PR introduces a Zod-based parser/serializer that handles parsing, validation, and type transformation in a single pass — so the new store layer (PR #28821) can persist a clean domain model without pulling in frontend routing code.
What
route-settings-parser.ts— Zod schemas, parser, serializer & validationBuilds on the
RouteSettingstype model from PR #28821 (already onmain). Uses Zod schemas for runtime validation + parsing + transformation, eliminating unsafeascasts on the raw YAML input boundary.Parser —
parseRouteSettings(raw: unknown):RouteSettingsdomain model in one passpathproperty (YAML uses object keys)template(string or string[]) normalizes totemplates: string[]content_typemaps tocontentType(camelCase in domain model)RouteObjectSchemathat branches oncontroller— no union fallthrough that could silently reclassify malformed channel routes{slug}notation stays as-is, short-form data (tag.recipes) is not expanded — that is the activation bridge's job/about/:in YAML) produce friendly "Please define a template." errorsSerializer —
serializeRouteSettings(settings):/about/: about)rss: trueomitted (it is the default)contentTypemaps back tocontent_type(snake_case in YAML)Validation (via Zod schemas):
:slugcolon-notation rejection on routes, collections, and taxonomiestag/authortag/page/post/authorfor shortform,tags/posts/pages/authorsfor longform)slugrequired onreaddata entries viaz.discriminatedUnionauthorkey detected via recordsuperRefineTests
:slugrejection on routes/collections/taxonomies, taxonomy keys, data format, resource validation, reserved key names — with message assertions on key user-facing errors