Skip to content

refactor: typegen improvements (OpenAPI-generated Cloudflare types, openapi-fetch, node scripts)#784

Open
andykenward wants to merge 9 commits into
mainfrom
pr/typegen-improvements
Open

refactor: typegen improvements (OpenAPI-generated Cloudflare types, openapi-fetch, node scripts)#784
andykenward wants to merge 9 commits into
mainfrom
pr/typegen-improvements

Conversation

@andykenward

Copy link
Copy Markdown
Owner

Summary

Tooling and type-safety improvements for the Cloudflare Pages integration. Two themes:

  1. Derive Cloudflare types from the canonical OpenAPI schema instead of the hand-maintained @cloudflare/types.
  2. Migrate bin scripts from tsx to native node type-stripping where possible.

No user-facing behavior change — internal refactors only.

Cloudflare types from OpenAPI (refactor: generate cloudflare types from openapi schema)

  • New codegen (bin/codegen/cloudflare-pages.ts, pnpm run codegen:cloudflare) fetches Cloudflare's canonical OpenAPI schema (cloudflare/api-schemas), ref-walks from the Pages operations this action uses to prune to only the referenced components, and runs openapi-typescript on that subset → a focused __generated__/types/cloudflare/pages.ts rather than a ~10 MB dump of the whole API.
  • PagesDeployment now sources from the generated schema; @cloudflare/types is removed (and its orphaned pnpm.peerDependencyRules). The hand-rolled aliases: string[] | null patch is dropped — the schema already models it correctly.
  • One behavior correction: the schema's deployment stage status never includes skipped, so the dead case 'skipped' in status.ts is removed.

openapi-fetch for REST calls (feat: openapi-fetch)

  • New typed client (src/common/cloudflare/api/client.ts) over the generated paths, with middleware injecting auth once per request.
  • fetch-result.ts reduced from raw-fetch + as FetchResult<T> casts to unwrap/unwrapSuccess — a thin layer applying Cloudflare's {success, result, errors} envelope semantics over the client's {data, error, response}. Response types are now inferred from the endpoint instead of asserted.
  • getCloudflareApiEndpoint (manual URL builder) retired; getCloudflareLogEndpoint (dashboard URL) kept.

Tooling migrations (tsxnode)

  • codegen:events, deployments:delete, and download scripts now run with native node type-stripping (no tsx). bin/codegen flattened (index.ts + types/ → flat *.ts).
  • Lint/format/settings tweaks and docs (CLAUDE.md).

Validation

pnpm run all passes end to end: sync-versions → knip → codegen → codegen:events → codegen:cloudflare → tsc → format → lint → 79 tests → build.

Bundle grew ~10 kB (deploy) from openapi-fetch, as expected.

Notes / follow-ups

  • This is a private package; no changeset added (internal refactor, no user-facing change).
  • Out of scope: bin/deployments/index.ts still hand-builds URLs / uses raw fetch — could move to the typed client for consistency.

🤖 Generated with Claude Code

@changeset-bot

changeset-bot Bot commented Jun 5, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: bef996d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
github-actions-cloudflare-pages Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions

github-actions Bot commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

Cloudflare Pages Deployment

Event Name: pull_request
Environment: preview
Project: cloudflare-pages-action
Built with commit: 1ede9f0
Preview URL: https://711e1c56.cloudflare-pages-action-a5z.pages.dev
Branch Preview URL: https://pr-typegen-improvements.cloudflare-pages-action-a5z.pages.dev

Wrangler Output

⛅️ wrangler 4.86.0
───────────────────
Uploading... (3/3)
✨ Success! Uploaded 0 files (3 already uploaded) (0.64 sec)

✨ Uploading _headers
✨ Uploading Functions bundle
🌎 Deploying...
✨ Deployment complete! Take a peek over at https://711e1c56.cloudflare-pages-action-a5z.pages.dev
✨ Deployment alias URL: https://pr-typegen-improvements.cloudflare-pages-action-a5z.pages.dev

}

const fetchSchema = async (): Promise<JsonObject> => {
const response = await fetch(SCHEMA_URL)
@andykenward andykenward marked this pull request as ready for review June 7, 2026 14:14
@andykenward

Copy link
Copy Markdown
Owner Author

@claude review the pull request

@claude

claude Bot commented Jun 7, 2026

Copy link
Copy Markdown

Claude finished @andykenward's task in 2m 37s —— View job


PR Review

  • Gather context and read changed files
  • Review security flag (SSRF in codegen script)
  • Review OpenAPI type generation changes
  • Review openapi-fetch client migration
  • Review tsxnode script migration
  • Review test changes
  • Post final review

Overall this is a solid, well-scoped refactor. The three themes (OpenAPI codegen, typed client, tsxnode) each stand alone and the implementation quality is high. A few things to address below.


Security: SSRF warning (CodeQL) — bin/codegen/cloudflare-pages.ts:24–86

The CodeQL flag is real and should be fixed before merge. CLOUDFLARE_API_SCHEMAS_REF is interpolated directly into the URL without validation:

// Line 24–25
const REF = process.env['CLOUDFLARE_API_SCHEMAS_REF'] ?? 'main'
const SCHEMA_URL = `https://raw.githubusercontent.com/cloudflare/api-schemas/${REF}/openapi.json`

While the domain is hardcoded (githubusercontent.com), an attacker who controls this env var in CI can craft a REF with path traversal characters or redirect to a completely different repo path. The fix is a one-liner — validate the REF against a safe pattern before use:

assert.ok(
  /^[a-zA-Z0-9._\-/]+$/u.test(REF),
  `CLOUDFLARE_API_SCHEMAS_REF must be a valid git ref (got: ${JSON.stringify(REF)})`
)

Fix this →


Changeset inconsistency

The PR description says "no changeset added", but .changeset/silly-moments-occur.md IS present and marks this as minor. The changeset-bot confirms it will trigger a version bump. Since openapi-fetch lands in dependencies (not devDependencies) this is arguably correct — even though end-users consume the bundled dist/, any audit of the package.json would surface the new runtime dep and the +10 kB bundle increase. Worth deciding intentionally: either delete the changeset (if truly internal) or update the PR description to acknowledge the version bump.


openapi-typescript version not pinned — package.json:72

Every other devDependency is pinned to an exact version, but openapi-typescript uses a caret range:

"openapi-typescript": "^7.13.0"

A minor version bump to openapi-typescript can change the generated output format and break the codegen, leading to unexpected diffs. Pin it to "7.13.0" to match project conventions.

Fix this →


unwrapSuccess silent-false on {success: false, errors: []}src/common/cloudflare/api/fetch-result.ts:51–59

const envelope = data ?? error
if (envelope && !envelope.success && envelope.errors.length > 0) {
  throwFetchError(...)
}
return envelope?.success ?? false  // ← returns false silently if errors is empty

If the API returns {success: false, errors: []}, this returns false without throwing. The caller in delete.ts does manually throw when success !== true, so the behavior is safe — but it creates an asymmetry with unwrap, which always throws on failure. Worth either documenting the intentional behavior or adding a fallback throw here.


bin/download/index.ts extension inconsistency

The import was changed from a .js to a .ts extension:

-import './payloads/github-webhooks-payload-examples.js'
+import './payloads/github-webhooks-payload-examples.ts'

This works under Node 24's native TypeScript stripping, but it's the only .ts extension used in an import across the entire source tree. All other TypeScript files follow the TypeScript ESM convention of .js extensions for sibling imports. Consider reverting this to .js for consistency.


Strengths

  • Pruned schema codegen is elegant: fetching the full 10 MB Cloudflare schema and ref-walking to a minimal subset is the right approach. The collectRefs cycle-detection via the used Set is correct.
  • openapi-fetch integration is clean: client creation, auth injection, and envelope semantics are properly separated into client.ts and fetch-result.ts.
  • @cloudflare/types removal eliminates the React peer-dep hack — good riddance.
  • PagesDeployment derivation from components['schemas']['pages_deployment'] is the right pattern; it'll automatically track schema changes on regen.
  • case 'skipped' removal in status.ts is correct per the schema and removes dead code.
  • Test coverage for unwrap/unwrapSuccess is thorough, including the null-result and error envelope cases.
  • tsxnode migration is appropriate now that Node 24 is required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants