Skip to content

feat(core): validation redesign with coercion pipeline and API renames#56

Open
aryasaatvik wants to merge 8 commits into
mainfrom
feat/validation-redesign
Open

feat(core): validation redesign with coercion pipeline and API renames#56
aryasaatvik wants to merge 8 commits into
mainfrom
feat/validation-redesign

Conversation

@aryasaatvik
Copy link
Copy Markdown
Member

@aryasaatvik aryasaatvik commented Mar 27, 2026

Summary

  • New coercion system: String CLI arguments are now automatically coerced to their target types (boolean, number, Date) before validation
  • New fs-validators: file() and directory() combinators for path validation with mustExist and extensions options
  • New AggregateValidationError: Collects multiple validation issues with hints and typo suggestions
  • API renames (breaking):
    • option()defineOption()
    • createPlugin()definePlugin()

Breaking changes

  • option() renamed to defineOption()
  • createPlugin() renamed to definePlugin()

Test plan

  • bun test passes for all new tests
  • Coercion tests cover boolean, number, date types
  • fs-validator tests cover mustExist and extension constraints

Open with Devin

Add coercion system that transforms string CLI arguments to their target
types (boolean, number, Date) before validation. The pipeline tries each
coercion strategy in order and validates the result against the schema.

New files:
- coerce.ts: coerceValue(), coerceArray() for string→type coercion
- fs-validators.ts: file() and directory() combinators for path validation
- schema-helpers.ts: extractSchemaType(), generateHint() utilities

Enhanced:
- validateValue() now applies coercion before validation
- validateValues() returns structured ValidationIssue[] instead of strings
- parser tracks unknown flags for typo suggestions
- AggregateValidationError collects multiple issues with hints
Add comprehensive tests for the new value coercion pipeline and
fs-validators. Tests cover:
- Boolean coercion (true/false/yes/no/1/0)
- Number coercion (integers, floats, negative, scientific notation)
- Date coercion (ISO strings)
- Number schema constraints (min, max)
- File validator (mustExist, extensions)
- Directory validator (mustExist)
Rename the public API to use more descriptive verb prefixes:
- `option()` → `defineOption()`
- `createPlugin()` → `definePlugin()`
- `createTestPlugin()` → `defineTestPlugin()`

This affects all packages (core, cli, plugins), examples, templates,
docs, cursor rules, and skill references.

BREAKING CHANGE: These are public API renames. Update imports and usage.
Remaining prompt command files that were missed in the earlier rename.
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Mar 27, 2026

⚠️ No Changeset found

Latest commit: 8e4d727

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c1ee0760ef

ℹ️ 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".

Comment on lines +89 to +90
if (!name || !options[name]) {
continue
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Report unknown short flags during strict parsing

In strict mode, unknown long flags are accumulated into issues, but unknown short flags are silently dropped here, so a typo like -v (when only -V exists) is ignored and the command executes with defaults instead of failing with an AggregateValidationError. This makes strict validation inconsistent and defeats the typo-protection the redesign adds for unknown options.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8e4d727. Unknown short flags now push to unknownFlags (line 90), matching the long-flag behavior. In strict mode, -v will now produce an AggregateValidationError with a typo suggestion.

devin-ai-integration[bot]

This comment was marked as resolved.

… unknownFlags

- CoercionResult now carries an `issues` field with the best constraint-
  violation error from the type-matched coercion step, so callers see
  "Number must be <= 65535" instead of "Expected number, received string"
- Gate Date coercion behind a DATE_LIKE regex to prevent numeric strings
  (e.g. "70000") from being misinterpreted as dates and overwriting the
  real constraint error from the Number step
- Parser uses coercion-provided issues directly instead of re-validating
  the raw string value
- Return the actual collected unknownFlags array instead of hardcoded []
- Add tests for constraint-error surfacing, unknownFlags in lenient mode
devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: ced3ac7335

ℹ️ 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".

Comment thread packages/core/src/parser.ts Outdated
hint: generateHint(option.schema, values)
})
} else {
flags[name] = values
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve validated repeatable option output

Use the validated result.value in this fallback branch instead of the original values. coerceArray() can succeed on the raw-array strategy while still returning transformed output from the schema (for example element .trim()/.transform() or array-level transforms), but this assignment drops that output and passes unnormalized raw CLI strings to handlers. That is a behavioral regression from the previous validation path and can break commands that rely on transformed repeatable option values.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in 8e4d727. The fallback branch now uses result.value (schema-validated output) instead of raw values, preserving transforms like .trim() on repeatable options.

devin-ai-integration[bot]

This comment was marked as resolved.

…orms, validateValue coercion

- Track unknown short flags in unknownFlags array (was silently skipping)
  so strict mode catches typos like -v when only -V exists
- Use result.value instead of raw values in repeatable flag fallback
  branch to preserve schema transforms (e.g. .trim())
- Use coercion pipeline's constraint errors in validateValue instead of
  falling through to raw validation with misleading type-mismatch errors
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8e4d7279fc

ℹ️ 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".

hint: generateHint(option.schema, rawValue)
})
} else {
flags[name] = rawValue
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Use validated fallback value for non-repeatable flags

When coerceValue() succeeds without coercion (coerced: false, no issues), this branch stores rawValue instead of result.value. That drops schema-produced outputs such as defaults/transforms for explicitly provided flags and can violate runtime type guarantees. For example, with defineOption(z.number().default(3000)), passing --port yields parsed.flags.port === undefined here, and the later “missing option” pass is skipped because the option was already marked as provided.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 new potential issue.

View 11 additional findings in Devin Review.

Open in Devin Review

@@ -30,7 +30,7 @@ This is a monorepo for the Bunli CLI framework, organized into apps, packages, a
## Key Entry Points

- **[packages/core/src/index.ts](mdc:packages/core/src/index.ts)** - Core framework exports (defineCommand, createCLI, option)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Incomplete rename: .cursor/rules/bunli-project-structure.mdc still references option instead of defineOption

Line 32 still says Core framework exports (defineCommand, createCLI, option) but option was renamed to defineOption throughout this PR. The adjacent line 33 was correctly updated from createPlugin to definePlugin, showing this was an oversight. This .mdc file is a special rule file used by agents to understand the codebase, so the stale reference could cause agents to use the wrong function name.

Suggested change
- **[packages/core/src/index.ts](mdc:packages/core/src/index.ts)** - Core framework exports (defineCommand, createCLI, option)
- **[packages/core/src/index.ts](mdc:packages/core/src/index.ts)** - Core framework exports (defineCommand, createCLI, defineOption)
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant