Skip to content

feat(ack-pay): add approval model types#96

Open
EfeDurmaz16 wants to merge 5 commits into
agentcommercekit:mainfrom
EfeDurmaz16:feat/ack-pay-approval-model
Open

feat(ack-pay): add approval model types#96
EfeDurmaz16 wants to merge 5 commits into
agentcommercekit:mainfrom
EfeDurmaz16:feat/ack-pay-approval-model

Conversation

@EfeDurmaz16
Copy link
Copy Markdown

@EfeDurmaz16 EfeDurmaz16 commented May 15, 2026

Summary

  • Add minimal exported ACK-Pay types for approval requests and approval decisions
  • Document how HITL approval objects fit around payment execution without adding a workflow engine to ACK core
  • Clarify that denied decisions should stop before signing or settlement, while approved decisions can be recorded as audit evidence

Fixes #93

Verification

  • pnpm run build
  • pnpm --filter ./packages/ack-pay check:types
  • pnpm --filter ./packages/ack-pay exec vitest run
  • pnpm exec oxfmt --check docs/ack-pay/hitl.mdx packages/ack-pay/src/index.ts packages/ack-pay/src/payment-approval.ts
  • git diff --check

Summary by CodeRabbit

  • New Features

    • Added payment approval request and decision types to ACK‑Pay to support approval workflows and decisions.
  • Documentation

    • Added "Approval Request and Decision Shape" page with TypeScript examples illustrating request and decision structures.
  • Tests

    • Added schema tests validating approval request and decision payloads across supported schema implementations.
  • Chores

    • Included a changeset entry to publish a patch release with these additions.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

Warning

Rate limit exceeded

@EfeDurmaz16 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 41 minutes and 48 seconds before requesting another review.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ace7be8c-e6ab-42c7-a9ac-ee30f756c01b

📥 Commits

Reviewing files that changed from the base of the PR and between 77fb9d3 and 44e73f0.

📒 Files selected for processing (4)
  • packages/ack-pay/src/payment-approval.test.ts
  • packages/ack-pay/src/schemas/valibot.ts
  • packages/ack-pay/src/schemas/zod/v3.ts
  • packages/ack-pay/src/schemas/zod/v4.ts

Walkthrough

The 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.

Changes

Approval Request and Decision Model

Layer / File(s) Summary
Approval request and decision type definitions
packages/ack-pay/src/payment-approval.ts
New module exports PaymentApprovalRequest, PaymentApprovalDecisionValue (`"approved"
Public API export
packages/ack-pay/src/index.ts
Package entrypoint re-exports ./payment-approval, making the approval types available via the main package export.
Schema implementations (Valibot, Zod v3, Zod v4)
packages/ack-pay/src/schemas/valibot.ts, packages/ack-pay/src/schemas/zod/v3.ts, packages/ack-pay/src/schemas/zod/v4.ts
Adds validation schemas for approval requests and decisions across Valibot and Zod (v3 & v4), including the decision value union and decision schema with decidedAt.
Schema tests (Vitest)
packages/ack-pay/src/payment-approval.test.ts
Vitest suite that iterates over Valibot, Zod v3, and Zod v4 wrappers to assert successful validation for well-formed requests/decisions and failures for malformed payloads.
Documentation and release
docs/ack-pay/hitl.mdx, .changeset/add-approval-types.md
HITL docs add an "Approval Request and Decision Shape" subsection with a TypeScript example; changeset records a patch release announcing the new approval model types.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 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: adding approval model types to the ack-pay package.
Linked Issues check ✅ Passed All requirements from issue #93 are met: PaymentApprovalRequest and PaymentApprovalDecision types are defined with required fields (id, requestId, decision, decidedAt) and optional fields (paymentOptionId, requesterDid, approverDid, reason, expiresAt, metadata), types are exported from packages/ack-pay, documentation is added, and runtime schemas are provided.
Out of Scope Changes check ✅ Passed All changes align with issue #93 objectives: type definitions, schema implementations, documentation, and tests directly support the approval model feature without introducing unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ 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

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

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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 {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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>
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 6f65936 and 05cd68c.

📒 Files selected for processing (4)
  • .changeset/add-approval-types.md
  • docs/ack-pay/hitl.mdx
  • packages/ack-pay/src/index.ts
  • packages/ack-pay/src/payment-approval.ts

Comment thread docs/ack-pay/hitl.mdx Outdated
Signed-off-by: EfeDurmaz16 <efebarandurmaz05@gmail.com>
@EfeDurmaz16
Copy link
Copy Markdown
Author

Addressed the stale approval expiry example in 5a358db by replacing the fixed past timestamp with <FUTURE_ISO_TIMESTAMP>.

@EfeDurmaz16
Copy link
Copy Markdown
Author

Addressed the runtime-schema review item in a small follow-up commit.

Changes:

  • Added paymentApprovalRequestSchema, paymentApprovalDecisionValueSchema, and paymentApprovalDecisionSchema to Valibot exports.
  • Added matching Zod v3 and Zod v4 schemas.
  • Added focused coverage across Valibot/Zod v3/Zod v4 for valid and malformed approval requests/decisions.

Verification:

  • pnpm --filter @agentcommercekit/ack-pay... build
  • pnpm --filter @agentcommercekit/ack-pay test -- payment-approval.test.ts
  • pnpm --filter @agentcommercekit/ack-pay check:types
  • pnpm exec oxfmt --check packages/ack-pay/src/payment-approval.test.ts packages/ack-pay/src/schemas/valibot.ts packages/ack-pay/src/schemas/zod/v3.ts packages/ack-pay/src/schemas/zod/v4.ts
  • git diff --check

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 5a358db and 77fb9d3.

📒 Files selected for processing (4)
  • packages/ack-pay/src/payment-approval.test.ts
  • packages/ack-pay/src/schemas/valibot.ts
  • packages/ack-pay/src/schemas/zod/v3.ts
  • packages/ack-pay/src/schemas/zod/v4.ts
✅ Files skipped from review due to trivial changes (1)
  • packages/ack-pay/src/payment-approval.test.ts

Comment thread packages/ack-pay/src/schemas/valibot.ts Outdated
Comment thread packages/ack-pay/src/schemas/zod/v3.ts Outdated
Comment thread packages/ack-pay/src/schemas/zod/v4.ts Outdated
@EfeDurmaz16
Copy link
Copy Markdown
Author

Follow-up for the timestamp consistency review is pushed.

Changes:

  • Reused the existing payment request timestamp normalization pattern for approval expiresAt and decidedAt.
  • Kept expiresAt optional and decidedAt required.
  • Added Date-input coverage across Valibot, Zod v3, and Zod v4 approval schemas.

Verification:

  • pnpm --filter @agentcommercekit/ack-pay test -- payment-approval.test.ts (50 tests passed)
  • pnpm --filter @agentcommercekit/ack-pay check:types
  • pnpm exec oxfmt --check packages/ack-pay/src/payment-approval.test.ts packages/ack-pay/src/schemas/valibot.ts packages/ack-pay/src/schemas/zod/v3.ts packages/ack-pay/src/schemas/zod/v4.ts
  • git diff --check

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.

feat(ack-pay): add minimal approval request and decision model

1 participant