feat: add Stripe connector (Restricted API Key + custom MCP tools)

[yiboyasss]

Summary

Adds a Stripe MCP connector (key-based, not OAuth) covering accounts, balance, customers, charges, refunds, payment intents, invoices, subscriptions, products, and prices.

Why not OAuth

Stripe's own OAuth flow (Connect "Standard accounts") is built for platforms that aggregate many other merchants' accounts (a marketplace/SaaS model), not for a single user granting a tool access to their own account:

• Stripe's docs explicitly discourage this OAuth flow for new integrations.
• Since June 2021, a read_write-scope OAuth connection fails outright if the account is already connected to another platform.
• The token exchange authenticates with your own platform secret key via HTTP Basic Auth rather than a client_id/client_secret pair — a materially different shape than every other OAuth connector in this codebase.

Instead this uses a Restricted API Key (rk_live_.../rk_test_...), which the user generates themselves in the Stripe Dashboard scoped to only the permissions they grant. This is also what Stripe's own agent-toolkit/MCP server recommends for third-party agent integrations — same no-review self-serve bar as an OAuth App, without the marketplace-shaped restrictions above. Follows the same key-based pattern already used for AWS/Google Maps/PostHog (required_env, connected via POST /api/mcp/apps/{id}/connect).

Implementation notes

• Stripe's API takes application/x-www-form-urlencoded bodies (not JSON), including nested params via bracket notation (e.g. metadata[order_id]=6735). Added a _flatten_form_params helper since requests does not flatten nested dict/list values on its own.
• Caught during self-review before committing: requests serializes a bare Python bool as True/False in query strings, but Stripe's API only accepts lowercase true/false for boolean filters (active on products/prices) — fixed by explicitly lowercasing, with a regression test locking it in.
• Error responses are {"error": {"type", "code", "message", "param"}}; the message field is surfaced to the LLM instead of the raw envelope.

Test plan

• tests/web/tools/test_stripe_mcp.py — 35 tests covering headers, form-param flattening, error handling, and all 14 tools
• tests/alembic/test_20260818_seed_stripe_mcp_app.py — seed migration upgrade/idempotency/downgrade + registry-drift check
• Full alembic chain (alembic upgrade head) verified against a scratch sqlite db — single head, applies cleanly
• ruff check / ruff format / mypy / codespell / pre-commit all clean

[gemini-code-assist]

Code Review

This pull request introduces a built-in, key-based Stripe MCP connector, adding a database migration to seed the application, registering it in the built-in registry, and implementing the core Stripe MCP server with tools for managing accounts, customers, charges, refunds, invoices, subscriptions, products, and prices. The review feedback suggests improving parameter serialization in _flatten_form_params by explicitly converting boolean values to lowercase "true" or "false" strings to match Stripe's API expectations, along with adding a corresponding unit test to verify this behavior.

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[yiboyasss]

/gemini review

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[rogercloud]

PR Summary

This PR adds Stripe as a new key-based builtin MCP connector: one alembic-seeded catalog row, a registry entry, and a hand-rolled MCP server (stripe.py, ~526 lines) exposing 14 tools wrapping Stripe's REST API (accounts, balance, customers, charges, refunds, payment intents, invoices, subscriptions, products, prices). The user supplies a Stripe API key (intended to be a Restricted API Key) which flows through the existing encrypted user_mcpservers.env path and is injected into the connector subprocess at call time. Test coverage is substantial (35 tests, 539 lines), and the one previously bot-flagged issue (nested booleans not lowercased for Stripe's form encoding) is confirmed fixed and tested at every recursion depth.

Blocking: yes — recommended event: REQUEST_CHANGES

Round 0 — Approach Verdict: Acceptable with reservations

The design choice to use a user-generated Restricted API Key instead of Stripe's marketplace-shaped OAuth flow is sound — it matches Stripe's own agent-toolkit guidance and this codebase's existing key-based connector pattern (AWS, Google Maps, PostHog). No new tables are introduced; the connector reuses the existing encrypted secret storage and subprocess-invocation pattern used by sibling connectors, and the migration is a faithful, idempotent clone of the existing AWS seed migration. New helpers (_flatten_form_params, _idempotency_key, _extract_error_detail, _paginated_results) are appropriately scoped — _flatten_form_params in particular is genuinely necessary (no stdlib/library equivalent for Stripe's bracket-notation form encoding) and not over-engineered.

Two design-level reservations, elaborated below as line-level findings:

• Data minimization (M2): unlike every sibling connector (aws.py, intercom.py), 12 of 14 tools return Stripe's raw response object unprojected, including payment-PII-adjacent fields.
• Safety-guidance placement (N1): the "use a Restricted Key, not a full secret key" recommendation exists only as a source comment, never surfaced to the end user in the actual connect UI, and nothing server-side discourages a full sk_live_ key from being pasted in.

Architectural coherence is otherwise good: nothing scattered, nothing reimplemented that already exists elsewhere in the codebase.

Line-Level Findings

Major

M1 — src/xagent/web/tools/mcp/stripe.py:64-76 (also used at :126-135)
Purely content-derived idempotency key causes silent false-dedup of legitimate distinct calls. _idempotency_key computes sha256(method + path + form_data) with no nonce or timestamp, and is sent as Stripe's Idempotency-Key header on stripe_create_customer and stripe_create_refund. Stripe caches idempotency keys for ~24h and replays the original response for any later request with an identical key. Two genuinely distinct, intentional calls with identical arguments within that window (e.g. stripe_create_customer(name="Acme") for two different real Acme entities) will silently return the same object as the first call, status: success, with no distinguishing signal — the code never checks Stripe's Idempotent-Replayed response header. This is a deliberate tradeoff per the code's own comment (dedupe accidental agent retries), but the cost is real, untested, and unmitigated.
Suggested fix: check the Idempotent-Replayed response header and surface a warning to the caller, or mix a caller-supplied/generated nonce into the key for create operations.

M2 — src/xagent/web/tools/mcp/stripe.py (12 of 14 tools, e.g. customer=result in stripe_create_customer, charge=result in stripe_get_charge, charges=charges in stripe_list_charges)
Raw Stripe objects — including payment-PII-adjacent fields — are returned unprojected to the LLM, breaking this codebase's established convention. Only stripe_get_account_info and stripe_get_balance project their response to a safe field subset; the other 12 tools return Stripe's raw response object/array directly. A real Stripe charge object includes billing_details (name/email/phone/address) and payment_method_details.card (brand, last4, country, expiry). aws.py projects every response explicitly (e.g. aws_get_caller_identity returns only account/arn/user_id), and intercom.py routes everything through summary helpers with zero raw passthrough — this connector is the outlier. Since the querying user already owns the Stripe account and has dashboard access to the same data, this is not a cross-tenant authorization vulnerability, but it is a real data-minimization / LLM-context-hygiene gap (unnecessary PII flowing into logs, model-provider context, and token usage).
Suggested fix: add projection helpers for at least the charge/customer/invoice-detail tools, matching the pattern already used for stripe_get_account_info/stripe_get_balance.

M3 — src/xagent/web/tools/mcp/stripe.py:511-514 (mirrors :477-480); tests at tests/web/tools/test_stripe_mcp.py:491-518
stripe_list_prices's boolean active filter has no test coverage, reproducing exactly the bug class this PR's own description says was "caught and locked in." stripe_list_products and stripe_list_prices independently lowercase the active filter before sending it as a query param, using identical copy-pasted logic. test_list_products_uses_active_filter and test_list_products_serializes_active_false_as_lowercase_string cover stripe_list_products; the only test touching stripe_list_prices (test_list_prices_uses_product_filter, line 518) never passes active and never asserts on it. A future edit that broke the lowercasing specifically in stripe_list_prices would pass CI undetected.
Suggested fix: add a test_list_prices_uses_active_filter test mirroring the existing products test.

Minor

N1 — src/xagent/web/builtin_mcp_registry.py (~lines 871-887)
The recommendation to use a Restricted API Key (rk_live_/rk_test_) rather than a full secret key exists only as a Python comment — the stored/rendered description field never mentions it, and the connect dialog renders a generic password input with a static, connector-agnostic hint. The backend connect handler performs zero prefix/format validation, so nothing rejects a pasted sk_live_/sk_test_ (full access) key. By contrast, aws.py enforces read-only access server-side via an explicit STS session policy rather than relying on user diligence. This doesn't mishandle the key once given (storage/encryption/usage are all correct), but it's a real, fixable UX gap.
Suggested fix: update the user-facing description text to recommend a restricted key by name, and consider a soft (non-blocking) warning if a pasted key starts with sk_.

N2 — src/xagent/web/tools/mcp/stripe.py:40-47
_headers() sends only Authorization; no Stripe-Version header is pinned, unlike intercom.py which pins INTERCOM_API_VERSION via an Intercom-Version header. Low likelihood of triggering, but a Stripe account API version change could silently shift response shapes relied on for field-plucking and pagination.
Suggested fix: pin a known-good Stripe-Version header, consistent with intercom.py.

N3 — src/xagent/web/tools/mcp/stripe.py:239-249, 308-318, 417-427
stripe_get_customer, stripe_get_charge, and stripe_get_invoice build request URLs via _path_segment(id) with no check that the id is non-empty; _path_segment("") returns "", so an empty id produces /customers/ (a collection URL) rather than a clean 404. stripe_create_refund (:340-341) already guards against this same case.
Suggested fix: add the same empty/missing-id guard used in stripe_create_refund to these three tools.

N4 — src/xagent/web/tools/mcp/stripe.py:168-170
_paginated_results's defensive truncation slice (data[:limit]) is untested — no test mocks a Stripe response returning more items than the requested limit. Legitimate but low-risk coverage gap.
Suggested fix: add a test asserting the slice truncates correctly when Stripe returns more rows than limit.

Body-only informational notes (no inline comment)

• N5 — stripe_create_refund moves real money with no user-confirmation gate, but this is a pre-existing, house-wide limitation: the waiting_for_user confirmation protocol requires a top-level status key that the stdio MCP adapter never produces, making this gate structurally unreachable from any current MCP connector, not just this one. Out of scope for this PR; noted for awareness only.
• N6 — Tool surface is asymmetric: customers/charges/invoices get both list_* and get_* tools, while payment_intents/subscriptions/products/prices get list_* only, so an agent surfacing a sub_.../pi_... id from a list has no dedicated detail tool. Not a bug — worth a deliberate note if intentional for v1.

Simplification Opportunities

• stripe.py:98-99, 477-480, 511-514 — the exact "true" if value else "false" lowercase-boolean expression is written three times (once in _flatten_form_params's recursive bool branch, twice hand-inlined identically — including a duplicated 2-line comment — for the active filter in stripe_list_products/stripe_list_prices). Extract a _bool_str(value) helper and reuse it in all three places (~4 lines saved).
• stripe.py:163-164 — the 204/empty-body branch in _request is currently unreachable (no tool issues DELETE) and untested. Not urgent, just flagged so it isn't mistaken for exercised code.
• stripe.py:95-97 — latent (currently unreachable) bug adjacent to the simplification above: _flatten_form_params's list-flattening drops a None list element without renumbering remaining indices (e.g. {'items': [1, True, None, 'x']} → indices 0,1,3, skipping 2), producing a non-contiguous bracket-index sequence Stripe's parser may reject. No tool currently passes a list into form_data, but the helper is generic enough to invite future use (e.g. expand params). Low priority, flag for awareness.
• stripe.py:12 — nit: truncate_error_text is imported from web/utils/graphql_errors.py, a module docstring-framed as Linear/GraphQL-specific even though the function is fully generic. Reusing it (rather than duplicating, as jira.py does) is the right call here; consider relocating the helper to a neutral module in a follow-up.
• tests/web/tools/test_stripe_mcp.py:9-18 — MockResponse doesn't define a headers attribute by default; two tests (lines 160, 175) bolt it on post-construction to exercise the 429/Retry-After path. Add self.headers: dict = {} to the constructor.

net: -4 lines possible

Blocking Status & Recommended Decision

Blocking: yes
Recommended event: REQUEST_CHANGES

• M1 [new] — content-only idempotency key silently dedups legitimate distinct create calls within Stripe's 24h window, unmitigated.
• M2 [new] — 12 of 14 tools return raw Stripe objects (including payment-PII-adjacent fields) unprojected, inconsistent with house convention.
• M3 [new] — stripe_list_prices's active boolean filter has zero test coverage, reproducing the exact bug class this PR was written to lock down.

[yiboyasss]

Thanks for the thorough review — pushed a fix commit addressing the actionable findings:

• M1 (idempotency false-dedup): fixed at the root — stripe_create_customer/stripe_create_refund now surface an idempotent_replayed flag in their response (via Idempotent-Replayed from Stripe) so a content-collision dedup is visible instead of silently returning what looks like a fresh object.
• Empty-id guard: added to all three single-resource lookups (stripe_get_customer, stripe_get_charge, stripe_get_invoice), matching stripe_create_refund's existing guard.
• stripe_list_prices active-filter test gap: added, mirroring the existing stripe_list_products coverage.
• Duplicated bool-string logic: extracted into a shared _bool_str() helper used by all three call sites (not just the one this PR's own review round had already touched).
• Untested truncation slice: added a direct unit test for _paginated_results' over-limit branch.
• Restricted Key guidance: the connector's user-facing description now names "Restricted API Key" explicitly instead of only in a source comment.

Three items I'm intentionally not fixing in this PR, tracked in #1578 instead:

• Data minimization (raw PII-adjacent fields in 12/14 tools): real gap, but fixing it properly means adding projection helpers across most of the file's tools and touches every affected test — a design-level change bigger than this review round should absorb inline.
• Soft-warning on a pasted sk_-prefixed key: the connect flow has no per-connector validation hook today (confirmed — nothing in connect_mcp_app or the frontend dialog is connector-aware beyond required_env presence), so this needs new shared infrastructure, not a Stripe-only change.
• Stripe-Version header pinning: agree this would be good hygiene, but I didn't want to hardcode a specific dated API version string without being able to verify it's currently valid — picking a stale/wrong one would actively break every request. Happy to add this if someone can confirm the version to pin, or it can ship as a fast-follow.

[rogercloud]

PR summary

This PR adds a first-class Stripe connector for a user's own account, using the existing key-based MCP connection flow instead of Stripe Connect OAuth. It seeds the Stripe catalog entry and runtime registry with a fixed stdio launch command, carries STRIPE_API_KEY through the existing encrypted per-user/shared/platform environment flow, and exposes a curated 14-tool FastMCP surface covering account/balance, customers, charges/refunds, payment intents, invoices/subscriptions, and products/prices. The connector includes two POST writes (customer creation and refunds), bounded list/retry behavior, and a fixed Stripe API origin.
Blocking: yes — recommended event: REQUEST_CHANGES

Update summary

Since the previous reviewed snapshot 75230f8ae9479ac10d0e8e420c7abd5f7afeffc9, three commits were pushed. 1cd923c4 adds empty-ID guards and regression coverage for prices, truncation, and related review fixes; 7f2f28b4 rebases the Stripe migration onto the new PostHog merge head; and ac24dcf2 propagates replay metadata on successful POSTs, adds replay tests, and tidies _request. These updates improve input guards and successful-replay observability, but the content-derived operation key and the other findings below remain in the current head.

Approach verdict

acceptable-with-reservations. The macro direction is sound: a curated local FastMCP adapter, key-based built-in registry entry, additive catalog seed, fixed Stripe origin, existing encrypted key flow, and bounded request/list behavior fit the repository's AWS/PostHog pattern better than Stripe Connect OAuth for a user's own account. The reservations are material boundary decisions rather than a rejection of the overall shape: mutating operation identity must be explicit, the advertised Restricted API Key boundary must be enforced, and migration ownership/collision handling must be safe for pre-existing catalog rows.

Prior-findings checklist

• NOT FIXED — G1/H2, content-derived idempotency false-dedup. The prior M1 review 4989589310 and inline finding 3827184797 remain applicable. The author's reply 5365644532 said that surfacing idempotent_replayed fixed M1; current code still hashes only method/path/form data and sends that key for every POST, so the reply does not resolve distinct same-argument creates or refunds. This is carried forward once below, not reported as a duplicate.
• FIXED — H1, boolean form serialization. _bool_str and recursive nested encoding now cover both boolean values; prior sources were review 4959050889, inline 3802431005, and inline 3802431014.
• FIXED — H4/M3, prices active-filter coverage. The shared helper and true/false regression cases are present; see prior review 4989589310 and inline 3827184805.
• FIXED — H7/N3, empty resource IDs. The customer, charge, and invoice guards and their tests are present; see prior inline 3827184803.
• FIXED — H8/N4, truncation test gap. The over-limit slice/flag regression is now covered; see prior inline 3827184816.
• FIXED/DROPPED — H9, boolean-helper simplification. _bool_str was extracted and the repeated conversion was removed; see prior inline 3827184823. The prior opportunity is closed, and no new simplification item is reported.
• PARTIAL / TRACKED — H5/N1, Restricted-Key user guidance and soft warning. The registry description now names a Restricted API Key at builtin_mcp_registry.py:901-904, while the remaining soft warning is intentionally tracked in open #1578, as recorded in the author's reply 5365644532. G3 below is a distinct hard-enforcement/security-boundary issue, not a duplicate of this guidance root.
• DROPPED / TRACKED — H3/M2, raw Stripe PII-adjacent objects. The author explicitly opened and tracked #1578; preserve prior inline 3827184803 and the reply 5365644532, but do not re-report this root.
• DROPPED / TRACKED — H6/N2, Stripe-Version pinning. The author said this is tracked in #1578; preserve prior inline 3827184812 and reply 5365644532, but do not re-report it.
• DROPPED — G7, live-provider/hidden-rollout concern. The repository has a visible key-based POST precedent and no concrete external mismatch was demonstrable, so this process concern is not carried as a finding.
• DROPPED / INFORMATIONAL — prior body-only N5/N6 and other non-actionable notes. The generic refund-confirmation limitation and asymmetric detail-tool observations are house-wide or pre-existing/out of scope; the mock-header note is fixed. They remain represented by prior review 4989589310 without duplicate findings.

Confirmed findings

Major

G1 — Content-derived idempotency can false-deduplicate distinct writes

[major][prior] src/xagent/web/tools/mcp/stripe.py:154

I saw the author's reply 5365644532 that surfacing idempotent_replayed fixed M1. I re-checked the current implementation: _idempotency_key still hashes only method/path/form data at :72-91, this path sends that key for every POST, and the customer/refund tools accept no logical-operation key. The flag populated at :186-191 is post-response observability; it cannot make a second identical customer creation or equal partial refund execute. Please accept a caller-provided or freshly generated key per logical operation and reuse it only for retries; do not require callers to mutate business metadata to obtain a new operation identity.

G2 — Replayed errors lose replay metadata

[major][new] src/xagent/web/tools/mcp/stripe.py:178

The status_code >= 400 branch raises before Idempotent-Replayed is read at :186-191. A replayed final failure is therefore returned as an ordinary failure, with no signal that the body/status is a cached replay and no reliable retry or reconciliation guidance. Capture the header before the error branch and propagate a bounded replay flag through the error envelope/exception; add replayed-error coverage for _request and both POST tools.

G3 — The advertised Restricted API Key boundary is not enforced

[major][new] src/xagent/web/tools/mcp/stripe.py:41

_headers checks only that STRIPE_API_KEY is non-empty, then sends any value as Bearer authentication. A user or shared/platform provider can therefore supply an sk_live_/sk_test_ full secret despite the catalog and migration promising a Restricted API Key, expanding the LLM-operated connector's account privileges beyond the stated least-privilege boundary. Enforce rk_live_/rk_test_ at this final header boundary (covering own/shared/platform env sources), reject other key classes with a clear message, and test accepted/rejected prefixes; do not rely only on the description.

G4 — Error handling can expose credential-bearing detail and bypass bounds

[major][new] src/xagent/web/tools/mcp/stripe.py:179

Structured error.message values from _extract_error_detail are returned unredacted and unbounded, while the fallback truncation covers only response.text; requests.RequestException is also not sanitized before public catches log and return str(e). A proxy exception can contain a credential-bearing proxy URL, or a gateway/response can echo credentials, allowing raw detail into logs and the LLM error envelope; this is possible leakage, not a claim that every response contains a secret. Catch/redact/bound RequestException and apply the existing redaction and truncation to both structured and fallback response detail before raising or returning it.

G5 — reason="fraudulent" hides an account-level side effect

[major][new] src/xagent/web/tools/mcp/stripe.py:383

The MCP docstring presents fraudulent as an ordinary refund reason, and the unconstrained reason value is forwarded to Stripe. For applicable card payments, Stripe documents that this fraud report can add the associated card fingerprint and payment email(s) to the account's default Radar block lists and improve fraud signals; it is not merely refund metadata. See the Stripe refund API and Radar transaction risk prevention. Disclose the side effect and require a real confirmation/guard before forwarding it, or reject/omit this reason when stdio MCP cannot represent informed confirmation; validate the reason in the function and cover the guard through FastMCP tests.

G6 — Migration ownership collision can lead to unsafe downgrade deletion

[major][new] src/xagent/migrations/versions/20260818_seed_stripe_mcp_app.py:62 (downgrade :80)

Upgrade returns for any existing app_id="stripe" without checking provenance or shape, so a pre-existing custom row is skipped and then treated by the new registry overlay as the built-in Stripe execution config. Downgrade subsequently deletes every row with that ID, including an administrator-owned row that this migration did not create. Distinguish inserted/adopted/pre-existing ownership, make the registry overlay and downgrade ownership-safe, and add collision plus upgrade/downgrade-preservation tests.

Minor

G8 — Least-privilege permission guidance is incomplete

[minor][new] src/xagent/web/builtin_mcp_registry.py:903

The description names a Restricted API Key but gives no Dashboard setup link or resource-level permission matrix. Add concise guidance for the minimum reads covering account/balance, customers, charges, payment intents, invoices, subscriptions, products, and prices, plus the Customers and Refunds write permissions, and keep the migration and runtime descriptions consistent. This is distinct from H5/N1's already-tracked key-name/soft-warning guidance.

G9 — Tests bypass FastMCP registration and schema validation

[minor][new] tests/web/tools/test_stripe_mcp.py:269

The Stripe tests call Python functions directly, so they do not prove that the 14 decorated tools are listed, that generated input schemas accept the intended arguments, or that protocol-level calls reach the mutating tools. Add an async FastMCP-level test using mcp.list_tools/mcp.call_tool to assert the public names and representative schemas and to exercise at least one mocked mutating call; retain the direct unit tests for request details.

Review execution note

No local tests, builds, linters, formatters, or network calls were run under review policy, and dependencies were not installed. The pre-flight CI rollup checks were green. Both Simplification Lens runs were unavailable due to the agent usage limit; with H9 already fixed, no new simplification finding is included.

Blocking status & recommended decision

Blocking: yes. Six confirmed major roots remain unresolved; G8 and G9 are minor and do not independently change the decision.

Recommended event: REQUEST_CHANGES

• src/xagent/web/tools/mcp/stripe.py:154 — major — content-derived idempotency can collapse distinct customer/refund operations into one replay. [prior]
• src/xagent/web/tools/mcp/stripe.py:178 — major — replayed final failures lose Idempotent-Replayed metadata before it is captured. [new]
• src/xagent/web/tools/mcp/stripe.py:41 — major — full Stripe secret keys pass through a connector advertised as Restricted-Key-only. [new]
• src/xagent/web/tools/mcp/stripe.py:179 — major — structured or network error detail can expose credential-bearing text and bypass size limits. [new]
• src/xagent/web/tools/mcp/stripe.py:383 — major — the fraudulent refund reason can trigger undocumented account-level Radar block-list side effects. [new]
• src/xagent/migrations/versions/20260818_seed_stripe_mcp_app.py:62 — major — collision skipping combined with unconditional downgrade deletion can commandeer or delete an existing catalog row. [new]

[yiboyasss]

Thanks for the second, deeper pass — you were right that the first reply didn't actually fix the idempotency root cause, just made the symptom visible. Pushed two more commits (a66cf3aa, b0c270a4) since 75230f8a.

Fixed at the root, not just the cited example:

• G1/H2 (idempotency false-dedup) — actually fixed now. Replaced the content-derived key entirely: _idempotency_key(method, path, form_data) is gone. stripe_create_customer/stripe_create_refund now take an explicit idempotency_key argument — omitted, each call gets its own fresh uuid4 key (never derived from arguments, so two distinct calls with identical args are never merged); passed and reused verbatim across a deliberate retry, Stripe dedupes as intended. No business-metadata workaround required.
• Idempotent-Replayed lost on error responses — the header is now read before the >=400 branch raises, not after, so a replayed failure keeps its replay signal ("...retrying with that same key will reproduce the same failure; use a different idempotency_key...") instead of looking like a fresh error.
• Unredacted/unbounded network exceptions — requests.RequestException is now caught around the request call, redacted with the same redact_sensitive_text helper posthog.py already uses for the identical proxy-credential-in-exception-text exposure (both share setup_proxy_env()), and length-bounded with truncate_error_text. Previously a raw ConnectionError/Timeout reached the LLM/logs unbounded and unredacted.
• Follow-on gap I found in my own fix: giving every call a fresh key by default closes the false-dedup problem but left a plain retry-after-network-error completely unprotected, with no hint in the error message. The RequestException message now includes the exact idempotency_key that was used, so an agent that decides to retry has a concrete, discoverable way to do it safely — documented in both tools' docstrings.
• H7/N3 (empty resource IDs), H8/N4 (truncation test gap), H4/M3 (prices active-filter coverage), H9 (bool-helper simplification) — confirmed still in place from the previous round, unaffected by this round's changes.
• Fraudulent refund side effect — stripe_create_refund's reason docstring now discloses that "fraudulent" reports the charge to Stripe Radar and can add the card/customer to the account's fraud block lists.
• Test-coverage gap (bypassing FastMCP's own layer) — added test_mcp_registers_all_fourteen_tools and an async mcp.call_tool test that exercises the real MCP argument-coercion path (a JSON-string metadata argument), on top of the existing direct-call unit tests.

Intentionally not fixed here — recorded, not dropped:

• Hard-rejecting non-rk_ API keys in _headers(). Real gap, but it directly conflicts with the explicit non-goal already recorded in Stripe connector: data-minimize raw tool responses + surface Restricted Key guidance in connect flow #1578 ("not proposing a hard rejection of full secret keys"). That's a product/security tradeoff, not something I want to flip unilaterally in a review-fix pass — happy to do it if that decision changes.
• Migration ownership/collision on upgrade+downgrade. Verified this is an exact, faithful copy of the same if app_id in existing: return / delete().where(app_id == APP_ID) pattern already in aws.py's seed migration (and ~9 others). Fixing only Stripe's copy would be inconsistent; this reads as a cross-cutting migration-authoring issue if it's worth addressing.
• Full per-resource permission matrix in the connector's description. Every connector's description is a one-line browse-card summary (longest existing one, Slack's, is 220 chars) with no secondary long-text field in the schema. A 9-category read/write breakdown doesn't fit that shape — this needs a schema/UI change (new field + card rendering), not a description edit, so I left it out rather than degrade the card for a bad fit.

All 71 tests pass, ruff/mypy/codespell/the alembic head-check are clean, and the full migration chain (alembic upgrade head from empty) applies cleanly at b0c270a4.

[rogercloud]

Summary

This PR adds a Stripe MCP connector using a user-supplied Restricted API Key (not OAuth), following the existing key-based connector pattern already used by aws.py/posthog.py (required_env: ["STRIPE_API_KEY"]). It exposes 14 tools (account/balance lookups, customer/charge/invoice/subscription/product/price listing and detail fetches, customer creation, and refund issuance) via a single _request choke point, plus a seed migration registering the connector in public_mcp_apps.

Update summary (since last review)

Commits 376b84a3, 2ccf6c45, 2bb41aa6, edb5432b addressed round-3 findings G1-G9. Verified outcomes below.

Round 0 design verdict

Acceptable with reservations. The engineering is above the codebase's typical bar: reuses the established key-based connector pattern, no new abstractions, _request is a single well-designed choke point, no SSRF surface (fixed BASE_URL), path-traversal-safe (quote(safe="")), and the idempotency design is now sound. The OAuth-vs-key rationale in the PR description holds up. Two reservations remain before merge — not because the approach is wrong, but because the code doesn't yet enforce its own stated security premise, and because this is the first irreversible money-moving tool in a codebase with no HITL gating for any MCP tool.

Resolved findings (verified fixed)

• G1 — idempotency false-dedup: FIXED. _generate_idempotency_key() (stripe.py:73-88) is now uuid.uuid4().hex, fresh per call by default; explicit opt-in idempotency_key param added to stripe_create_customer/stripe_create_refund for intentional retries. Covered by tests pinning both behaviors.
• G2 — replay signal lost on error: FIXED. stripe.py:205-224 reads Idempotent-Replayed before the >=400 branch raises and folds it into the raised message. Covered by tests.
• G5 — undisclosed "fraudulent" refund side effect: FIXED. stripe_create_refund's docstring (stripe.py:430-435) now discloses the Stripe Radar block-list side effect.
• G9 — tests bypassed FastMCP registration: FIXED. test_mcp_registers_all_fourteen_tools and test_create_customer_via_mcp_layer_parses_json_string_metadata now exercise mcp.list_tools()/mcp.call_tool().

Blocking findings

Major — carried forward

• G3 — Restricted-Key format not enforced in _headers(). stripe.py:41-48 only checks the key is non-empty; no rk_/rk_live_/rk_test_ prefix check, so a pasted sk_live_/sk_test_ full-access secret is silently accepted, contradicting the connector's stated security premise. See inline comment for the detailed rebuttal to the author's #1578 citation.

Major — new

• D2 — stripe_create_refund has no confirmation/approval gate, and none is structurally reachable today. See inline comment on stripe.py:465 for the full platform-gap analysis and recommended scoped mitigation.

Major — carried forward (partially fixed, unresolved half)

• G4 — unredacted/unbounded error detail on the HTTP response-body path. The network-exception path was fixed (stripe.py:180), but stripe.py:211-224 (the structured-message and raw-text-fallback branches) still never calls redact_sensitive_text. See inline comment for the concrete leak scenario.

Non-blocking notes

• G6 — migration ownership collision on upgrade/downgrade. Confirmed real (upgrade skips existing rows, downgrade unconditionally deletes by app_id), but byte-for-byte identical to the pattern already used in ~10 other seed migrations in this repo (aws, posthog, google_maps, google_ads, granola, google_analytics, google_sheets, notion, chrome). Not unique to this PR — recommend a dedicated cross-cutting follow-up issue covering all affected migrations rather than a Stripe-only fix. Does not block this PR.
• G8 — connector description lacks a permission matrix. The schema-constraint defense (no secondary long-text field on public_mcp_apps) is accurate, but the cited comparison is wrong: Slack's description is 179 chars, not "220 (longest existing)" — GitHub's is actually the longest at 275 chars. Doesn't change the conclusion; noting for the record. Non-blocking.

Minor findings (new)

F3, T4, T9, F1, F2, F7, F10, F18, T2, T3 — see inline comments for details on each.

Simplification opportunities

• List-tool helper. The 7 list tools repeat an identical _clamp_limit -> build params -> _request("GET", ...) -> _paginated_results -> _success(...) shape, differing only in path/result-key/filter kwargs. A shared _list_resource(path, result_key, max_results, **filters) helper would collapse each ~14-line body to ~3 lines with no behavior change.
• response_meta out-parameter. The mutable response_meta: dict = {} out-parameter can be replaced by having _request return (result, idempotent_replayed) as a tuple for POST calls.
• Net: roughly -40 lines possible from these two changes; not a "lean already" verdict, but not blocking either — can be a fast follow.

Two other candidate simplifications were considered and dropped: collapsing the 14 tools' try/except Exception blocks into a decorator (would make stripe.py a codebase-consistency outlier vs. every other file in src/xagent/web/tools/mcp/, which uses this same per-tool pattern), and inlining _generate_idempotency_key() (its docstring documents a deliberately-rejected alternative design that _request's own docstring cross-references, plus a dedicated unit test).

Blocking status & recommended decision

Blocking: yes.

Blocking findings:

• G3 [prior] — Restricted-Key format not enforced in _headers() (stripe.py:41-48)
• G4 [new] — unredacted error detail on HTTP response-body path (stripe.py:211-224) — unresolved half of a prior finding
• D2 [new] — no confirmation/approval gate on stripe_create_refund (stripe.py:414-478)

Non-blocking: G6, G8 (see above).

Minor (non-blocking): F3, T4, T9, F1, F2, F7, F10, F18, T2, T3.

Recommended event: REQUEST_CHANGES.

[yiboyasss]

Thanks for the third, deeper pass — pushed b2d80cd1 addressing the three blocking findings, plus a self-review round on top that caught one more gap in my own fix (92860e23).

Blocking findings — fixed at the root:

• G3 (Restricted-Key prefix not enforced). Fair rebuttal — Stripe connector: data-minimize raw tool responses + surface Restricted Key guidance in connect flow #1578 is scoped to the connect-flow UI's soft warning, not this per-request runtime path, so it didn't actually cover this. _headers() now rejects any STRIPE_API_KEY that doesn't start with rk_live_/rk_test_, at the one place this connector actually controls enforcement.
• G4 (unresolved half — HTTP response-body path still unredacted). Confirmed and fixed: the previous round's redact_sensitive_text fix only covered the network-exception path. Both HTTP-error-body sources (_extract_error_detail's structured message, and the raw-text fallback) now go through the same redact+truncate pipeline uniformly, closing the gap everywhere instead of just where you re-cited it.
• D2 (no confirmation gate on stripe_create_refund). Traced this to a genuine platform-level gap — MCPToolAdapter's fixed result shape has no status key for the HITL protocol to key off of, so no MCP tool (not just Stripe's) can reach it today, and destructiveHint/readOnlyHint are never read anywhere in core/tools/. Opened No HITL confirmation gate reachable for MCP connector tools (blocks safe gating of stripe_create_refund and other destructive actions) #1585 to track the real fix (wiring MCP results into the existing HITL protocol) rather than giving Stripe a one-off opt-in-env-var convention no other connector shares.

Minor findings — fixed:

• F3: stripe_create_refund now rejects when both charge_id and payment_intent_id are given, not just when neither is.
• F1: a non-JSON 2xx body now raises a clear error instead of a bare JSONDecodeError, matching posthog.py's existing pattern.
• F2: _paginated_results now validates Stripe's data field is actually a list before slicing it.
• F7: _flatten_form_params now rejects form-field keys containing [/]/empty string, which would otherwise collide with Stripe's own bracket-notation nesting.
• T2/T3/T4/T9: added coverage for limit-clamping, starting_after forwarding, and one more mocked-failure case each for a write tool and a list tool.

Self-review catch (not from this review round, found before you would need to re-report it): the F2 fix above had its own gap — data = payload.get("data") or [] ran before the isinstance check, so a falsy-but-invalid value (0, "", False) was silently coerced to an empty list and never actually reached the validation. Fixed by checking the type before applying any "missing means empty" fallback, with tests for both the falsy-invalid and genuinely-missing cases.

Non-blocking, left as-is (per your own notes):

• G6 (migration ownership pattern) — confirmed pre-existing across ~10 seed migrations, not fixed here.
• G8 (description length) — noted, no code change needed.
• F10 (pagination cursor) and the two simplification suggestions (list-tool helper, response_meta→tuple) — no behavior change, tracked as possible fast-follows rather than done inline here to keep this changeset focused.

90 tests pass, ruff/mypy/codespell/the alembic head-check are clean, and the full migration chain applies cleanly at 92860e23.

[rogercloud]

Summary

This PR adds a Stripe MCP connector to the built-in connector registry, authenticated with a Stripe Restricted API Key (rk_live_/rk_test_) rather than OAuth — Stripe's OAuth Connect flow is marketplace-shaped and does not fit the single-user self-access model these connectors use. It exposes 14 tools spanning accounts, balance, customers, charges, refunds, payment intents, invoices, subscriptions, products and prices, and handles Stripe's application/x-www-form-urlencoded body format (including bracket-notation nesting) via a _flatten_form_params helper plus a boolean-lowercasing fix for Stripe's true/false string requirement. Ships with a seed migration for the connector catalog row and ~1000 lines of tests.

What changed since the last review round

Two commits landed after the previous CHANGES_REQUESTED:

• b2d80cd1 — the substantive round-4 fix commit: server-side Restricted Key enforcement (rk_live_/rk_test_ prefix check, rejecting full sk_ secrets outright), complete error redaction so all three error-message sources converge on the same truncate_error_text(redact_sensitive_text(...)) call, refund guard-clause validation (rejecting both charge_id and payment_intent_id together), 2xx non-JSON-body handling, and data-field type validation in _paginated_results.
• 92860e23 — a self-caught follow-up: the or [] fallback introduced by the previous commit was itself masking falsy-invalid data values (0, "", False) from ever reaching the isinstance check. Replaced with an explicit is None check so those shapes now correctly raise.

Both were verified against the actual code at HEAD, not taken from the commit messages.

Design verdict

An independent design pass (run blind to the prior review history) rated this acceptable-with-reservations: architecturally a faithful sibling of the existing AWS/PostHog/Jira key-based connectors — registry entry, encrypted env-based secret, request/response helper pattern — with a solid, idempotent seed migration covered by a registry-drift test.

Of the reservations that pass raised: connect-time key-format validation and the inaccurate parity comment are now resolved (the key check is real and enforced server-side, and the comment now describes what the code actually does). Raw PII/card-data exposure in tool responses, no approval gate on the refund tool, and amount/reason validation remain open — the first two deliberately deferred to tracked issues, the third confirmed below as a minor hardening gap rather than a money-safety hole (Stripe hard-enforces both constraints server-side).

Prior findings

FIXED (16)

• #1 Nested/list booleans lowercased for Stripe's form encoding — _flatten_form_params recurses before the bool check; covered by test.
• #2 Content-derived idempotency key silently deduped genuinely distinct calls — now uuid.uuid4().hex, with an explicit optional idempotency_key parameter on stripe_create_customer/stripe_create_refund and a load-bearing docstring explaining why. Tested.
• #4 stripe_list_prices active filter — now tested.
• #7 Empty-string id guards on get_customer/get_charge/get_invoice — all three present.
• #8 _paginated_results truncation slice — now tested.
• #9 Duplicated boolean-string logic — extracted to _bool_str(), reused 3x.
• #15 Test helper MockResponse missing default headers — fixed.
• #17 Replayed HTTP error responses lost their Idempotent-Replayed signal — the header is now read before the >=400 branch raises, and the replay warning is folded into the raised message. Two tests.
• #18 Restricted Key format not enforced server-side — real prefix check now rejects sk_ keys with an actionable message.
• #19 Error handling could leak credential-bearing detail — verified comprehensively: the network-exception path, the structured-error path, and the raw-text fallback all converge on the same redact+truncate call.
• #20 reason="fraudulent" Radar block-list side effect undisclosed — the docstring now discloses it clearly. (The separate validation gap this surfaced is reported fresh as finding B.)
• #23 Tests bypassed FastMCP registration/schema validation — a registration test plus an async mcp.call_tool(...) test now genuinely exercise the FastMCP layer.
• #24 Refund accepted both charge_id and payment_intent_id — now rejected, with a test asserting no request is sent.
• #27 2xx success path could throw a bare JSONDecodeError — now raises a clear RuntimeError, symmetric with the error path. Tested.
• #28 _paginated_results didn't validate data is a list — fixed, including the author's own self-caught or [] regression at HEAD. All three cases (missing, invalid-truthy, invalid-falsy) tested.
• #29 Metadata keys containing [/] or empty string — rejected with a clear message. Tested.

WAIVED / TRACKED (6 + 1 new)

• #3 Raw Stripe objects (PII/card data) returned unprojected to the LLM — deferred to #1578. Still present in code as described; a known, explicit scope deferral rather than a regression.
• #5 Restricted Key recommendation not surfaced at connect time (runtime rejection only) — #1578.
• #6 No Stripe-Version pinning — no issue number; the author declined to hardcode a version they had not verified and is open to a fast-follow. Reasonable.
• #10 No approval gate on stripe_create_refund — confirmed a genuine platform-wide structural gap: MCPToolAdapter._execute_mcp_call never emits the status key that tool_result_waits_for_user() checks for, so no MCP connector in this codebase can request HITL. Tracked in #1585. Fixing this PR-locally would mean inventing an ad-hoc mechanism the platform supports nowhere else.
• #21 Seed-migration ownership collision (upgrade skips a pre-existing app_id; downgrade deletes unconditionally) — verified byte-identical in ~16 sibling seed migrations. Cross-cutting, correctly not blocked on here.
• #22 Connector description lacks a Dashboard setup link / permission matrix — the one-line card description is the uniform schema across all connectors; not a Stripe-specific gap.
• New: finding I (platform admin-key fallback, see below) — same treatment: non-blocking, but please open a tracking issue.

PARTIAL (4)

• #25 Tool exception-path coverage — now 3 of 14 tools (get_account_info, list_charges, create_refund). This matches the scope the author stated ("a write tool and a list tool"), but 11 tools' identical except Exception blocks remain untested.
• #26 truncated flag end-to-end coverage — now 2 of 7 list tools (customers, charges); 5 uncovered.
• #32 _clamp_limit [1,100] bounding — only the upper bound is tested, and only via stripe_list_charges. The lower bound (<1 → 1) is untested anywhere.
• #33 starting_after pagination — 1 of 7 list tools covered.

None of these are blocking, but #32's untested lower bound is the cheapest of the four to close.

NOT FIXED (1)

• #31 The migration's row filter (src/xagent/migrations/versions/20260818_seed_stripe_mcp_app.py:60,65) silently drops any ROW key absent from the live table, and because the app_id-exists guard returns early, a row seeded while a column was missing can never self-heal on re-run. Real-world probability is low (both is_visible_in_connector and launch_config long predate this migration), but the migration's own defensiveness is what's at issue, and no test exercises a table missing those columns. Inline comment below.

Still open, informational (7)

• #11 No get_* detail tool for payment_intents/subscriptions/products/prices, asymmetric with customers/charges/invoices — a scope choice, not a defect.
• #12 _request's 204/empty-body branch untested.
• #13 _flatten_form_params drops a None list element without renumbering the remaining bracket indices — no current tool schema can reach this, but it is a latent edge case.
• #14 truncate_error_text imported from a GraphQL-named module despite being generic. Notably graphql_errors.py:10-12 now documents the misfit rather than fixing it — fine as a stopgap, but the honest fix is a rename/move.
• #30 List tools expose only truncated: bool, no forward cursor — deferred as a fast-follow.
• #34, #35 — see Simplification below.

Dropped (1)

• #16 — a vague earlier concern with no recoverable detail; no action possible.

New findings

All of the following are MINOR. None block.

I. [Platform-level, flagged] The existing global/admin API-key fallback now reaches a money-moving tool.
src/xagent/web/api/mcp.py (pre-existing, not modified by this PR) resolves credentials with a generic global < shared < user precedence, so on a self-hosted deployment a user who has never connected their own Stripe key falls back to an admin-configured platform key. For a read-mostly connector like PostHog the blast radius of that fallback is data exposure; for Stripe it extends to stripe_create_refund — any user of the deployment can move real money on the admin's live account. This PR cannot fix it (the mechanism is generic and lives outside the diff), so consistent with #3/#5/#10 I am treating it as track-and-defer, not blocking — but it is the most consequential of the three platform gaps and I'd ask that the tracking issue be opened now rather than after merge. If a per-connector opt-out of the global fallback is cheap to add, excluding write-capable connectors from it is the smallest useful mitigation.

A. response_meta's docstring contradicts its behavior on the error path.
src/xagent/web/tools/mcp/stripe.py:170-178 states response_meta["idempotent_replayed"] is populated "for both a successful response and a >=400 one", but the assignment at :257-258 sits after the >=400 branch has already raised at :253. On an error, the caller's dict is silently left unpopulated. Low impact — the replay signal still reaches the LLM through the error message text — but the docstring is wrong as written. Inline comment below.

E. _paginated_results doesn't guard against a non-dict payload.
stripe.py:271 calls payload.get("data") without _request ever having checked that response.json() returned a dict. A bare JSON array or string in a 2xx body raises AttributeError. It degrades gracefully (each tool's outer except Exception catches it) rather than crashing, but produces an unclean Python-internal message and has no test — notably the only remaining hole in what is otherwise now a thorough malformed-response guard set.

F. The 429 retry rarely fires against real Stripe traffic, and 5xx is never retried.
stripe.py:218-226: Retry-After defaults to "0" when absent — which Stripe's real 429s frequently are — so the 0 < retry_after guard fails and no backoff happens, despite the module comment at :27-30 presenting this as working rate-limit mitigation. Separately, 500/502/503 (Stripe's more common transient failures) are never retried at all. This bounded-429-only pattern is shared verbatim with ~6 sibling connectors (jira.py, intercom.py, slack.py, posthog.py, …), so it is an inherited codebase convention rather than a Stripe-specific defect — worth a cross-cutting follow-up like #21, not a change here. Both the missing-header case and the 5xx case are untested.

D. No prefix-shape validation on Stripe resource ids.
stripe.py:498-503 (refund) and the three get-by-id tools at :357, :450, :597. An LLM passing a pi_… value into charge_id is very plausible — stripe_list_payment_intents returns exactly those ids — and currently yields a confusing Stripe-side error instead of a clear local one. Purely error-clarity; no correctness or security impact.

B. stripe_create_refund's reason isn't validated against Stripe's enum.
stripe.py:511-512 passes reason straight through. This matches the file's own convention (status filters in list_invoices/list_subscriptions are also pass-through, relying on Stripe to reject), so it is not a special-case oversight. To be explicit about the original worry: local validation would not prevent an accidental fraud report, because a typo produces a Stripe-rejected string, not a silent collision with the literal "fraudulent". A one-line allowlist buys a friendlier local error, nothing more.

C. stripe_create_refund's amount isn't locally validated.
stripe.py:509-510 — no >0 check, no ceiling. Verified that Stripe's Refunds API hard-enforces both a positive-amount constraint and the remaining-refundable ceiling, returning a 400 (amount_too_large etc.) that _request already surfaces cleanly. So the cost of a hallucinated amount is a wasted round-trip and a less friendly message, not an erroneous refund. Stripe's backstop is a genuine hard block here, not merely permissive.

G. Empty error bodies produce a trailing-colon message.
stripe.py:234-237 — a 502 with an empty body yields "Stripe API error (status 502): ". Untested path; a small or "<no response body>" fallback fixes it.

H. [Enhancement] _extract_error_detail discards Stripe's code/type/param.
stripe.py:140-158 keeps only message. For parameter_invalid_*-class errors, param names exactly which argument was wrong, which is the single most useful field for an LLM trying to self-correct. A design choice rather than a defect, but appending param when present is a cheap win.

Simplification opportunities

Non-blocking, both already acknowledged by the author as fast-follows:

• #34 The 7 list tools repeat an identical clamp → build-params → request → paginate → success shape, roughly 150 duplicated lines. A single _list_resource(path, extra_params, key) helper collapses them, and would also make the PARTIAL coverage gaps (#26, #32, #33) testable once instead of seven times — which is the strongest argument for doing it. Independently confirmed by a fresh mechanical simplification pass over the full diff.
• #35 The mutable response_meta: dict = {} out-parameter (2 call sites) is an awkward interface; returning a (payload, meta) tuple, or a small result object, reads better and would have made finding A structurally impossible. Note this is the interface observation only — the docstring/behavior mismatch is a real bug tracked separately as A.

Blocking status

Applying the rule (blocking only if a confirmed CRITICAL or MAJOR finding remains) to what is actually confirmed at HEAD:

• CRITICAL remaining: 0.
• MAJOR remaining: 0. Every previously-confirmed major item is FIXED and verified in the code, not merely claimed: Restricted Key enforcement (#18), complete redaction across all three error paths (#19), non-colliding idempotency keys (#2), refund mutual-exclusion (#24), 2xx/data malformed-response guards (#27, #28), and replay metadata on errors (#17).
• Remaining open work is: four test-coverage PARTIALs (#25, #26, #32, #33), one migration-defensiveness gap (#31), seven informational items, and new findings A–I — all MINOR.
• Remaining deferrals (#3, #5, #6, #10, #21, #22, and I) are each either tracked by an issue (#1578, #1585) or verified as a pre-existing codebase-wide pattern this PR inherited rather than introduced.

No blocking finding remains.

Blocking: no. Recommended decision: APPROVE.

Suggested (non-blocking) before or shortly after merge, in priority order: open the tracking issue for finding I; correct the response_meta docstring (A); add the payload-is-dict guard (E); close the _clamp_limit lower-bound test gap (#32). The list-tool consolidation (#34) is the highest-leverage follow-up because it makes the remaining coverage gaps cheap to close.

Nice work — the response-validation hardening across b2d80cd1/92860e23, and especially catching the or [] masking bug in your own fix, is exactly the right instinct for a payments connector.

[yiboyasss]

Thanks for the approval and the thorough final pass — pushed ac1c7e82 addressing the actionable minor findings on top of it, plus opened a tracking issue for the one platform-level concern you flagged as worth raising now.

Fixed:

• A: corrected _request's docstring — idempotent_replayed is only ever populated in response_meta on success (the >=400 branch has already raised by then); the replay signal on error still reaches the caller via the raised message, just not through response_meta.
• E: added an isinstance(payload, dict) check once, at the single choke point in _request right after response.json() — closes the "non-dict 2xx body" gap for all 14 tools at once instead of per-caller.
• D: added local prefix-shape validation (cus_/ch_/in_/pi_) — applied to every id-taking tool (get_customer, get_charge, get_invoice, and both of create_refund's id params), not just the one you cited, since it's the same mistake class everywhere an LLM could pass the wrong kind of Stripe id.
• C: stripe_create_refund's amount now rejects non-positive values locally instead of round-tripping to Stripe.
• G: fixed the dangling-colon message on an empty error body.
• H: _extract_error_detail now appends Stripe's param field when present, for the exact self-correction benefit you described.
• Qwen3.5-35B-A3B Support? #31: the seed migration now logs a warning naming any ROW key dropped because a table column is missing, instead of degrading silently.
• feat(auth): warn at startup when JWT env values still match example.env defaults #32: added the missing lower-bound test for _clamp_limit.

Opened, not fixed here:

• Finding I (platform-level global/shared key fallback reaching a money-moving tool) — agreed this is the most consequential of the platform gaps and shouldn't wait past merge. Opened Platform-level global/shared credential fallback lets any user reach a money-moving tool (stripe_create_refund) without their own key #1589 with the mechanism traced (_app_platform_env_available/_app_shared_env_available/_app_user_env_configured) and three mitigation options at increasing scope, same treatment as Stripe connector: data-minimize raw tool responses + surface Restricted Key guidance in connect flow #1578/No HITL confirmation gate reachable for MCP connector tools (blocks safe gating of stripe_create_refund and other destructive actions) #1585 — this connector's own code doesn't touch that mechanism, so fixing it here would mean reinventing a Stripe-specific guard for a generic platform behavior.

Left as-is, per your own framing:

• F (429 Retry-After defaulting to "0", no 5xx retry) — confirmed byte-identical to jira.py/intercom.py/slack.py/posthog.py's shared convention, so not something to diverge on Stripe-only; agree it's a cross-cutting follow-up candidate.
• B (reason enum validation) — per your own analysis, a local allowlist wouldn't actually prevent an accidental fraud report (a typo just becomes a different rejected string), and it matches this file's existing pass-through convention for other enum-shaped filters. Not adding it.
• fix(frontend): improve build page form and Select component behavior. #25/feat(rag): enhance pipeline observability #26/feat(rag): add parse result display api #33 (remaining per-tool exception/truncated/pagination test coverage) — still partial as disclosed; closing all of it fully is a bigger lift than this round, tracked informally alongside fix(frontend): drag-and-drop file upload and API error handling on the knowledge base page. #34 (the list-tool consolidation you noted would make these gaps cheap to close in one place instead of seven).

95 tests pass, ruff/mypy/codespell/the alembic head-check are clean, full migration chain applies at ac1c7e82.