fix(ack-pay): reject empty payment option identifiers

[crazywriter1]

Problem

paymentOptionSchema accepted empty strings for id, currency, and recipient, and paymentRequestSchema accepted an empty request id. Other fields already reject invalid values — amount must be positive, and the zod schema rejects negative decimals — so an all-empty option was not an intentional allowance, just v.string() / z.string() with no length check.

A signed payment request token can carry those fields through to verification, so the schema is the gate for HTTP 402 bodies and anything that parses them with verifyPaymentRequestToken.

Fix

Introduce a shared nonEmptyString in both schema files and use it for the payment request id and for id, currency, and recipient on each payment option. Valibot and zod stay in sync, per the repo's dual-schema rule.

Tests

Extended schemas.test.ts with parity coverage: each schema rejects a payment option with an empty id, currency, or recipient, and rejects a payment request with an empty top-level id.

Patch changeset included for @agentcommercekit/ack-pay.

AI usage

Per AI_POLICY.md, Cursor did the work during a rescan of main: it measured the empty-string acceptance, wrote the schema change and parity tests, and added the changeset. I read the diff before opening this.

Summary by CodeRabbit

• Bug Fixes

  • Payment options now reject empty IDs, currencies, and recipients.
  • Payment requests now reject empty IDs.
  • Validation behavior is consistent across supported schema implementations.

• Tests

  • Added coverage confirming empty required fields are correctly rejected.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 721e2859-d489-4dbe-8c3b-fa83237e89d0

📥 Commits

Reviewing files that changed from the base of the PR and between 0732d97 and d5c2cab.

📒 Files selected for processing (4)

• .changeset/payment-option-empty-fields.md
• packages/ack-pay/src/schemas.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 schemas now reject empty strings for payment option identifiers, currency, recipient, and payment request identifiers in both Valibot and Zod. Parameterized tests cover these constraints, and a patch changeset documents the update.

Changes

Payment schema validation

Layer / File(s)	Summary
Non-empty schema constraints packages/ack-pay/src/schemas/valibot.ts, packages/ack-pay/src/schemas/zod.ts	Both schema implementations use reusable non-empty string validation for payment option id, currency, and recipient, plus payment request id.
Validation coverage and release metadata packages/ack-pay/src/schemas.test.ts, .changeset/payment-option-empty-fields.md	Parameterized tests verify rejection of empty fields in both implementations. The changeset declares a patch release.

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

Merge Risk: ⚪ Minimal · up to d5c2c

The change rejects empty payment-request and payment-option identifiers across both supported schemas, preventing malformed inputs without adding new runtime or deployment behavior. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: efedurmaz16

🚥 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 describes the main schema change: rejecting empty payment option identifiers. It is concise and directly related to the changeset.
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…
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

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 files. (1 skipped: 1 unsupported.)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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