feat(connector): Stripe, Okta, Shopify, Mailchimp, OneDrive, SharePoint, QuickBooks, and RabbitMQ connectors#147
Conversation
…ks, and RabbitMQ connectors Adds 17 new connector actions across 7 services: - stripe/create_charge, stripe/create_customer, stripe/create_refund - okta/list_users, okta/create_user - shopify/list_orders, shopify/list_products, shopify/create_order - mailchimp/list_members, mailchimp/add_member - onedrive/upload, sharepoint/list_items - quickbooks/create_invoice, quickbooks/list_invoices - rabbitmq/publish, rabbitmq/consume Also adds parseJSONBody shared helper to tools.go and github.com/rabbitmq/amqp091-go v1.11.0 as the AMQP protocol client. Closes #94, #108, #109, #111, #112, #114, #115 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 48 minutes and 14 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
📝 WalkthroughWalkthroughThis PR adds eight new connector modules that bridge the engine with popular SaaS platforms—Stripe, Okta, QuickBooks, RabbitMQ, OneDrive/SharePoint, Shopify, and Mailchimp—each providing multiple API actions with consistent patterns for credential extraction, HTTP request handling, error wrapping, and response parsing. ChangesSaaS Connector Integrations
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7403cbd9a7
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- OneDrive: remove url.PathEscape on filePath to preserve literal "/" separators in Graph API path templates (escaping produced %2F which resolved to a file literally named "documents%2Fhello.txt") - RabbitMQ: ack each message inside the consume loop when auto_ack=false so messages are not requeued when the channel closes on return Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
packages/engine/internal/connector/shopify.go (1)
16-26: ⚡ Quick winConsolidate the triplicated
apiURL/baseURLlogic.All three connectors carry an identical
baseURLfield and a byte-for-byte identicalapiURLmethod. Extract a single package-level helper to remove the duplication and keep the base-URL/API-version logic in one place as more Shopify actions are added.♻️ Proposed shared helper
+func shopifyAPIURL(baseURL, shop, path string) string { + if baseURL != "" { + return baseURL + path + } + return fmt.Sprintf("https://%s.myshopify.com/admin/api/%s%s", shop, shopifyAPIVersion, path) +}Then drop the three
apiURLmethods and callshopifyAPIURL(c.baseURL, shop, "/orders.json")directly. I avoided struct embedding here since promoted fields would break any&ShopifyListOrdersConnector{Client: ...}composite-literal construction in the registry.Also applies to: 60-71, 105-116
🤖 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/engine/internal/connector/shopify.go` around lines 16 - 26, Multiple Shopify connectors (e.g., ShopifyListOrdersConnector and the two other connectors with identical baseURL fields and apiURL methods) duplicate the same base-URL / API-version logic; add a single package-level helper function shopifyAPIURL(baseURL string, shop string, path string) that returns baseURL+path when baseURL != "" else fmt.Sprintf("https://%s.myshopify.com/admin/api/%s%s", shop, shopifyAPIVersion, path), then remove the duplicate apiURL methods from the connector types and replace their calls with shopifyAPIURL(c.baseURL, shop, path) (leave the structs with their baseURL fields intact so existing composite literals like &ShopifyListOrdersConnector{...} keep working).
🤖 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.
Inline comments:
In `@packages/engine/go.mod`:
- Line 129: The dependency github.com/rabbitmq/amqp091-go is currently marked as
// indirect in the module's go.mod but is directly imported (as amqp
"github.com/rabbitmq/amqp091-go") in internal/connector/rabbitmq.go; update the
module's go.mod to move github.com/rabbitmq/amqp091-go v1.11.0 from an indirect
entry to a direct require entry (i.e., add or change it under require without
the // indirect comment) so the dependency is explicit and versioned for builds.
In `@packages/engine/internal/connector/onedrive_test.go`:
- Around line 11-25: The test HTTP handler (srv created with http.HandlerFunc)
currently ignores r.URL.Path so it doesn't verify nested uploads; update the
handler to assert r.URL.Path matches the expected nested path (e.g.,
"/drive/root:/documents/hello.txt:/content" or whatever the Upload path produced
by the onedrive connector is) to catch slash-escaping bugs introduced by
url.PathEscape in onedrive.go; modify the check inside the handler that
currently inspects r.Method and Authorization to also compare r.URL.Path and
fail the test (t.Errorf) if it differs.
In `@packages/engine/internal/connector/onedrive.go`:
- Around line 44-50: The code builds the OneDrive endpoint using
url.PathEscape(filePath) which escapes slashes and breaks nested paths; update
the endpoint construction in the block that sets endpoint (the branch using
params["drive_id"] and the else branch) to escape each path segment instead of
the whole filePath: split filePath by "/" (use strings.Split), apply
url.PathEscape to each segment, then rejoin with "/" (strings.Join) and pass
that escapedPath into c.apiURL(fmt.Sprintf(..., url.PathEscape(driveID),
escapedPath)) (and similarly for the "/me/drive" branch); add the strings import
if missing.
In `@packages/engine/internal/connector/quickbooks.go`:
- Around line 75-89: The code builds a QuickBooks query by concatenating
params["where"] and params["order_by"] directly into query (see variables query,
params, where, orderBy, and helper extractInt), which allows clause injection;
fix by validating or sanitizing those inputs before concatenation: enforce a
strict whitelist/regex for allowed characters and keywords (e.g., alphanumeric,
dots, spaces, comparison operators and boolean keywords) for where and order_by,
reject or return an error for values that don't match, and/or escape any
disallowed tokens so they cannot inject MAXRESULTS/STARTPOSITION or other
clauses; keep the existing max_results and start_position handling (maxResults,
startPos) but only append where/order_by after validation.
In `@packages/engine/internal/connector/rabbitmq_test.go`:
- Around line 31-51: The tests fail to hit validation because Execute currently
calls amqp.Dial before checking inputs; update the Execute implementations
(e.g., RabbitMQPublishConnector.Execute and RabbitMQConsumeConnector.Execute in
rabbitmq.go) to perform input validation first (ensure publish checks for either
exchange or routing_key and non-empty body; consume checks for queue) and return
a validation error before attempting amqp.Dial/connection; only if validation
passes, proceed to dial and the rest of the logic.
In `@packages/engine/internal/connector/rabbitmq.go`:
- Around line 72-93: The current code defaults params["auto_ack"] -> autoAck to
true which causes at-most-once silent loss; change the default to false (autoAck
:= false) and make autoAck opt-in, then stop auto-acking on Get and instead
perform explicit acknowledgements (use ch.Ack with msg.DeliveryTag) only after
the step is durably checkpointed; if you must preserve the opt-in behavior keep
the params["auto_ack"] override but ensure the normal path uses explicit Ack
after checkpointing (and document that enabling auto_ack opts into at-most-once
delivery).
- Around line 14-31: Move parameter validation before any network dial so
invalid requests are rejected without opening connections: in
RabbitMQPublishConnector.Execute, validate params["exchange"] (or routing_key)
and params["body"] before calling newRabbitMQChannel so the function returns
validation errors first; similarly, in RabbitMQConsumeConnector.Execute validate
params["queue"] before calling newRabbitMQChannel. Keep the same error
messages/formatting when returning fmt.Errorf and only call newRabbitMQChannel
after all required parameter checks pass.
- Around line 123-126: The current call to amqp.Dial(amqpURL) cannot be canceled
via the request ctx; change the connection logic in the RabbitMQ connector so
dialing runs where ctx is available and supply a custom Dial function in
amqp.DialConfig.Config.Dial that uses a net.Dialer with DialContext bound to the
incoming ctx (or applies ctx-aware deadlines). Specifically, replace the direct
amqp.Dial call inside the connector with amqp.DialConfig(amqpURL,
amqp.Config{Dial: func(network, addr string) (net.Conn, error) { return
(&net.Dialer{}).DialContext(ctx, network, addr) }}) (or equivalent closure that
captures ctx) so the dial respects cancellation; ensure the function/method that
provides ctx (the surrounding connector function) passes that ctx into the
DialConfig closure and handle/propagate any dial errors as before.
---
Nitpick comments:
In `@packages/engine/internal/connector/shopify.go`:
- Around line 16-26: Multiple Shopify connectors (e.g.,
ShopifyListOrdersConnector and the two other connectors with identical baseURL
fields and apiURL methods) duplicate the same base-URL / API-version logic; add
a single package-level helper function shopifyAPIURL(baseURL string, shop
string, path string) that returns baseURL+path when baseURL != "" else
fmt.Sprintf("https://%s.myshopify.com/admin/api/%s%s", shop, shopifyAPIVersion,
path), then remove the duplicate apiURL methods from the connector types and
replace their calls with shopifyAPIURL(c.baseURL, shop, path) (leave the structs
with their baseURL fields intact so existing composite literals like
&ShopifyListOrdersConnector{...} keep working).
🪄 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: fea18759-17bc-4ed8-bbf6-0d8c1d15f0c4
⛔ Files ignored due to path filters (1)
packages/engine/go.sumis excluded by!**/*.sum
📒 Files selected for processing (17)
packages/engine/go.modpackages/engine/internal/connector/connector.gopackages/engine/internal/connector/mailchimp.gopackages/engine/internal/connector/mailchimp_test.gopackages/engine/internal/connector/okta.gopackages/engine/internal/connector/okta_test.gopackages/engine/internal/connector/onedrive.gopackages/engine/internal/connector/onedrive_test.gopackages/engine/internal/connector/quickbooks.gopackages/engine/internal/connector/quickbooks_test.gopackages/engine/internal/connector/rabbitmq.gopackages/engine/internal/connector/rabbitmq_test.gopackages/engine/internal/connector/shopify.gopackages/engine/internal/connector/shopify_test.gopackages/engine/internal/connector/stripe.gopackages/engine/internal/connector/stripe_test.gopackages/engine/internal/connector/tools.go
- rabbitmq: validate params before dialing (exchange/routing_key/body for publish, queue for consume) so invalid requests fail without a connection - rabbitmq: use amqp.DialConfig with net.Dialer.DialContext so the dial respects context cancellation - rabbitmq: default auto_ack=false (at-least-once); true opts into at-most-once - onedrive: add escapePath helper to encode individual path segments without escaping "/" separators, so nested folders work with Graph path addressing - onedrive_test: assert request URL path to catch future slash-escaping regressions - quickbooks: reject where/order_by values that contain structural keywords (maxresults, startposition, orderby) to prevent clause injection - shopify: extract shopifyAPIURL package-level helper, removing three identical apiURL methods - go.mod: go mod tidy promotes amqp091-go from indirect to direct Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Keep patched versions of okta.go and mailchimp.go that include the Codex fixes (activate param and upsert endpoint) over the originals that merged from PR #147. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Adds 17 new connector actions across 7 services:
q,filter,limit; SSWS token authprofilemap; optionalcredentials,group_ids,activatestatus,limit,since_idlimit,product_type,vendorline_items;X-Shopify-Access-Tokenauthmerge_fields,tags,status/me/driveor a specific drive IDexpand=fields; optional$top,$filterLineitems; Bearer token + realm_idwhere,order_by,max_resultsmax_messagesfrom queue (non-blocking poll)Also adds
parseJSONBodyshared helper totools.goandgithub.com/rabbitmq/amqp091-go v1.11.0as the AMQP client.All HTTP connectors use
httptestservers for testing. RabbitMQ tests cover validation paths only (no live server needed).Test plan
go test ./internal/connector/... -run "TestStripe|TestOkta|TestShopify|TestMailchimp|TestOneDrive|TestSharePoint|TestQuickBooks|TestRabbitMQ|TestRegistry")go build ./cmd/mantlesucceedsCloses #94, #108, #109, #111, #112, #114, #115
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests
Chores