refactor(openapi): inject fetch, remove transport.ts and undici by tonyxiao · Pull Request #190 · stripe/sync-engine

tonyxiao · 2026-03-29T18:34:09Z

Summary

packages/openapi was accumulating HTTP infrastructure it shouldn't own — introduced in e4149ee when proxy support was added. This duplicated the proxy logic already in source-stripe/src/transport.ts.

Remove transport.ts and undici from packages/openapi
Add a required fetch: typeof globalThis.fetch parameter to buildListFn, buildRetrieveFn, and resolveOpenApiSpec
source-stripe (the only consumer) passes its own fetchWithProxy — no behavioral change
Delete stale duplicate specFetchHelper.ts left in source-stripe/src/openapi/ pointing at an old GitHub path

packages/openapi now has zero HTTP infrastructure: it knows what to fetch (Stripe-specific GitHub URLs, pagination patterns) but not how to make HTTP calls.

Design doc

docs/plans/2026-03-29-openapi-pure-package-design.md

Test plan

cd packages/openapi && pnpm test — all pass
cd packages/source-stripe && pnpm test — all pass
pnpm build — clean

cc @kdhillon-stripe — this addresses the transport duplication introduced in e4149ee (#185). The packages/openapi/transport.ts added there is removed here in favour of injecting fetch from the caller. Behaviorally identical — source-stripe passes its fetchWithProxy which routes through the same proxy.

🤖 Generated with Claude Code

Copilot

Pull request overview

Refactors @stripe/sync-openapi to be transport-agnostic by requiring callers to inject fetch, removing the package’s proxy/HTTP infrastructure and consolidating transport behavior in source-stripe.

Changes:

Remove packages/openapi/transport.ts and the undici dependency; update openapi helpers to accept an injected fetch.
Update source-stripe call sites to pass fetchWithProxy (via an apiFetch adapter) into openapi helpers; delete the stale duplicate spec fetch helper in source-stripe.
Introduce sslmode parsing for Postgres connection strings in destination/state Postgres packages (changes default SSL behavior).

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
pnpm-lock.yaml	Drops `undici` from `packages/openapi` dependency graph.
packages/openapi/package.json	Removes `undici` dependency (empty dependencies object).
packages/openapi/transport.ts	Deletes openapi-owned proxy/transport implementation.
packages/openapi/specFetchHelper.ts	Makes `fetch` required; routes all GitHub/spec fetches through injected `fetch`.
packages/openapi/listFnResolver.ts	Makes `fetch` required for list/retrieve builders; removes transport import.
packages/openapi/tests/transport.test.ts	Deletes tests for the removed transport module.
packages/openapi/tests/specFetchHelper.test.ts	Updates tests to pass an injected fetch mock (no global stubbing).
packages/openapi/tests/listFnResolver.test.ts	Updates tests to pass an injected fetch mock.
packages/source-stripe/src/index.ts	Passes proxy-aware fetch into `resolveOpenApiSpec`.
packages/source-stripe/src/resourceRegistry.ts	Passes proxy-aware fetch into `buildListFn`/`buildRetrieveFn`.
packages/source-stripe/src/openapi/specFetchHelper.ts	Removes stale duplicate OpenAPI spec fetch helper.
packages/source-stripe/src/openapi/specFetchHelper.test.ts	Removes tests for the deleted helper.
packages/source-stripe/src/openapi/index.ts	Stops re-exporting the deleted helper.
packages/state-postgres/src/state-store.ts	Derives `pg` SSL config from connection-string `sslmode`.
packages/state-postgres/src/migrate.ts	Derives `pg` SSL config from connection-string `sslmode` (and changes import ordering).
packages/destination-postgres/src/index.ts	Derives `pg` SSL config from connection-string `sslmode` for non-AWS connections.
packages/destination-postgres/src/buildPoolConfig.test.ts	Updates/expands tests for the new `sslmode` mapping behavior.
docs/plans/2026-03-29-openapi-pure-package-design.md	Adds design doc describing the “pure openapi package” decision and API changes.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-30T15:40:06Z

packages/destination-postgres/src/index.ts

+ * `ssl` option. Defaults to `false` (no SSL) when no sslmode is present — SSL
+ * must be opted into explicitly via `sslmode=require` (or `verify-ca`/`verify-full`).
+ */
+function sslConfigFromConnectionString(connStr: string): PoolConfig['ssl'] {
+  try {
+    const sslmode = new URL(connStr).searchParams.get('sslmode')
+    if (sslmode === 'disable') return false
+    if (sslmode === 'verify-ca' || sslmode === 'verify-full') return true
+    if (sslmode === 'require') return { rejectUnauthorized: false }
+    return false
+  } catch {


sslConfigFromConnectionString only handles sslmode=disable/require/verify-*; other valid libpq values like "prefer" and "allow" currently fall through to the default and disable SSL. Consider explicitly handling these modes (or throwing on unsupported values) to avoid silently doing the wrong thing for users who pass them.

Suggested change

* `ssl` option. Defaults to `false` (no SSL) when no sslmode is present — SSL

* must be opted into explicitly via `sslmode=require` (or `verify-ca`/`verify-full`).

*/

function sslConfigFromConnectionString(connStr: string): PoolConfig['ssl'] {

try {

const sslmode = new URL(connStr).searchParams.get('sslmode')

if (sslmode === 'disable') return false

if (sslmode === 'verify-ca' || sslmode === 'verify-full') return true

if (sslmode === 'require') return { rejectUnauthorized: false }

return false

} catch {

* `ssl` option.

*

* Behavior:

* - No `sslmode` query param: `false` (no SSL).

* - `sslmode=disable`: `false` (no SSL).

* - `sslmode=verify-ca` or `sslmode=verify-full`: `true` (SSL, verify cert).

* - `sslmode=require`: `{ rejectUnauthorized: false }` (SSL, do not verify CA).

*

* Other libpq sslmodes like `prefer` and `allow` cannot be represented

* faithfully with the `pg` client's `ssl` option. Passing these (or any

* unsupported value) will throw to avoid silently disabling SSL.

*/

function sslConfigFromConnectionString(connStr: string): PoolConfig['ssl'] {

try {

const sslmode = new URL(connStr).searchParams.get('sslmode')

// No sslmode specified: preserve existing default (no SSL).

if (sslmode === null) {

return false

}

if (sslmode === 'disable') {

return false

}

if (sslmode === 'verify-ca' || sslmode === 'verify-full') {

return true

}

if (sslmode === 'require') {

return { rejectUnauthorized: false }

}

// sslmode values that are valid in libpq but not faithfully representable

// in node-postgres are rejected explicitly to avoid surprising behavior.

if (sslmode === 'prefer' || sslmode === 'allow') {

throw new Error(

`Unsupported Postgres sslmode "${sslmode}". ` +

'Use sslmode=require, sslmode=verify-ca, or sslmode=verify-full.'

)

}

// Any other non-null value is also treated as unsupported.

throw new Error(`Unsupported Postgres sslmode "${sslmode}".`)

} catch {

// If the connection string is not a valid URL, fall back to no SSL,

// preserving existing behavior.

@tonyxiao this will also conflict with #191

Copilot · 2026-03-30T15:40:07Z

packages/destination-postgres/src/index.ts

+/**
+ * Map the `sslmode` query parameter from a Postgres connection string to a pg
+ * `ssl` option. Defaults to `false` (no SSL) when no sslmode is present — SSL
+ * must be opted into explicitly via `sslmode=require` (or `verify-ca`/`verify-full`).
+ */


This PR introduces new Postgres SSL behavior (defaulting to ssl: false unless sslmode is set) in destination-postgres/state-postgres, but the PR title/description focuses on openapi transport refactoring. Please call out this behavior change explicitly in the PR description (or split it into a separate PR) since it can impact existing deployments.

Copilot · 2026-03-30T15:40:07Z

packages/destination-postgres/src/index.ts

+/**
+ * Map the `sslmode` query parameter from a Postgres connection string to a pg
+ * `ssl` option. Defaults to `false` (no SSL) when no sslmode is present — SSL
+ * must be opted into explicitly via `sslmode=require` (or `verify-ca`/`verify-full`).
+ */
+function sslConfigFromConnectionString(connStr: string): PoolConfig['ssl'] {
+  try {
+    const sslmode = new URL(connStr).searchParams.get('sslmode')
+    if (sslmode === 'disable') return false
+    if (sslmode === 'verify-ca' || sslmode === 'verify-full') return true
+    if (sslmode === 'require') return { rejectUnauthorized: false }
+    return false
+  } catch {
+    return false
+  }
+}


sslConfigFromConnectionString is duplicated across multiple packages (destination-postgres + state-postgres). To avoid drift (e.g., different handling for future sslmode values), consider extracting this into a shared helper (likely in @stripe/sync-util-postgres) and reusing it here.

Copilot · 2026-03-30T15:40:07Z

packages/state-postgres/src/state-store.ts

+    if (sslmode === 'disable') return false
+    if (sslmode === 'verify-ca' || sslmode === 'verify-full') return true
+    if (sslmode === 'require') return { rejectUnauthorized: false }
+    return false


sslConfigFromConnectionString only recognizes sslmode=disable/require/verify-*; other valid sslmode values (e.g. "prefer"/"allow") will currently fall through to the default and disable SSL. Consider explicitly handling these modes (or rejecting unsupported ones) so callers don’t get unexpected plaintext connections.

Suggested change

if (sslmode === 'disable') return false

if (sslmode === 'verify-ca' || sslmode === 'verify-full') return true

if (sslmode === 'require') return { rejectUnauthorized: false }

return false

// If sslmode is not specified, preserve the existing default of no SSL.

if (!sslmode || sslmode === 'disable') return false

if (sslmode === 'verify-ca' || sslmode === 'verify-full') return true

if (sslmode === 'require') return { rejectUnauthorized: false }

// Explicitly reject unsupported sslmode values so we do not silently

// disable SSL when the caller has requested an SSL-related mode.

throw new Error(`Unsupported sslmode "${sslmode}" in Postgres connection string`)

Copilot · 2026-03-30T15:40:07Z

packages/state-postgres/src/migrate.ts

+  try {
+    const sslmode = new URL(connStr).searchParams.get('sslmode')
+    if (sslmode === 'disable') return false
+    if (sslmode === 'verify-ca' || sslmode === 'verify-full') return true
+    if (sslmode === 'require') return { rejectUnauthorized: false }
+    return false
+  } catch {
+    return false
+  }


sslConfigFromConnectionString only handles sslmode=disable/require/verify-*; other valid sslmode values (e.g. "prefer"/"allow") currently fall through to the default (false) and disable SSL. Consider explicitly handling or rejecting those values to avoid silently changing connection security.

Suggested change

try {

const sslmode = new URL(connStr).searchParams.get('sslmode')

if (sslmode === 'disable') return false

if (sslmode === 'verify-ca' || sslmode === 'verify-full') return true

if (sslmode === 'require') return { rejectUnauthorized: false }

return false

} catch {

return false

}

let sslmode: string | null

try {

sslmode = new URL(connStr).searchParams.get('sslmode')

} catch {

// If the connection string cannot be parsed as a URL, fall back to no SSL.

return false

}

// If sslmode is not specified, keep the existing behavior of disabling SSL.

if (!sslmode) return false

if (sslmode === 'disable') return false

if (sslmode === 'verify-ca' || sslmode === 'verify-full') return true

if (sslmode === 'require') return { rejectUnauthorized: false }

// Explicitly handle other common valid sslmode values without changing

// the current effective behavior (they previously fell through to false).

if (sslmode === 'prefer' || sslmode === 'allow') return false

// Any other non-empty sslmode is considered unsupported to avoid silently

// changing connection security characteristics.

throw new Error(`Unsupported sslmode "${sslmode}" in database connection string.`)

Copilot · 2026-03-30T15:40:08Z

packages/state-postgres/src/migrate.ts

+  }
+}
 import { renderMigrationTemplate } from './migrationTemplate.js'
 import type { Migration } from './migrations/index.js'
 import { migrations as allMigrations } from './migrations/index.js'


Import declarations appear after executable code (the sslConfigFromConnectionString function). Many TS/ESM lint setups enforce imports-first (and it’s generally clearer to keep all imports together). Move the renderMigrationTemplate / migrations imports above the helper function or move the helper below the imports.

Committed-By-Agent: claude

…dency Remove HTTP infrastructure from packages/openapi. buildListFn, buildRetrieveFn, and resolveOpenApiSpec now accept an injected `fetch` parameter instead of importing fetchWithProxy from the deleted transport.ts. packages/source-stripe passes its own fetchWithProxy at each call site. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Committed-By-Agent: claude

… casts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Committed-By-Agent: claude

Committed-By-Agent: claude

…lently disabling SSL sslmode=prefer and sslmode=allow are valid libpq values that pg cannot faithfully represent. Previously they fell through to ssl:false, silently disabling SSL. Now they throw with a clear error message pointing to the supported values. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Committed-By-Agent: claude

tonyxiao force-pushed the openapi-inject-fetch branch from d2a29d0 to aa2dbcb Compare March 30, 2026 01:03

kdhillon-stripe requested a review from Copilot March 30, 2026 15:34

Copilot started reviewing on behalf of kdhillon-stripe March 30, 2026 15:35 View session

Copilot AI reviewed Mar 30, 2026

View reviewed changes

tonyxiao and others added 5 commits March 30, 2026 09:45

docs: design for making packages/openapi a pure utility (no transport)

fcb8cc3

Committed-By-Agent: claude

fix(openapi): remove dead specFetchHelper copy, replace unsound fetch…

bd8ad6f

… casts Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Committed-By-Agent: claude

style: prettier

fcb268f

Committed-By-Agent: claude

tonyxiao force-pushed the openapi-inject-fetch branch from aa2dbcb to 9a5e2f2 Compare March 30, 2026 16:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(openapi): inject fetch, remove transport.ts and undici#190

refactor(openapi): inject fetch, remove transport.ts and undici#190
tonyxiao wants to merge 5 commits intov2from
openapi-inject-fetch

tonyxiao commented Mar 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

kdhillon-stripe Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Copilot AI Mar 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

- * `ssl` option. Defaults to `false` (no SSL) when no sslmode is present — SSL
- * must be opted into explicitly via `sslmode=require` (or `verify-ca`/`verify-full`).
- */
-function sslConfigFromConnectionString(connStr: string): PoolConfig['ssl'] {
-  try {
-    const sslmode = new URL(connStr).searchParams.get('sslmode')
-    if (sslmode === 'disable') return false
-    if (sslmode === 'verify-ca' || sslmode === 'verify-full') return true
-    if (sslmode === 'require') return { rejectUnauthorized: false }
-    return false
-  } catch {
+ * `ssl` option.
+ *
+ * Behavior:
+ * - No `sslmode` query param: `false` (no SSL).
+ * - `sslmode=disable`: `false` (no SSL).
+ * - `sslmode=verify-ca` or `sslmode=verify-full`: `true` (SSL, verify cert).
+ * - `sslmode=require`: `{ rejectUnauthorized: false }` (SSL, do not verify CA).
+ *
+ * Other libpq sslmodes like `prefer` and `allow` cannot be represented
+ * faithfully with the `pg` client's `ssl` option. Passing these (or any
+ * unsupported value) will throw to avoid silently disabling SSL.
+ */
+function sslConfigFromConnectionString(connStr: string): PoolConfig['ssl'] {
+  try {
+    const sslmode = new URL(connStr).searchParams.get('sslmode')
+    // No sslmode specified: preserve existing default (no SSL).
+    if (sslmode === null) {
+      return false
+    }
+    if (sslmode === 'disable') {
+      return false
+    }
+    if (sslmode === 'verify-ca' || sslmode === 'verify-full') {
+      return true
+    }
+    if (sslmode === 'require') {
+      return { rejectUnauthorized: false }
+    }
+    // sslmode values that are valid in libpq but not faithfully representable
+    // in node-postgres are rejected explicitly to avoid surprising behavior.
+    if (sslmode === 'prefer' || sslmode === 'allow') {
+      throw new Error(
+        `Unsupported Postgres sslmode "${sslmode}". ` +
+          'Use sslmode=require, sslmode=verify-ca, or sslmode=verify-full.'
+      )
+    }
+    // Any other non-null value is also treated as unsupported.
+    throw new Error(`Unsupported Postgres sslmode "${sslmode}".`)
+  } catch {
+    // If the connection string is not a valid URL, fall back to no SSL,
+    // preserving existing behavior.

Conversation

tonyxiao commented Mar 29, 2026

Summary

Design doc

Test plan

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

kdhillon-stripe Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants