Skip to content

fix(ack-pay): reject empty payment option network - #220

Open
kutluhaneth46 wants to merge 1 commit into
agentcommercekit:mainfrom
kutluhaneth46:cursor/fix-ack-pay-empty-network-219-88c1
Open

kutluhaneth46 wants to merge 1 commit into
agentcommercekit:mainfrom
kutluhaneth46:cursor/fix-ack-pay-empty-network-219-88c1

Conversation

@kutluhaneth46

@kutluhaneth46 kutluhaneth46 commented Sep 18, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Fixes bug(ack-pay): paymentOptionSchema accepts an empty network string #219: paymentOptionSchema accepted network: \"\" even though an empty network is not a meaningful chain hint.
  • Valibot uses v.optional(nonEmptyString); zod uses z.string().min(1).optional(). Omitting network remains valid.
  • Parity tests cover omitted, non-empty, and empty network on both schemas.

AI usage

Cursor assisted locating the gap, writing the schema change, parity tests, and changeset. I reviewed and understand the change: when network is present it must be non-empty; absence is still allowed.

Test plan

  • pnpm --filter @agentcommercekit/ack-pay exec vitest run src/schemas/payment-option.test.ts — 19/19 pass

Summary by CodeRabbit

  • Bug Fixes

    • Payment options now reject an empty network value when the field is provided.
    • A missing network value remains valid.
    • Non-empty network names continue to be accepted consistently across supported validation methods.
  • Tests

    • Added coverage confirming validation behavior for omitted, empty, and non-empty network values.

When network is present, require a non-empty string in both valibot and
zod schemas. Omitting the field stays valid. Fixes agentcommercekit#219.

AI usage: Cursor assisted locating the gap, writing the schema change,
parity tests, and changeset. I reviewed and understand the change.
@coderabbitai

coderabbitai Bot commented Sep 18, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 25307f13-3125-41c7-952a-8094dcf80d69

📥 Commits

Reviewing files that changed from the base of the PR and between b5b25ce and 6f78f4f.

📒 Files selected for processing (4)
  • .changeset/reject-empty-network.md
  • packages/ack-pay/src/schemas/payment-option.test.ts
  • packages/ack-pay/src/schemas/valibot.ts
  • packages/ack-pay/src/schemas/zod.ts

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


Walkthrough

The payment option schemas now reject an empty network string when present. Omitted and non-empty values remain valid. Tests cover both Valibot and Zod, and a patch changeset documents the update.

Changes

Payment network validation

Layer / File(s) Summary
Schema validation
packages/ack-pay/src/schemas/valibot.ts, packages/ack-pay/src/schemas/zod.ts
Valibot adds a reusable non-empty string schema. Both validators require at least one character for network when the field is present.
Validation and release
packages/ack-pay/src/schemas/payment-option.test.ts, .changeset/reject-empty-network.md
Parameterized tests verify omitted, non-empty, and empty network values against both validators. A patch changeset documents the behavior change.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Medium

Suggested reviewers: venables

Merge Risk: ⚪ Minimal · up to 6f78f

Payment options now reject empty network values while continuing to accept omitted or non-empty values; the change is ready to merge.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the main change: rejecting an empty payment option network in ack-pay.
Linked Issues check ✅ Passed The pull request meets issue #219. packages/ack-pay/src/schemas/valibot.ts applies a minimum length of 1 to the optional network field. packages/ack-pay/src/schemas/zod.ts applies `z.string().mi…
Out of Scope Changes check ✅ Passed The changes stay within issue #219. The schema updates implement the requested validation. The parity tests verify the behavior. The changeset documents the patch-level package change. No unrelated ch…
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 3…
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

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.

bug(ack-pay): paymentOptionSchema accepts an empty network string

1 participant