feat: built-in provider adapters for Webhook and GitHub#691
feat: built-in provider adapters for Webhook and GitHub#6912witstudios merged 2 commits intomasterfrom
Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
No actionable comments were generated in the recent review. 🎉 📝 WalkthroughWalkthroughThe changes introduce two new built-in provider adapters (Generic Webhook and GitHub) with comprehensive test coverage, establish a centralized provider registry system with lookup utilities, and enhance transform output to support dot-notation nested property access. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 5 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts (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.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/lib/src/integrations/providers/generic-webhook.ts`:
- Around line 62-70: The path interpolation currently used by pathTemplate
'/{path}' (see interpolatePath) does not URL-encode special characters so query
strings pass through literally; add automated tests for send_get_webhook that
assert values like path: 'status?key=value' produce '/status?key=value' and also
add tests that demonstrate unencoded characters (spaces, &, #) to lock in
current behavior; then update the generic-webhook.ts documentation (or the
send_get_webhook docstring) to state that interpolatePath does not encode path
values and callers must pre-encode if they expect percent-encoding, or
alternatively modify interpolatePath to perform encoding for all characters
except the query delimiter if you prefer server-side encoding (pick one approach
and document the chosen behavior).
🧹 Nitpick comments (3)
packages/lib/src/integrations/providers/github.ts (1)
151-162:labelsmapping passes through raw GitHub label objects.The
labels: 'labels'mapping (Line 158) will include the full label objects from the GitHub API (each withid,node_id,url,name,color,default,description). This could be verbose for an AI agent. Consider mapping to a more concise representation if the transform engine supports it, or document that downstream consumers should expect full label objects.packages/lib/src/integrations/execution/transform-output.ts (1)
64-66: Source keys containing literal dots cannot be accessed.
resolveSourceKeyalways interprets.as a path separator, so a source key like"user.login"(literal key with a dot) would be traversed assource["user"]["login"]instead ofsource["user.login"]. This is likely fine for API response data (GitHub, webhooks) but worth documenting in the JSDoc if other providers might have dotted literal keys.packages/lib/src/integrations/providers/index.ts (1)
1-28: Clean registry module — looks good.The code is well-structured: typed without
any, concise, and the?? nullfallback ingetBuiltinProvideris a nice touch over returningundefined. Re-exports are tidy.One optional nit per the coding guidelines:
builtinProvidersandbuiltinProviderListare module-level constants — the guideline calls forUPPER_SNAKE_CASEfor constants (e.g.,BUILTIN_PROVIDERS,BUILTIN_PROVIDER_LIST). That said, this is a grey area for non-primitive constants and many TS codebases reserve UPPER_SNAKE_CASE for true compile-time constants, so feel free to keep as-is if that's the team convention. As per coding guidelines, "Use UPPER_SNAKE_CASE for constants".
| execution: { | ||
| type: 'http', | ||
| config: { | ||
| method: 'POST', | ||
| pathTemplate: '/{path}', | ||
| bodyTemplate: { $param: 'body' }, | ||
| bodyEncoding: 'json', | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find interpolatePath implementation
ast-grep --pattern $'export const interpolatePath = ($$$) => {
$$$
}'Repository: 2witstudios/PageSpace
Length of output: 47
🏁 Script executed:
# Search for interpolatePath and URL encoding logic
rg -n 'interpolatePath|encodeURIComponent|encodeURI' --type=ts -C3Repository: 2witstudios/PageSpace
Length of output: 30276
🏁 Script executed:
# Also check if there are integration tests for the webhook provider
fd -e test.ts -e spec.ts | xargs grep -l "interpolatePath|generic-webhook" 2>/dev/null | head -5Repository: 2witstudios/PageSpace
Length of output: 47
🏁 Script executed:
# View the interpolatePath implementation
sed -n '14,40p' packages/lib/src/integrations/execution/build-request.tsRepository: 2witstudios/PageSpace
Length of output: 725
🏁 Script executed:
# View the build-request.test.ts file to see all interpolatePath test cases
cat -n packages/lib/src/integrations/execution/build-request.test.ts | head -100Repository: 2witstudios/PageSpace
Length of output: 3823
🏁 Script executed:
# Check buildHttpRequest to see how the final URL is constructed
sed -n '130,160p' packages/lib/src/integrations/execution/build-request.tsRepository: 2witstudios/PageSpace
Length of output: 1012
🏁 Script executed:
# Check generic-webhook.test.ts for path handling tests
cat -n packages/lib/src/integrations/providers/generic-webhook.test.ts | grep -A 10 -B 5 "path\|query"Repository: 2witstudios/PageSpace
Length of output: 3442
🏁 Script executed:
# View the full generic-webhook.test.ts to see how paths are tested
wc -l packages/lib/src/integrations/providers/generic-webhook.test.tsRepository: 2witstudios/PageSpace
Length of output: 132
🏁 Script executed:
# Check if there are any tests with special characters in path values
rg -n "path.*\?" packages/lib/src/integrations/providers/generic-webhook.test.tsRepository: 2witstudios/PageSpace
Length of output: 47
🏁 Script executed:
# Let's check the actual generic-webhook.ts file to see the complete definition
cat -n packages/lib/src/integrations/providers/generic-webhook.ts | head -150Repository: 2witstudios/PageSpace
Length of output: 4077
🏁 Script executed:
# Check if there are any other URL building utilities or encoding helpers
rg -n "encodeURIComponent|encodeURI" packages/lib/src/integrations/ --type=tsRepository: 2witstudios/PageSpace
Length of output: 47
🏁 Script executed:
# Verify what happens when URL is constructed with path containing query string
# by checking if there are integration tests that cover this
rg -n "query|searchParams" packages/lib/src/integrations/execution/ --type=ts -A 3Repository: 2witstudios/PageSpace
Length of output: 3332
🏁 Script executed:
# View more of the build-request.test.ts to see all queryParams tests
sed -n '215,260p' packages/lib/src/integrations/execution/build-request.test.tsRepository: 2witstudios/PageSpace
Length of output: 1477
🏁 Script executed:
# Check if generic-webhook has any test with query string in path
cat -n packages/lib/src/integrations/providers/generic-webhook.test.ts | grep -A 5 "query\|?"Repository: 2witstudios/PageSpace
Length of output: 47
Clarify path interpolation behavior and add test coverage for query strings.
The pathTemplate: '/{path}' pattern works correctly with query strings—the ? character is not URL-encoded and will pass through literally (e.g., path: 'status?key=value' produces /status?key=value). However, interpolatePath applies no URL encoding to any special characters in the path value. While this permits query strings by design, it means other special characters (spaces, &, #, etc.) are not encoded either.
Add test coverage for the documented behavior where path "can include query string" (line 82 of send_get_webhook), and consider documenting whether callers are expected to pre-encode path values or if encoding should be applied at the interpolation layer.
🤖 Prompt for AI Agents
In `@packages/lib/src/integrations/providers/generic-webhook.ts` around lines 62 -
70, The path interpolation currently used by pathTemplate '/{path}' (see
interpolatePath) does not URL-encode special characters so query strings pass
through literally; add automated tests for send_get_webhook that assert values
like path: 'status?key=value' produce '/status?key=value' and also add tests
that demonstrate unencoded characters (spaces, &, #) to lock in current
behavior; then update the generic-webhook.ts documentation (or the
send_get_webhook docstring) to state that interpolatePath does not encode path
values and callers must pre-encode if they expect percent-encoding, or
alternatively modify interpolatePath to perform encoding for all characters
except the query delimiter if you prefer server-side encoding (pick one approach
and document the chosen behavior).
…ests The URL API's pathname setter percent-encodes ?, #, and spaces — so query strings in the path param are NOT treated as query delimiters. Updated the send_get_webhook description to remove the misleading "can include query string" claim, added module-level documentation of encoding behavior, and added 6 tests locking in the exact encoding for ?, &, #, spaces, and slashes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
send_webhook(JSON POST),send_get_webhook(GET),send_form_webhook(form POST) — closes [Providers] Task 30: Generic webhook adapter #612list_repos,get_issues,create_issue,create_pr_comment,get_pull_request,list_pull_requests— closes [Providers] Task 27: Built-in GitHub adapter #609builtinProviders,getBuiltinProvider,isBuiltinProvider)applyMapping()to support dot-notation source keys (e.g.user.login) for nested property access in output transformsintegrations/index.tsAdapters are static
IntegrationProviderConfigobjects — pure data. The execution engine (executeToolSaga) handles all runtime: auth, request building, HTTP execution, retry, rate limiting, output transformation, and audit logging.Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests