Skip to content

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

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

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

Conversation

@kutluhaneth46

@kutluhaneth46 kutluhaneth46 commented Sep 27, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #219

AI disclosure: prepared with Cursor assistance; I reviewed the schema change and tests.

Summary by CodeRabbit

  • Bug Fixes
    • Payment options now reject an empty network value when provided. The network field remains optional, and non-empty values are accepted.

When network is present, require a non-empty string in both valibot and
zod schemas (agentcommercekit#219).

AI disclosure: prepared with Cursor assistance; I reviewed the schema
change and tests.
@coderabbitai

coderabbitai Bot commented Sep 27, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: 73bede97-20b0-4f41-b915-a7dc7f361af7

📥 Commits

Reviewing files that changed from the base of the PR and between 54e763c and 57367a7.

📒 Files selected for processing (4)
  • .changeset/reject-empty-payment-option-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: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.


Walkthrough

The Valibot and Zod payment-option schemas now reject an empty network value when provided. The field remains optional. Tests cover omitted, non-empty, and empty values, and a patch changeset records the change.

Changes

Payment Network Validation

Layer / File(s) Summary
Payment-option network validation
packages/ack-pay/src/schemas/valibot.ts, packages/ack-pay/src/schemas/zod.ts, packages/ack-pay/src/schemas/payment-option.test.ts, .changeset/reject-empty-payment-option-network.md
Both schemas require at least one character when network is present. Tests verify that omitted and non-empty values parse, while empty values fail. The changeset declares a patch release.

Priority: ⬇️ Low

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

Change: Bug fix · Severity of issue fixed: Low

Merge Risk: ⚪ Minimal · up to 57367

Both validators reject explicitly empty networks while preserving omission, and matching tests cover both cases. No actionable merge risk is evident in the reviewed change.

Architecture Summary

Architecture risk: 🔵 Low · up to 57367

The change affects 1 system.

Changed systems: packages/ack-pay

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — packages/ack-pay (library) was modified; 3 changed files map to changed impact.

Before / after behavior

  • observed — Modified behavior in packages/ack-pay/src/schemas/payment-option.test.ts: Added matching Valibot and Zod schema tests for omitted, non-empty, and empty network values; the first two must parse successfully, while the empty value must fail.
  • observed — Modified behavior in packages/ack-pay/src/schemas/valibot.ts: paymentOptionSchema.network now validates that a supplied string has at least one character; the field remains optional.
  • observed — Modified behavior in packages/ack-pay/src/schemas/zod.ts: paymentOptionSchema.network now requires at least one character when present; previously, any string, including an empty string, was accepted.
  • observed — Modified behavior in .changeset/reject-empty-payment-option-network.md: Adds a patch changeset for @agentcommercekit/ack-pay describing the empty-string network rejection and omission allowance.
🚥 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 empty payment-option network strings.
Linked Issues check ✅ Passed Issue #219 requires a non-empty network when present and continued acceptance when omitted. The Valibot schema now uses v.optional(v.pipe(v.string(), v.minLength(1))). The Zod schema now uses `z.s…
Out of Scope Changes check ✅ Passed The changes stay within issue #219. The test additions verify the required schema behavior. The changeset documents the patch release for the affected package. No unrelated code changes are present.
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.

@kutluhaneth46

Copy link
Copy Markdown
Contributor Author

@venables — small validation harden: empty network strings rejected at schema parse. CI green + changeset included. Ready for review.

This branch has not been deployed

No deployments
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