feat: add MCP server for ACK-ID and ACK-Pay operations#72
Open
ak68a wants to merge 7 commits intoagentcommercekit:mainfrom
Open
feat: add MCP server for ACK-ID and ACK-Pay operations#72ak68a wants to merge 7 commits intoagentcommercekit:mainfrom
ak68a wants to merge 7 commits intoagentcommercekit:mainfrom
Conversation
Expose 9 tools via stdio MCP transport: identity (create/sign/verify credentials, resolve DIDs), payments (create/verify payment requests and receipts), and keypair generation. Signing tools accept JWK to prevent curve mismatches. MCP SDK pinned to 1.21.2 for zod 3 compatibility.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
WalkthroughAdds a new private ESM MCP server package with CLI entrypoint, registers identity/payment-request/payment-receipt/utility MCP tools, introduces shared utilities, TypeScript and Vitest configs, and multiple unit and end-to-end tests. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
tools/mcp-server/package.json (1)
17-19: Consider addingtsxto devDependencies.The
bin.ack-mcpentry (line 18) points to a TypeScript file, and thestartscript (line 25) explicitly usestsx. Whiletsxis available from the workspace root, adding it explicitly to this package's devDependencies would make the dependency clear and the package more self-contained.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/mcp-server/package.json` around lines 17 - 19, The package.json currently exposes a TypeScript entrypoint ("bin"."ack-mcp": "./src/index.ts") and the start script relies on tsx; add "tsx" to this package's devDependencies (matching your workspace version or a compatible semver) so the package is self-contained—update package.json devDependencies to include tsx and run the package install (or add via npm/yarn pnpm) to lock it in.tools/mcp-server/src/tools/payment-requests.ts (2)
77-81: Condition always truthy due to default value.Since
expiresInSecondsdefaults to3600(line 51), the conditionif (expiresInSeconds)will always be true for normal usage. However, if a caller explicitly passes0to disable expiration, the falsy check would skip settingexpiresAt—which may be intentional.If
0should explicitly mean "no expiration," consider documenting this behavior in the schema description, or use an explicitundefinedcheck:if (expiresInSeconds !== undefined && expiresInSeconds > 0) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/mcp-server/src/tools/payment-requests.ts` around lines 77 - 81, The conditional using expiresInSeconds currently always evaluates truthy because a default of 3600 is applied; update the check around setting init.expiresAt so callers can explicitly pass 0 to mean "no expiration" — replace the truthy check with an explicit undefined and positive-value check (e.g., check expiresInSeconds !== undefined && expiresInSeconds > 0) in the block that assigns init.expiresAt; alternatively, document in the input schema or parameter docs that 0 means no expiration if you prefer to keep the current behavior.
43-64: Consider using Zod's.min(1)instead of manual validation.The manual check on lines 62-64 is redundant with Zod's built-in array constraints. Using
.min(1)would validate at schema level and provide consistent error handling.♻️ Suggested refactor
paymentOptions: z .array(paymentOptionSchema) + .min(1, "At least one payment option is required") .describe( "Array of payment options (amount, currency, recipient, network)", ),Then remove the manual check:
async ({ description, paymentOptions, expiresInSeconds, jwk, did }) => { try { - if (paymentOptions.length === 0) { - throw new Error("At least one payment option is required") - } - const keypair = keypairFromJwk(jwk)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/mcp-server/src/tools/payment-requests.ts` around lines 43 - 64, Add Zod array constraint to validate at schema level by changing the paymentOptions schema definition (the z.array(paymentOptionSchema) in the handler/schema block) to use .min(1) so empty arrays are rejected by Zod; then remove the manual runtime check that throws "At least one payment option is required" inside the async handler (the if (paymentOptions.length === 0) { throw new Error(...) } block) to avoid redundant validation and rely on Zod's consistent error handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tools/mcp-server/package.json`:
- Around line 17-19: The package.json currently exposes a TypeScript entrypoint
("bin"."ack-mcp": "./src/index.ts") and the start script relies on tsx; add
"tsx" to this package's devDependencies (matching your workspace version or a
compatible semver) so the package is self-contained—update package.json
devDependencies to include tsx and run the package install (or add via npm/yarn
pnpm) to lock it in.
In `@tools/mcp-server/src/tools/payment-requests.ts`:
- Around line 77-81: The conditional using expiresInSeconds currently always
evaluates truthy because a default of 3600 is applied; update the check around
setting init.expiresAt so callers can explicitly pass 0 to mean "no expiration"
— replace the truthy check with an explicit undefined and positive-value check
(e.g., check expiresInSeconds !== undefined && expiresInSeconds > 0) in the
block that assigns init.expiresAt; alternatively, document in the input schema
or parameter docs that 0 means no expiration if you prefer to keep the current
behavior.
- Around line 43-64: Add Zod array constraint to validate at schema level by
changing the paymentOptions schema definition (the z.array(paymentOptionSchema)
in the handler/schema block) to use .min(1) so empty arrays are rejected by Zod;
then remove the manual runtime check that throws "At least one payment option is
required" inside the async handler (the if (paymentOptions.length === 0) { throw
new Error(...) } block) to avoid redundant validation and rely on Zod's
consistent error handling.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 08757648-7bf6-4ba2-aa7d-59eadde0d8c2
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
tools/mcp-server/package.jsontools/mcp-server/src/index.tstools/mcp-server/src/tools/identity.test.tstools/mcp-server/src/tools/identity.tstools/mcp-server/src/tools/payment-receipts.tstools/mcp-server/src/tools/payment-requests.test.tstools/mcp-server/src/tools/payment-requests.tstools/mcp-server/src/tools/utility.tstools/mcp-server/src/util.test.tstools/mcp-server/src/util.tstools/mcp-server/tsconfig.jsontools/mcp-server/vitest.config.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tools/mcp-server/src/tools/workflow.test.ts (1)
33-130: Test currently validates SDK workflow, not MCP tool contract.This covers core
agentcommercekitflows well, but it does not execute MCP tool handlers (e.g.,ack_verify_payment_request) and therefore misses wrapper behavior like structuredverification(false, { reason })responses vs thrown errors. Add at least one integration path through registered MCP tools to validate real tool I/O contracts.Also applies to: 157-188
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/mcp-server/src/tools/workflow.test.ts` around lines 33 - 130, The test exercises SDK flows directly (createSignedPaymentRequest, verifyPaymentRequestToken, createPaymentReceipt, verifyPaymentReceipt) but must also assert real MCP tool I/O by invoking the registered MCP handler (e.g., ack_verify_payment_request) so wrapper behavior (structured verification(false, { reason }) vs thrown errors) is validated; update the test to register or use the existing MCP tools/dispatcher, call the ack_verify_payment_request handler with the produced paymentRequestToken (instead of directly calling verifyPaymentRequestToken) and assert the handler returns the expected structured verification response, then mirror this integration approach for the second similar test block that covers receipt/payment verification to ensure tool-level contracts are asserted.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/mcp-server/src/tools/workflow.test.ts`:
- Around line 34-35: Rename the non-assertive test titles to follow the
project's assertive naming pattern (it("creates...", it("throws...",
it("requires...", it("returns...")). For the test currently titled "completes an
identity + payment cycle end-to-end" (and the other tests flagged at the same
file), update the string to an assertive form such as "creates an identity and
processes payment end-to-end"; similarly replace any "completes..." with
"creates..." and "rejects..." with "throws..." (or other appropriate assertive
verbs) so that the test descriptions in the workflow.test.ts suite conform to
the required pattern and remain semantically equivalent.
---
Nitpick comments:
In `@tools/mcp-server/src/tools/workflow.test.ts`:
- Around line 33-130: The test exercises SDK flows directly
(createSignedPaymentRequest, verifyPaymentRequestToken, createPaymentReceipt,
verifyPaymentReceipt) but must also assert real MCP tool I/O by invoking the
registered MCP handler (e.g., ack_verify_payment_request) so wrapper behavior
(structured verification(false, { reason }) vs thrown errors) is validated;
update the test to register or use the existing MCP tools/dispatcher, call the
ack_verify_payment_request handler with the produced paymentRequestToken
(instead of directly calling verifyPaymentRequestToken) and assert the handler
returns the expected structured verification response, then mirror this
integration approach for the second similar test block that covers
receipt/payment verification to ensure tool-level contracts are asserted.
🪄 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: 89e9b0c7-84f6-49d9-8d53-69caec8d0b2c
📒 Files selected for processing (1)
tools/mcp-server/src/tools/workflow.test.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tools/mcp-server/src/tools/utility.ts (1)
19-19: Reduce duplicate private-key exposure in response payload.Line 32 already returns private key material via JWK (for private JWKs), and Line 33 returns the same secret again as hex. Duplicating secrets increases leakage risk in MCP transcripts/logs. Consider returning only one private representation (preferably JWK) or gating
privateKeybehind explicit opt-in.Suggested minimal change
- "Generate a new cryptographic keypair. Returns the private key (hex), public key (hex), JWK, DID, and curve. Use the JWK value when calling other tools that require signing.", + "Generate a new cryptographic keypair. Returns public key (hex), JWK, DID, and curve. Use the JWK value when calling other tools that require signing.", @@ did: createDidKeyUri(keypair), jwk: JSON.stringify(keypairToJwk(keypair)), - privateKey: bytesToHexString(keypair.privateKey), publicKey: bytesToHexString(keypair.publicKey),Also applies to: 32-33
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/mcp-server/src/tools/utility.ts` at line 19, The response currently exposes private key material twice (private JWK + privateKey hex); update the keypair generator to avoid duplicate secret exposure by returning only the private JWK by default and removing the redundant privateKey hex from the default payload (or make the hex privateKey returned only when an explicit opt-in flag is passed, e.g. add an optional boolean parameter like returnPrivateKeyHex to the generator function). Update the function signature and response shape used by the generator and its callers, ensure docs/string description reflects that privateKey hex is gated behind opt-in, and keep the JWK (with private fields) as the canonical private representation (referencing the privateKey and JWK fields in the current implementation).tools/mcp-server/src/tools/payment-requests.ts (1)
44-48: Move non-emptypaymentOptionsenforcement into schema and remove the cast.The runtime length check + tuple cast is brittle. Enforcing non-empty at schema level gives cleaner runtime errors and correct static typing for
PaymentRequestInit.♻️ Proposed refactor
paymentOptions: z - .array(paymentOptionSchema) + .array(paymentOptionSchema) + .nonempty("At least one payment option is required") .describe( "Array of payment options (amount, currency, recipient, network)", ), @@ - if (paymentOptions.length === 0) { - throw new Error("At least one payment option is required") - } - const keypair = keypairFromJwk(jwk) @@ - paymentOptions: paymentOptions as [ - (typeof paymentOptions)[0], - ...typeof paymentOptions, - ], + paymentOptions, }Also applies to: 63-76
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/mcp-server/src/tools/payment-requests.ts` around lines 44 - 48, Update the zod schema so paymentOptions is enforced as a non-empty array at schema level (use z.array(paymentOptionSchema).nonempty() or equivalent) and remove any runtime length check plus tuple cast that coerces typing (references: paymentOptions, paymentOptionSchema, and the PaymentRequestInit usage). Apply the same change to the other schema occurrence mentioned (the block around the second paymentOptions definition) so both runtime checks/casts are removed and TypeScript correctly infers a non-empty array from the schema.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/mcp-server/src/tools/identity.ts`:
- Around line 110-112: The catch blocks that return verification(false, {
reason: (e as Error).message }) can yield undefined for non-Error throws;
replace the cast with a safe message extractor (e.g., const reason = e
instanceof Error ? e.message : (typeof e === 'string' ? e : JSON.stringify(e) ||
String(e))) and return verification(false, { reason }); apply this change to the
identical patterns in identity.ts (the verification(...) call),
payment-requests.ts, and payment-receipts.ts so all non-Error throws produce a
stable, informative reason string.
In `@tools/mcp-server/src/tools/payment-requests.ts`:
- Around line 49-53: Remove the unnecessary .default(3600) from the Zod schema
for expiresInSeconds and update the runtime check that sets expiresAt to use
strict undefined comparison (if (expiresInSeconds !== undefined)) so an explicit
0 is honored; apply the same change to the other expiresInSeconds
schema/assignment in this file (the second occurrence that sets expiresAt) so
omitting the field leaves expiresAt unset and providing 0 correctly sets a
zero-second expiry.
---
Nitpick comments:
In `@tools/mcp-server/src/tools/payment-requests.ts`:
- Around line 44-48: Update the zod schema so paymentOptions is enforced as a
non-empty array at schema level (use z.array(paymentOptionSchema).nonempty() or
equivalent) and remove any runtime length check plus tuple cast that coerces
typing (references: paymentOptions, paymentOptionSchema, and the
PaymentRequestInit usage). Apply the same change to the other schema occurrence
mentioned (the block around the second paymentOptions definition) so both
runtime checks/casts are removed and TypeScript correctly infers a non-empty
array from the schema.
In `@tools/mcp-server/src/tools/utility.ts`:
- Line 19: The response currently exposes private key material twice (private
JWK + privateKey hex); update the keypair generator to avoid duplicate secret
exposure by returning only the private JWK by default and removing the redundant
privateKey hex from the default payload (or make the hex privateKey returned
only when an explicit opt-in flag is passed, e.g. add an optional boolean
parameter like returnPrivateKeyHex to the generator function). Update the
function signature and response shape used by the generator and its callers,
ensure docs/string description reflects that privateKey hex is gated behind
opt-in, and keep the JWK (with private fields) as the canonical private
representation (referencing the privateKey and JWK fields in the current
implementation).
🪄 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: 803e41db-cfd1-47a1-882b-5b71e3c9239f
📒 Files selected for processing (5)
tools/mcp-server/src/tools/identity.tstools/mcp-server/src/tools/payment-receipts.tstools/mcp-server/src/tools/payment-requests.tstools/mcp-server/src/tools/utility.tstools/mcp-server/src/tools/workflow.test.ts
✅ Files skipped from review due to trivial changes (1)
- tools/mcp-server/src/tools/workflow.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/mcp-server/src/tools/payment-receipts.ts
Comment on lines
+110
to
+112
| } catch (e) { | ||
| return verification(false, { reason: (e as Error).message }) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only check: find catch blocks that assume Error shape via `(e as Error).message`
rg -nP --type ts '\(e as Error\)\.message'Repository: agentcommercekit/ack
Length of output: 414
🏁 Script executed:
cd tools/mcp-server && find . -name "identity.ts" -type fRepository: agentcommercekit/ack
Length of output: 87
🏁 Script executed:
cat -n tools/mcp-server/src/tools/identity.ts | sed -n '100,120p'Repository: agentcommercekit/ack
Length of output: 869
🏁 Script executed:
# Check the verification function signature and its usage
rg -A 3 -B 3 "const verification" tools/mcp-server/src/tools/Repository: agentcommercekit/ack
Length of output: 46
🏁 Script executed:
cat -n tools/mcp-server/src/tools/identity.ts | head -30Repository: agentcommercekit/ack
Length of output: 985
🏁 Script executed:
# Search for verification function definition
rg -n "function verification|const verification|export.*verification" tools/mcp-server/src/Repository: agentcommercekit/ack
Length of output: 125
🏁 Script executed:
cat -n tools/mcp-server/src/util.ts | sed -n '64,85p'Repository: agentcommercekit/ack
Length of output: 240
🏁 Script executed:
# Also check the other two files where this pattern appears
cat -n tools/mcp-server/src/tools/payment-requests.ts | sed -n '115,125p'Repository: agentcommercekit/ack
Length of output: 349
🏁 Script executed:
cat -n tools/mcp-server/src/tools/payment-receipts.ts | sed -n '78,88p'Repository: agentcommercekit/ack
Length of output: 374
Harden error handling for non-Error throws.
At line 111, (e as Error).message returns undefined when a non-Error value is thrown (e.g., throw "message" or throw { code: 123 }), which weakens failure diagnostics. This pattern appears in 3 locations across the codebase.
🔧 Proposed fix
- } catch (e) {
- return verification(false, { reason: (e as Error).message })
+ } catch (e) {
+ const reason = e instanceof Error ? e.message : String(e)
+ return verification(false, { reason })
}Apply this fix to:
tools/mcp-server/src/tools/identity.ts:111tools/mcp-server/src/tools/payment-requests.ts:120tools/mcp-server/src/tools/payment-receipts.ts:83
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } catch (e) { | |
| return verification(false, { reason: (e as Error).message }) | |
| } | |
| } catch (e) { | |
| const reason = e instanceof Error ? e.message : String(e) | |
| return verification(false, { reason }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tools/mcp-server/src/tools/identity.ts` around lines 110 - 112, The catch
blocks that return verification(false, { reason: (e as Error).message }) can
yield undefined for non-Error throws; replace the cast with a safe message
extractor (e.g., const reason = e instanceof Error ? e.message : (typeof e ===
'string' ? e : JSON.stringify(e) || String(e))) and return verification(false, {
reason }); apply this change to the identical patterns in identity.ts (the
verification(...) call), payment-requests.ts, and payment-receipts.ts so all
non-Error throws produce a stable, informative reason string.
- Change shebang from node to npx tsx so TypeScript source runs directly - Fix expiresInSeconds check: use !== undefined instead of truthy check so that 0 correctly creates an immediately-expiring request
- Use instanceof Error check instead of unsafe (e as Error).message cast in all 3 verification catch blocks (identity, payment-requests, payment-receipts) - Remove .default(3600) from expiresInSeconds schema so omitting the field leaves expiresAt unset rather than always setting it - Add .int().nonnegative() validation to expiresInSeconds
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
tools/mcp-server/src/tools/payment-receipts.ts (1)
78-81: Consider includingpaymentRequestTokenin the verification response.According to the
verifyPaymentReceiptreturn type (frompackages/ack-pay/src/verify-payment-receipt.ts), the function returnspaymentRequestTokenalongsidereceiptandpaymentRequest. The current implementation omits this field, which could be useful for callers who need the raw token for further processing or logging.♻️ Suggested enhancement
return verification(true, { receipt: result.receipt, paymentRequest: result.paymentRequest, + paymentRequestToken: result.paymentRequestToken, })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/mcp-server/src/tools/payment-receipts.ts` around lines 78 - 81, The verification response currently returns only receipt and paymentRequest; update the return object in the verification(true, {...}) call to also include result.paymentRequestToken (matching the verifyPaymentReceipt return type) so callers receive the raw paymentRequestToken; locate the return in tools/mcp-server/src/tools/payment-receipts.ts where verification is invoked and add paymentRequestToken: result.paymentRequestToken to the payload.tools/mcp-server/src/tools/identity.ts (1)
32-41: Tighten boundary schemas for DID/JWT inputs.These fields are accepted as plain strings, then coerced with casts later. Adding lightweight schema refinements (at least DID/JWT shape checks) will fail fast with clearer MCP errors.
Suggested refactor
+const didSchema = z + .string() + .regex(/^did:[a-z0-9]+:.+/i, "Invalid DID URI") + +const jwtSchema = z + .string() + .regex(/^[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+\.[A-Za-z0-9_-]+$/, "Invalid JWT format") + server.tool( "ack_create_controller_credential", @@ - subject: z - .string() + subject: didSchema .describe("DID of the subject (the entity being controlled)"), - controller: z - .string() + controller: didSchema .describe("DID of the controller (the entity with authority)"), - issuer: z - .string() + issuer: didSchema .optional() .describe("DID of the issuer. Defaults to the controller."), @@ - did: z.string().describe("DID of the signer"), + did: didSchema.describe("DID of the signer"), @@ - jwt: z.string().describe("The signed credential JWT string"), + jwt: jwtSchema.describe("The signed credential JWT string"), @@ - did: z.string().describe("The DID URI to resolve"), + did: didSchema.describe("The DID URI to resolve"),Also applies to: 69-70, 90-93, 121-122
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/mcp-server/src/tools/identity.ts` around lines 32 - 41, The schema currently accepts raw strings for DID/JWT fields (e.g., the z.string() definitions for subject, controller, issuer in tools/mcp-server/src/tools/identity.ts and the other string fields referenced around the file) and then casts/validates later; tighten these schemas by adding lightweight Zod refinements: for DID fields (subject, controller, issuer, etc.) use a .refine or .regex to assert the value has a DID-like shape (e.g., starts with "did:" and a non-empty method/identifier) and for JWT inputs add a .refine or .regex to assert the three-segment JWT structure (two dots). Update the relevant schema definitions (the subject/controller/issuer z.string() and the other string validators referenced) to include clear error messages so invalid input fails fast with readable MCP errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tools/mcp-server/src/tools/identity.ts`:
- Around line 74-75: When calling signCredential with JSON.parse(credential)
ensure the parsed value is a JSON object (not null, not an array, not a
primitive) before passing it to signCredential; replace the direct JSON.parse
call with a guarded parse: parse into a local variable, validate with typeof
parsed === "object" && parsed !== null && !Array.isArray(parsed), and if the
check fails throw or return a clear error (e.g., bad request) referencing the
input `credential`; keep the existing call to signCredential(did as DidUri, ...)
but pass the validated object.
---
Nitpick comments:
In `@tools/mcp-server/src/tools/identity.ts`:
- Around line 32-41: The schema currently accepts raw strings for DID/JWT fields
(e.g., the z.string() definitions for subject, controller, issuer in
tools/mcp-server/src/tools/identity.ts and the other string fields referenced
around the file) and then casts/validates later; tighten these schemas by adding
lightweight Zod refinements: for DID fields (subject, controller, issuer, etc.)
use a .refine or .regex to assert the value has a DID-like shape (e.g., starts
with "did:" and a non-empty method/identifier) and for JWT inputs add a .refine
or .regex to assert the three-segment JWT structure (two dots). Update the
relevant schema definitions (the subject/controller/issuer z.string() and the
other string validators referenced) to include clear error messages so invalid
input fails fast with readable MCP errors.
In `@tools/mcp-server/src/tools/payment-receipts.ts`:
- Around line 78-81: The verification response currently returns only receipt
and paymentRequest; update the return object in the verification(true, {...})
call to also include result.paymentRequestToken (matching the
verifyPaymentReceipt return type) so callers receive the raw
paymentRequestToken; locate the return in
tools/mcp-server/src/tools/payment-receipts.ts where verification is invoked and
add paymentRequestToken: result.paymentRequestToken to the payload.
🪄 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: 75003856-398d-48b4-9f78-d624af5d0390
📒 Files selected for processing (4)
tools/mcp-server/src/index.tstools/mcp-server/src/tools/identity.tstools/mcp-server/src/tools/payment-receipts.tstools/mcp-server/src/tools/payment-requests.ts
✅ Files skipped from review due to trivial changes (1)
- tools/mcp-server/src/index.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- tools/mcp-server/src/tools/payment-requests.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adds an MCP server that exposes ACK identity and payment operations as tools. Any MCP-compatible agent can generate keypairs, create and verify credentials, issue payment requests, and verify receipts.
Tools
Identity (ACK-ID)
ack_create_controller_credential— create ownership proofack_sign_credential— sign a credential, returns JWTack_verify_credential— verify signature, expiry, trusted issuersack_resolve_did— resolve did:key, did:web, did:pkhPayments (ACK-Pay)
ack_create_payment_request— create signed payment request tokenack_verify_payment_request— verify and parse a payment request JWTack_create_payment_receipt— issue a receipt as a Verifiable Credentialack_verify_payment_receipt— verify a payment receiptUtility
ack_generate_keypair— generate secp256k1/secp256r1/Ed25519 keypair with DIDSigning tools accept JWK (returned by
ack_generate_keypair) rather than raw private key + curve to prevent curve mismatches.Uses stdio transport. MCP SDK pinned to 1.21.2 for zod 3 compatibility with the workspace.
Also added a workflow eval that exercises the full pipeline end-to-end:
Figured if we're shipping MCP tools we should prove the whole cycle actually works.
Test plan
pnpm run checkcleanSummary by CodeRabbit
AI Disclosure: This PR was developed with assistance from Claude Code (Claude Opus).