fix: CLI input validation with Zod schemas (#60)#68
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR enhances CLI input validation across five command modules by replacing TypeScript interface-based typing with runtime Zod schema validation. Early validation failures now print formatted error messages and exit with status code 1. Additional validations include enforcing numeric ranges for Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 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)
Comment |
Manual Test Results ✅All 3 manual test cases passed. Automated tests also pass (427/427). 1.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
src/cli/commands/implement.ts (1)
113-117: 🧹 Nitpick | 🔵 TrivialDuplicate
--image-scalevalidation is now unreachable.This validation block at lines 113-117 duplicates the early validation at lines 44-51. Since the early check exits on invalid values, this code path can never be reached with an invalid
imageScale. Consider removing this redundant check.🧹 Remove redundant validation
if (figmaToken) { const imgScale = options.imageScale !== undefined ? Number(options.imageScale) : 2; - if (!Number.isFinite(imgScale) || imgScale < 1 || imgScale > 4) { - console.error("Error: --image-scale must be 1-4 (2 for PC, 3 for mobile)"); - process.exit(1); - } const { FigmaClient } = await import("../../core/adapters/figma-client.js");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/commands/implement.ts` around lines 113 - 117, Remove the redundant image-scale validation block that re-parses options.imageScale into imgScale and checks Number.isFinite/imgScale range; this is duplicative of the earlier validation and unreachable after the initial check, so delete the secondary check (the code using imgScale and calling console.error/process.exit) and keep only the first validation that validates options.imageScale.src/cli/commands/save-fixture.ts (1)
143-148: 🧹 Nitpick | 🔵 TrivialDuplicate
--image-scalevalidation is now unreachable.This validation at lines 143-148 duplicates the early validation at lines 47-54. Since the early check exits on invalid values, this code path can never be reached with an invalid
imageScale. Consider removing this redundant check.🧹 Remove redundant validation
const imageNodes = collectImageNodes(file.document); if (imageNodes.length > 0) { const imgScale = options.imageScale !== undefined ? Number(options.imageScale) : 2; - if (!Number.isFinite(imgScale) || imgScale < 1 || imgScale > 4) { - console.error("Error: --image-scale must be 1-4 (2 for PC, 3 for mobile)"); - process.exit(1); - } const imageDir = resolve(fixtureDir, "images");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/commands/save-fixture.ts` around lines 143 - 148, Remove the redundant image-scale validation inside the block that checks imageNodes (the imgScale check using options.imageScale), since the same validation already runs earlier and exits on invalid values; simply delete the inner validation and its process.exit path in the save-fixture logic so the imageNodes branch uses the already-validated imgScale value (references: imageNodes, imgScale, options.imageScale).src/cli/commands/init.ts (1)
7-22: 🧹 Nitpick | 🔵 TrivialInconsistent validation pattern:
initcommand lacks Zod schema unlike other commands in this PR.The other commands (
analyze,implement,save-fixture,visual-compare) define a Zod schema and usesafeParsefor runtime validation. This command uses a plain TypeScript interface with a manual guard. For consistency and compliance with coding guidelines requiring Zod validation for external inputs, consider aligning this command with the established pattern.♻️ Suggested refactor to use Zod schema
+import { z } from "zod"; + -interface InitOptions { - token?: string; - mcp?: boolean; -} +const InitOptionsSchema = z.object({ + token: z.string().optional(), + mcp: z.boolean().optional(), +}).refine( + (opts) => !(opts.token && opts.mcp), + { message: "--token and --mcp are mutually exclusive. Choose one." } +); + +type InitOptions = z.infer<typeof InitOptionsSchema>;Then in the action handler:
- .action((options: InitOptions) => { + .action((rawOptions: Record<string, unknown>) => { try { - if (options.token && options.mcp) { - console.error("Error: --token and --mcp are mutually exclusive. Choose one."); - process.exit(1); - } + const parseResult = InitOptionsSchema.safeParse(rawOptions); + if (!parseResult.success) { + const msg = parseResult.error.issues.map(i => i.message).join("\n"); + console.error(`\nInvalid options:\n${msg}`); + process.exit(1); + } + const options = parseResult.data;Based on learnings: "Validate all external inputs with Zod schemas" and "Infer TypeScript types from Zod schemas using
z.infer<typeof Schema>".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/cli/commands/init.ts` around lines 7 - 22, Replace the manual TypeScript interface and ad-hoc guard in registerInit with a Zod schema: define an InitSchema using z.object({ token: z.string().optional(), mcp: z.boolean().optional() }), infer the input type with z.infer<typeof InitSchema>, and in the .action handler call InitSchema.safeParse(options); if safeParse fails, print the validation error and exit; if it succeeds, use the parsed data and enforce the mutual-exclusion check (token && mcp) thereafter. Ensure you import z from "zod" and update references to InitOptions to use the inferred type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/cli/commands/visual-compare.ts`:
- Around line 8-15: VisualCompareOptionsSchema currently uses z.unknown() for
width and height which sacrifices validation; update the schema for the width
and height fields in VisualCompareOptionsSchema to accept explicit CLI-friendly
types (e.g., a union of string and number) so the parser can validate and coerce
values correctly; reference the VisualCompareOptionsSchema and the width and
height fields when making the change and consider using z.union([z.string(),
z.number()]) or a preprocess step if you want to coerce strings to numbers
before validation.
---
Outside diff comments:
In `@src/cli/commands/implement.ts`:
- Around line 113-117: Remove the redundant image-scale validation block that
re-parses options.imageScale into imgScale and checks Number.isFinite/imgScale
range; this is duplicative of the earlier validation and unreachable after the
initial check, so delete the secondary check (the code using imgScale and
calling console.error/process.exit) and keep only the first validation that
validates options.imageScale.
In `@src/cli/commands/init.ts`:
- Around line 7-22: Replace the manual TypeScript interface and ad-hoc guard in
registerInit with a Zod schema: define an InitSchema using z.object({ token:
z.string().optional(), mcp: z.boolean().optional() }), infer the input type with
z.infer<typeof InitSchema>, and in the .action handler call
InitSchema.safeParse(options); if safeParse fails, print the validation error
and exit; if it succeeds, use the parsed data and enforce the mutual-exclusion
check (token && mcp) thereafter. Ensure you import z from "zod" and update
references to InitOptions to use the inferred type.
In `@src/cli/commands/save-fixture.ts`:
- Around line 143-148: Remove the redundant image-scale validation inside the
block that checks imageNodes (the imgScale check using options.imageScale),
since the same validation already runs earlier and exits on invalid values;
simply delete the inner validation and its process.exit path in the save-fixture
logic so the imageNodes branch uses the already-validated imgScale value
(references: imageNodes, imgScale, options.imageScale).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: d9c48dfe-d669-492e-95e8-c0958b98e9cc
📒 Files selected for processing (5)
src/cli/commands/analyze.tssrc/cli/commands/implement.tssrc/cli/commands/init.tssrc/cli/commands/save-fixture.tssrc/cli/commands/visual-compare.ts
- analyze: validate --preset with Zod schema (rejects invalid values early) - save-fixture: move --image-scale validation before any file I/O - visual-compare: warn when --figma-url has no node-id - implement: warn for unscoped Figma URL + early --image-scale validation - init: reject --token and --mcp when both provided (mutual exclusivity) - All 4 public commands now use Zod safeParse for runtime option validation Closes #60 https://claude.ai/code/session_018Y1Y4GuLuyeUEp5vHnuUKu
All 3 manual test cases passed: - --preset invalid → Zod validation error - --token x --mcp → mutual exclusivity error - --image-scale abc → early validation error before file I/O https://claude.ai/code/session_0113aX578Sq8Q4RQeho8jsuV
- visual-compare: z.unknown() → z.union([z.string(), z.number()]) for width/height - implement: remove redundant --image-scale validation (unreachable after early check) - save-fixture: remove redundant --image-scale validation (unreachable after early check) - init: convert to Zod schema with .refine() for --token/--mcp mutual exclusivity https://claude.ai/code/session_018Y1Y4GuLuyeUEp5vHnuUKu
let-sunny
force-pushed
the
claude/v1-release-prep-HyMNQ
branch
from
f3b3285 to
6dbbb53
Compare
2 participants
Summary
--presetvalidated via Zod (relaxed|dev-friendly|ai-ready|strict). Invalid values now fail immediately with clear error.--image-scalevalidated beforedata.jsonsave — prevents incomplete fixture directories on bad input.--figma-urlhas nonode-id(results may be inaccurate for full files).--image-scalevalidation.--token+--mcptogether now rejected with explicit error (was silently ignoring--mcp).All 4 public commands (
analyze,save-fixture,visual-compare,implement) now useZod.safeParse()at handler entry for runtime option validation, per CLAUDE.md convention: "Validate all external inputs with Zod schemas".Test plan
pnpm lintpassespnpm test:runpasses (427/427)canicode analyze ./fixtures/material3-kit --preset invalid→ clear errorcanicode init --token x --mcp→ mutual exclusivity errorcanicode save-fixture <url> --image-scale abc→ error before any file creationCloses #60
https://claude.ai/code/session_018Y1Y4GuLuyeUEp5vHnuUKu
Summary by CodeRabbit
Bug Fixes
--image-scaleparameter to enforce valid range (1-4).--tokenand--mcpflags in theinitcommand.New Features
node-idparameter inimplementandvisual-comparecommands.