Skip to content

docs(ack-pay): document receipt evidence metadata#95

Open
EfeDurmaz16 wants to merge 1 commit into
agentcommercekit:mainfrom
EfeDurmaz16:docs/receipt-evidence-metadata
Open

docs(ack-pay): document receipt evidence metadata#95
EfeDurmaz16 wants to merge 1 commit into
agentcommercekit:mainfrom
EfeDurmaz16:docs/receipt-evidence-metadata

Conversation

@EfeDurmaz16
Copy link
Copy Markdown

@EfeDurmaz16 EfeDurmaz16 commented May 15, 2026

Summary

  • Document optional ACK-Pay receipt evidence metadata for policy, mandate, execution, and settlement references
  • Clarify that receipt metadata is non-normative application evidence and should not carry secrets or raw payment credentials
  • Add a verification round-trip test proving receipt metadata survives signing, JWT parsing, and receipt verification

Fixes #92

Verification

  • pnpm run build
  • pnpm --filter ./packages/ack-pay exec vitest run src/verify-payment-receipt.test.ts src/create-payment-receipt.test.ts
  • pnpm --filter ./packages/ack-pay check:types
  • pnpm exec oxfmt --check docs/ack-pay/receipt-verification.mdx packages/ack-pay/src/verify-payment-receipt.test.ts
  • git diff --check

Summary by CodeRabbit

  • Documentation

    • Added Receipt Evidence Metadata section documenting metadata usage as an optional extension point in receipts, including stability guarantees, non-normative constraints, and example JSON structure.
  • Tests

    • Added test case verifying metadata preservation during payment receipt verification.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 15, 2026

Walkthrough

This PR documents optional receipt evidence metadata as a non-normative extension point in ACK payment receipts, allowing implementers to link receipts to policy, mandate, and execution references without modifying the core credential format. Includes test setup improvements and a new test confirming metadata preservation through signing and verification.

Changes

Receipt Evidence Metadata Feature

Layer / File(s) Summary
Receipt metadata documentation
docs/ack-pay/receipt-verification.mdx
Adds "Receipt Evidence Metadata" section defining credentialSubject.metadata as an optional, verifier-specific extension point for evidence and reference hashes (policy, mandate, execution, settlement). Clarifies that core fields remain stable, metadata is non-normative, and secrets must be excluded. Includes JSON example.
Test setup refactoring for keypair and payment request reuse
packages/ack-pay/src/verify-payment-receipt.test.ts
Introduces outer-scoped receiptIssuerKeypair initialized in beforeEach for reuse across tests. Refactors payment-request creation to store the result, extract paymentRequestToken, and use the generated payment option id when building unsigned receipts.
Metadata preservation test
packages/ack-pay/src/verify-payment-receipt.test.ts
Adds test case that creates a receipt with metadata, signs it into a JWT, verifies it via verifyPaymentReceipt, and confirms the returned credential preserves the metadata under credentialSubject.metadata.

🎯 2 (Simple) | ⏱️ ~12 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 accurately summarizes the main change: documenting receipt evidence metadata for ACK-Pay, which is the primary focus of both the documentation additions and test modifications.
Linked Issues check ✅ Passed The PR fully addresses issue #92 requirements: documentation of optional receipt metadata fields is added, a test demonstrating metadata survives signing/parsing/verification is included, and core schema fields remain unchanged.
Out of Scope Changes check ✅ Passed All changes directly support the stated objective of documenting receipt evidence metadata without modifying core receipt schema or verifier semantics; no out-of-scope changes detected.
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

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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

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

🧹 Nitpick comments (2)
packages/ack-pay/src/verify-payment-receipt.test.ts (2)

117-120: ⚡ Quick win

Use derived paymentOptionId instead of a hardcoded literal.

This test now bypasses the setup’s dynamic fixture derivation by pinning "test-payment-option-id". Reusing a derived id keeps the round-trip test resilient to fixture changes.

♻️ Proposed change
   const receiptWithMetadata = createPaymentReceipt({
       paymentRequestToken,
-      paymentOptionId: "test-payment-option-id",
+      paymentOptionId: unsignedReceipt.credentialSubject.paymentOptionId,
       issuer: receiptIssuerDid,
🤖 Prompt for 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.

In `@packages/ack-pay/src/verify-payment-receipt.test.ts` around lines 117 - 120,
The test uses a hardcoded paymentOptionId string when calling
createPaymentReceipt; replace that literal with the dynamically derived id from
the test fixture/setup (the variable used earlier in the test setup, e.g.
paymentOption.id or derivedPaymentOptionId) so createPaymentReceipt({
paymentRequestToken, paymentOptionId: <use-derived-id>, issuer: receiptIssuerDid
}) uses the actual fixture-derived id and preserves round-trip resilience.

105-105: ⚡ Quick win

Rename the test to match the required assertive naming pattern.

Use one of the approved prefixes (e.g., returns) for consistency with test conventions.

✏️ Proposed rename
-  it("preserves receipt metadata through JWT verification", async () => {
+  it("returns receipt metadata through JWT verification", async () => {

As per coding guidelines, "Use assertive test names with patterns: it(\"creates...\") , it(\"throws...\") , it(\"requires...\") , it(\"returns...\")".

🤖 Prompt for 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.

In `@packages/ack-pay/src/verify-payment-receipt.test.ts` at line 105, The test
currently named "preserves receipt metadata through JWT verification" should be
renamed to follow the assertive naming convention (use an approved prefix such
as "returns"); update the it(...) description string in the test definition so
it begins with "returns" (for example: "returns receipt metadata after JWT
verification") to match the project's required assertive test-name pattern and
keep the same test body and assertions in verify-payment-receipt.test.ts.
🤖 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.

Nitpick comments:
In `@packages/ack-pay/src/verify-payment-receipt.test.ts`:
- Around line 117-120: The test uses a hardcoded paymentOptionId string when
calling createPaymentReceipt; replace that literal with the dynamically derived
id from the test fixture/setup (the variable used earlier in the test setup,
e.g. paymentOption.id or derivedPaymentOptionId) so createPaymentReceipt({
paymentRequestToken, paymentOptionId: <use-derived-id>, issuer: receiptIssuerDid
}) uses the actual fixture-derived id and preserves round-trip resilience.
- Line 105: The test currently named "preserves receipt metadata through JWT
verification" should be renamed to follow the assertive naming convention (use
an approved prefix such as "returns"); update the it(...) description string in
the test definition so it begins with "returns" (for example: "returns receipt
metadata after JWT verification") to match the project's required assertive
test-name pattern and keep the same test body and assertions in
verify-payment-receipt.test.ts.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5eca1f7f-3435-4c22-9f47-c11fee350a36

📥 Commits

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

📒 Files selected for processing (2)
  • docs/ack-pay/receipt-verification.mdx
  • packages/ack-pay/src/verify-payment-receipt.test.ts

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.

docs(ack-pay): document optional receipt evidence metadata

1 participant