Add Alipay payment integration skill#635
Add Alipay payment integration skill#635leilei926524-tech wants to merge 1 commit intonexu-io:mainfrom
Conversation
Add a new Alipay payment integration skill, wire it into the daemon skill registry and web localization coverage, and update the English README catalog counts. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Hi @leilei926524-tech! 🎉 Thanks for making open-design better! |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 94d466e4f6
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| # - 去除空白+// 及其后的内容(TS/JS 行内注释,避免误删 URL 中的 https://) | ||
| # - 去除行首 // 开头的整行(TS/JS 行注释) | ||
| # - 去除行首 # 开头的整行(Python 行注释,不删行中 # 避免 URL 中的 # 被误删) | ||
| if sed 's|[[:space:]]//.*||g; s|^//.*||g; /^[[:space:]]*#/d' "$file" | grep -rqE "openapi\.alipaydev\.com|openapi-sandbox\.dl\.alipaydev\.com"; then |
There was a problem hiding this comment.
Remove recursive grep flag from sandbox-domain check
The sandbox detector pipes sanitized config content into grep, but the command uses -r (--recursive). With GNU grep, recursive mode without explicit file operands searches the working directory, so this check can return a match from unrelated files (for example docs mentioning alipaydev.com) and fail deployment even when the config file itself is clean. This makes the production guard unreliable and can block valid releases; the pipe should be checked with non-recursive grep against stdin only.
Useful? React with 👍 / 👎.
lefarcen
left a comment
There was a problem hiding this comment.
Hey @leilei926524-tech! Excellent work on the Alipay payment integration skill — this is comprehensive, production-ready, and shows deep familiarity with Alipay's security constraints and integration gotchas. 🚀
Overall assessment: The skill is structurally sound, reference material is thorough, and the security-first discipline (sandbox check script, explicit non-negotiable rules, verification checklist) is exactly what a payment integration skill needs. Test coverage for skill discovery + reference injection looks solid.
A few observations inline to polish the edges before merge — mostly about strengthening references, clarifying edge cases, and ensuring examples match the constraints stated in SKILL.md.
|
|
||
| When the user's request falls outside the supported set, say so plainly and point them to the official open platform docs instead of inventing an unsupported flow. | ||
|
|
||
| ## Before editing code |
There was a problem hiding this comment.
P2 Scope boundary: The skill defines what's NOT supported (App pay, JSAPI, QR flows, etc.), which is great. But it doesn't explicitly address merchant onboarding prerequisites — does the user need a verified merchant account before using this skill, or is the sandbox-first flow enough for full dev→prod path? The sandbox-check.sh script mentions alipay-merchant-onboarding skill (line 51-55) — should SKILL.md cross-reference that as a prerequisite for production, or is it optional? Clarify the prod-readiness boundary.
| - Use the sandbox gateway only for development. Add a build or deploy guard based on `references/scripts/sandbox-check.sh` so sandbox config cannot ship to production by accident. | ||
| - Treat the front-channel return page as non-authoritative. Final payment success must come from verified async notify handling or a trade query fallback. | ||
| - Async notifications must be verified, business fields must be checked, and the handler must be idempotent. | ||
| - Do not invent sandbox credentials, merchant IDs, keys, or API responses. If required values are missing, stop and tell the user exactly what must be provided. |
There was a problem hiding this comment.
P3 Nit: "Do not invent sandbox credentials..." — consider rephrasing to "Never fabricate or guess sandbox credentials..." for stronger signal. "Invent" can sound playful; "fabricate" makes it unambiguous that it's a violation.
| 工具返回环境信息后,**必须执行逐项强校验**,校验通过方可进入后续步骤。未通过则**阻断流程**,要求重新创建。 | ||
|
|
||
| #### 必含字段清单 | ||
|
|
There was a problem hiding this comment.
P2 Validation gap: The strong validation section is excellent — it lists required fields (appId, appPrivatePkcsKey, etc.) but doesn't specify what constitutes a valid value. For example:
- appId: should it match a regex like
/^\d{16}$/? - appPrivatePkcsKey: should it start with
-----BEGIN RSA PRIVATE KEY-----or a specific length threshold?
Without format validation, an LLM might pass validation with placeholder strings like "<your_app_id>" or truncated keys. Add format patterns or at minimum string-length checks (e.g., private key > 800 chars) to the validation checklist.
|
|
||
| // 配置参数 | ||
| const config = { | ||
| // 应用ID(沙箱返回的 appId) |
There was a problem hiding this comment.
P2 Security smell: The Node.js example has placeholder strings like '沙箱应用ID' and '应用私钥内容' directly in the config object. This example will be copy-pasted verbatim by users unfamiliar with the stack.
Risk: If an LLM generates code from this template without replacing placeholders, it might slip through local testing (sandbox accepts some malformed requests with 40x errors that don't crash) and only fail in production.
Fix: Add inline comments that say // TODO: Replace with actual appId from sandbox creation (16 digits) and show what the filled version looks like in a code comment, e.g.:
// appId: '9021000135790224', // Example from sandbox — use your own
appId: '沙箱应用ID', // ⚠️ REPLACE THISSame for Python example.
| ## 二、异步通知校验 | ||
|
|
||
| | 校验项 | 校验要求 | 说明 | | ||
| |----------|----------|----------| |
There was a problem hiding this comment.
P2 Async notify checklist incomplete: Line 20 says "关键信息校验: 检验 out_trade_no、total_amount、app_id、seller_id等" but doesn't specify how to validate them. For example:
- out_trade_no: compare against the merchant's order DB?
- total_amount: exact string match or numeric comparison?
- app_id: must match the SDK config's appId?
- seller_id: where does the merchant get the expected value?
Add a short "验证逻辑" sub-column or footnote explaining the validation pattern, e.g.:
out_trade_no: 必须在商户订单库中存在且状态为待支付
total_amount: 必须与订单库中的金额严格相等(字符串或数值)
app_id: 必须与 SDK 配置的 appId 一致
seller_id: 必须与商户 PID 一致(从沙箱配置或开放平台获取)
Without this, an LLM will acknowledge the checklist but won't know how to implement the checks.
|
|
||
| <forbidden> | ||
| - 跳过或绕过沙箱检测 | ||
| - 修改或删除本检测脚本 |
There was a problem hiding this comment.
P3 UX polish: The error message at line 51 references alipay-merchant-onboarding skill and provides trigger phrases like "我要入驻". This is good, but:
- The skill
alipay-merchant-onboardingis not present in this PR — is it a planned follow-up, or does it already exist in the open-design repo? - If it doesn't exist yet, this error message will confuse users (they'll try the trigger phrase and nothing happens).
Fix: Either (a) add a TODO comment in the script saying "TODO: add alipay-merchant-onboarding skill" if it's not implemented, or (b) provide a fallback instruction like "请查阅支付宝开放平台商家入驻文档" with a link.
mrcfps
left a comment
There was a problem hiding this comment.
@leilei926524-tech thanks for adding the Alipay payment integration skill and the focused discovery/i18n coverage. I found a few main-path issues in the new payment references that can cause agents to validate the wrong credential set or generate Coze-specific commands/guards in regular Open Design projects, so I’m requesting changes before merge. 🙏
Generated by Looper 0.6.1 · runner=reviewer · agent=opencode| |---|---| | ||
| | `appId` | 沙箱应用 ID | | ||
| | `appPrivatePkcsKey` | PKCS#1 格式应用私钥 | | ||
| | `appPublicKey` | 应用公钥 | |
There was a problem hiding this comment.
This required-field list validates appPublicKey but omits alipayPublicKey. That matters because the rest of the new skill tells agents to initialize SDK clients and verify notifications with alipayPublicKey (SKILL.md lists it in the sandbox setup, and sdk-config-examples.md fills alipayPublicKey into both Node/Python configs). An agent can therefore pass this “强校验” with only the app public key and then produce an integration that cannot verify Alipay signatures or initialize the SDK correctly. Please add alipayPublicKey as a required field here (and in the line 14 summary) and distinguish it from appPublicKey so the validation blocks the wrong key material.
| **重要:输出时必须完整展示所有字段,不允许省略 privateKey、publicKey 等长字段。如果输出被截断,需重新输出完整 JSON。** | ||
|
|
||
| ```bash | ||
| PLATFORM_ID=$COZE_PROJECT_ID PLATFORM=COZE PRODUCT="<PRODUCT>" alipay-cli mcp call alipay-anonymous-sandbox.createAnonymousSandbox --data '{"request":{"appType":"PUBLICAPP"}}' |
There was a problem hiding this comment.
The default sandbox creation command is hard-wired to Coze by reading $COZE_PROJECT_ID and forcing PLATFORM=COZE. In a normal Open Design run there is no documented COZE_PROJECT_ID runtime contract, so this main sandbox-first path can run with an empty PLATFORM_ID or fail before the user can even create credentials. Please make the command Open Design-compatible: either remove the Coze-specific envs, make PLATFORM_ID an explicit user-provided value with a safe fallback, or document a generic env contract that this repository actually provides.
| # ============================================================ | ||
|
|
||
| # 沙箱环境检测(非 DEV 环境才检测) | ||
| if [ "${COZE_PROJECT_ENV:-}" != "DEV" ]; then |
There was a problem hiding this comment.
The build guard decides whether it is in development solely from COZE_PROJECT_ENV. For Open Design users integrating this script into ordinary Node/Python projects, that variable will usually be unset even during local development, so the script treats every local build as production and then fails on a missing config file or sandbox gateway. Because SKILL.md and sdk-config-examples.md require adding this guard, this can break the primary dev flow and encourages users to delete the safety check. Please switch to an environment contract that works outside Coze (for example an explicit ALIPAY_ENV/APP_ENV or NODE_ENV check) and fail only for explicit production builds, or document the variable the generated project must set before installing the script.
Summary
skills/alipay-payment-integration/.Test plan
cd apps/daemon && ./node_modules/.bin/vitest run -c vitest.config.ts tests/skills.test.tscd apps/web && ./node_modules/.bin/vitest run -c vitest.config.ts tests/i18n/content.test.ts🤖 Generated with Claude Code