feat(ack-pay): add approval model types#96
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
WalkthroughThe PR introduces a minimal approval model for ACK-Pay consisting of PaymentApprovalRequest and PaymentApprovalDecision types. These contracts are exported from the package, implemented with Valibot/Zod schemas, covered by tests, and documented in the HITL guide with examples. ChangesApproval Request and Decision Model
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 05ddd97b3f
ℹ️ 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".
| * ACK-Pay does not prescribe an approval workflow. This type gives applications | ||
| * and demos a shared object shape for approval-required paths. | ||
| */ | ||
| export interface PaymentApprovalRequest { |
There was a problem hiding this comment.
Add runtime schemas for approval models
Because this introduces new exported ACK-Pay model types, the repo’s schema contract requires matching Valibot and Zod v3/v4 schemas for new types. As written, applications receiving these approval objects over the wire can import only TypeScript interfaces, so runtime validation through @agentcommercekit/ack-pay/schemas/* is unavailable and inconsistent with existing payment request/receipt models; please add schemas for both PaymentApprovalRequest and PaymentApprovalDecision.
Useful? React with 👍 / 👎.
Signed-off-by: EfeDurmaz16 <efebarandurmaz05@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/ack-pay/hitl.mdx`:
- Line 48: The example's expiresAt field is stale (currently
"2026-01-01T12:00:00.000Z"); update the expiresAt value in the sample to a
future ISO8601 timestamp (e.g., a year or more ahead) or replace it with a
clearly labeled placeholder like expiresAt: "<FUTURE_ISO_TIMESTAMP>" so readers
copying the snippet get a valid approval window; locate and change the expiresAt
entry in the sample block where expiresAt is set.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 6240cde5-1695-4cca-8ab1-ff9ff2f15959
📒 Files selected for processing (4)
.changeset/add-approval-types.mddocs/ack-pay/hitl.mdxpackages/ack-pay/src/index.tspackages/ack-pay/src/payment-approval.ts
Signed-off-by: EfeDurmaz16 <efebarandurmaz05@gmail.com>
|
Addressed the stale approval expiry example in 5a358db by replacing the fixed past timestamp with |
|
Addressed the runtime-schema review item in a small follow-up commit. Changes:
Verification:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/ack-pay/src/schemas/valibot.ts`:
- Around line 46-60: The expiresAt and decidedAt fields in
paymentApprovalDecisionSchema currently accept any string; update them to use
the same normalized timestamp validator used by paymentRequestSchema (i.e.,
replace raw v.string() with the v.pipe(v.union([v.date(), v.string()]) +
v.transform(...) pattern used there) and keep expiresAt optional and decidedAt
required as before; locate paymentApprovalDecisionSchema and change the types
for the expiresAt and decidedAt fields to match the paymentRequestSchema
timestamp normalization implementation.
In `@packages/ack-pay/src/schemas/zod/v3.ts`:
- Around line 41-55: The approval schemas accept raw strings for timestamps;
align them with the existing paymentRequestSchema pattern by replacing
paymentApprovalRequestSchema.expiresAt and
paymentApprovalDecisionSchema.decidedAt validation with the same union([date(),
string()]).transform(...) timestamp pattern used in paymentRequestSchema so both
expiresAt and decidedAt consistently accept Date or ISO string and normalize to
a Date (or desired type) during parsing; update the validators in v3.ts (and
mirror changes in v4.ts/valibot.ts) by locating paymentApprovalRequestSchema and
paymentApprovalDecisionSchema and applying the union/date+string transform used
by paymentRequestSchema.
In `@packages/ack-pay/src/schemas/zod/v4.ts`:
- Around line 41-55: The expiresAt and decidedAt fields in
paymentApprovalDecisionSchema currently accept any string; update both to use
the same robust date validation used by paymentRequestSchema (lines 22–25) —
replace z.string().optional() for expiresAt and z.string() for decidedAt with
the identical z.preprocess/refine pattern used in paymentRequestSchema so they
only accept valid ISO date strings (and keep optionality for expiresAt). Locate
paymentApprovalDecisionSchema, paymentApprovalDecisionValueSchema, and the
paymentRequestSchema date validation pattern and apply that exact validation
logic to expiresAt and decidedAt.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4f013606-abc2-4d87-a8d6-0f9c825d2a67
📒 Files selected for processing (4)
packages/ack-pay/src/payment-approval.test.tspackages/ack-pay/src/schemas/valibot.tspackages/ack-pay/src/schemas/zod/v3.tspackages/ack-pay/src/schemas/zod/v4.ts
✅ Files skipped from review due to trivial changes (1)
- packages/ack-pay/src/payment-approval.test.ts
|
Follow-up for the timestamp consistency review is pushed. Changes:
Verification:
|
Summary
Fixes #93
Verification
Summary by CodeRabbit
New Features
Documentation
Tests
Chores