Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
leggetter
left a comment
There was a problem hiding this comment.
PR Review: feat: add openclaw-webhooks skill
Overall: Request Changes
The skill is solid in concept and security. OpenClaw is a legitimate, widely-used open-source AI agent platform with documented webhook functionality. No security vulnerabilities found. However, there are several AGENTS.md compliance issues to address.
Security Review ✅
No security vulnerabilities found. Good practices:
crypto.timingSafeEqual(JS) andhmac.compare_digest(Python) for timing-safe comparison- Graceful
timingSafeEquallength mismatch handling via try/catch - Query-string token rejection (prevents token leakage in server logs)
- Required payload field validation before processing
- Missing token/secret checks before comparison
SDK Usage ✅
OpenClaw has a TypeScript Plugin SDK (openclaw/plugin-sdk), but it's for building plugins inside the Gateway — not for verifying inbound webhooks. Since OpenClaw uses simple bearer token comparison (not HMAC signatures), there is no SDK verification method. Manual verification is the correct approach.
Issues to Fix
🔴 Dependency Versions (CRITICAL per AGENTS.md)
All three framework dependencies use outdated versions:
| Dependency | PR Uses | AGENTS.md Requires |
|---|---|---|
| Next.js | 14 |
15.x or later |
| Express | 4.18 |
4.21.x or later |
| FastAPI | >=0.100.0 |
>=0.115.x or later |
🔴 Missing Integration Updates (Required per checklist)
- README.md — OpenClaw not added to the Provider Skills table
- providers.yaml — No provider entry with documentation URLs and
testScenario
🟡 Missing Required SKILL.md Sections
Per AGENTS.md and established patterns (stripe-webhooks, github-webhooks):
- "Attribution" section — Should include the standard attribution comment block
- "Recommended: webhook-handler-patterns" section — Required for all provider skills, with absolute GitHub URLs to handler-sequence, idempotency, error-handling, and retry-logic references
🟡 Response Code Inconsistency
SKILL.md documents /hooks/agent returns 202 Accepted, but:
- Express example returns 200 ❌
- Next.js example returns 200 ❌
- FastAPI example returns 202 ✅
ℹ️ Minor
- Next.js example missing
tsconfig.json overview.mdreferences "Mapped Webhooks" (/hooks/<name>) but SKILL.md/examples don't cover this- Express example uses
jestwhile most other skills usevitest
Summary
| Category | Assessment |
|---|---|
| Useful? | ✅ Yes — real, popular platform; fills a gap |
| Accurate? | 🟡 Mostly — response code inconsistency needs fixing |
| SDK usage | ✅ Appropriate — no SDK verification method exists |
| Security | ✅ Good — timing-safe comparison, query-string rejection, input validation |
| Format compliance | 🟡 Missing Attribution and Recommended sections |
| Dependencies | 🔴 All three outdated per AGENTS.md CRITICAL guidelines |
| Integration | 🔴 README.md and providers.yaml not updated |
We have maintainer push access and will apply fixes directly to this branch. Will tag you once changes are pushed.
- Update dependency versions per AGENTS.md guidelines: express ^4.18.0 → ^4.21.0, next ^14.0.0 → ^15.0.0, react/react-dom ^18.2.0 → ^19.0.0, fastapi >=0.100.0 → >=0.115.0 - Fix agent hook response code: 200 → 202 in Express and Next.js examples to match SKILL.md documentation (FastAPI was already correct) - Add OpenClaw to README.md Provider Skills table - Add openclaw entry to providers.yaml with docs URLs and testScenario Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Hi @robinbg — thanks for this contribution! We've pushed a fix commit to your branch addressing the review feedback: Changes made:
All 27 tests pass across Express (10), Next.js (7), and FastAPI (10). Note: We initially flagged the Attribution and Recommended sections as missing — that was incorrect on our part; they were already present. Apologies for the noise there. Please review the changes and let us know if you have any questions or concerns! |
Summary
Authorization: Bearerandx-openclaw-tokenheaders),/hooks/agentand/hooks/wakeendpoint handlingSkill Structure
Key Features
Authorization: Bearerandx-openclaw-tokenTest Plan
npm test)pytest)vitest run)Made with Cursor