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 validation now rejects empty payment and request identifiers, currencies, and recipients.
  • Payment amounts provided as strings must be positive whole numbers, preventing empty, zero, zero-padded, or non-numeric values.

• Tests

  • Added coverage for invalid empty fields across supported payment validation formats.

• Documentation

  • Added release notes describing the updated validation behavior.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: 7e44b860-38c0-48ce-8807-3508305b725d

📥 Commits

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

📒 Files selected for processing (4)

• packages/ack-pay/src/schemas/payment-option.test.ts
• packages/ack-pay/src/schemas/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 identifiers and fields. The Valibot amount string branch also requires a positive integer string. Parameterized tests cover both Valibot and Zod validators, 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 reject empty payment option identifiers, currencies, recipients, and payment request identifiers. Valibot string amounts must match a positive integer pattern.
Validation coverage and release metadata packages/ack-pay/src/schemas/payment-option.test.ts, packages/ack-pay/src/schemas/schemas.test.ts, .changeset/payment-option-empty-fields.md	Parameterized tests verify rejection of empty fields in both validators. The changeset records a patch release.

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

Merge Risk: ⚪ Minimal · up to cdf5d

Payment request and option schemas now consistently reject empty identifiers and related fields, with coverage for both supported validators. No merge-blocking risk remains.

🚥 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 identifiers. It matches the schema updates and related tests.
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 5…
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.

✨ 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.}

[kutluhaneth46]

This PR is currently conflicting with main. Opening a fresh, rebased PR with the same empty-string rejection for payment request/option identifiers so the fix can land cleanly.

[crazywriter1]

Conflicts with main are fixed.