Repository navigation
Migrate spo sitescript commands to Zod. Closes #7342 - #7627
milanholemans merged 4 commits into
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The migrations preserve command behavior and include appropriate schema validation coverage.
0 open findings
What changed in this PR
Migrates five spo sitescript commands from legacy option initialization and validation to Zod schemas.
Changes:
- Adds strict Zod schemas with aliases and validation.
- Updates tests to parse options through command schemas.
- Adds unknown-option validation coverage.
| File | Description |
|---|---|
sitescript-add.ts |
Adds schema-based JSON validation. |
sitescript-add.spec.ts |
Updates schema and action tests. |
sitescript-get.ts |
Adds GUID and flag schema. |
sitescript-get.spec.ts |
Tests migrated validation. |
sitescript-list.ts |
Adds strict global-options schema. |
sitescript-list.spec.ts |
Tests empty and unknown options. |
sitescript-remove.ts |
Adds GUID and force schema. |
sitescript-remove.spec.ts |
Updates validation and action tests. |
sitescript-set.ts |
Adds update-option schemas and refinements. |
sitescript-set.spec.ts |
Covers migrated validation paths. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
milanholemans
left a comment
milanholemans
left a comment
There was a problem hiding this comment.
Few remarks while reviewing. Also I think the PR has to be rebased to the latest version of Zod to fix the code coverage issue (other PRs probably as well).
6092f94 to
0d59240
Compare
milanholemans
left a comment
milanholemans
left a comment
There was a problem hiding this comment.
With a few changes, it's good to go.
|
|
||
| export const options = z.strictObject({ | ||
| ...globalOptionsZod.shape, | ||
| id: z.uuid().alias('i'), |
There was a problem hiding this comment.
Instead of uuid, we should use guid. The UUID check is stricter than GUID:
- The first character of the 3rd group must be 1–8.
- The first character of the 4th group must be 8, 9, a or b.
|
|
||
| export const options = z.strictObject({ | ||
| ...globalOptionsZod.shape, | ||
| id: z.uuid().alias('i'), |
There was a problem hiding this comment.
Also here, let's use guid.
|
|
||
| export const options = z.strictObject({ | ||
| ...globalOptionsZod.shape, | ||
| id: z.uuid().alias('i'), |
| id: z.uuid().alias('i'), | ||
| title: z.string().optional().alias('t'), | ||
| description: z.string().optional().alias('d'), | ||
| version: z.string().optional() |
There was a problem hiding this comment.
If you move the .optional() to the back, you don't have to perform an undefined check.
Closes #7342