Skip to content

Audit CI coverage: main-only examples failures should block releases #2509

Description

@jbeckwith-oai

Problem

PR #2495 passed its required checks, but the exact examples job that would have caught the regression was skipped on the PR. After merge, the main-branch examples run failed in the CommonJS Jest/jsdom fixtures with SyntaxError: Unexpected token 'export'; PR #2507 adds the missing focused regression.

The larger concern is that a check we run on main is neither exercised before merge nor an explicit prerequisite for publication.

Current coverage

Based on .github/workflows/ci.yml and .github/workflows/create-releases.yml:

Coverage PR / merge candidate Push to main Release publication
lint, build, Node test matrix, benchmarks Runs for branch pushes, merge groups, fork PRs, and ready_for_review; same-repo PR event runs may show skipped duplicates Runs No explicit dependency from the release workflow
credential-free ecosystem tests Runs, but the default Node ecosystem runner installs and type-checks only; it skips npm test unless --live is enabled Runs No explicit dependency
examples job: live example plus ecosystem-tests/cli.ts --live Never runs; job is restricted to pushes to refs/heads/main Runs No explicit dependency
release validation Release-please PR title/version check only Release-please and publication-state checks run Publish depends on publication state, native-browser compatibility, and release-package build

Relevant workflow sections:

  • Main-only examples gate:
    ${{
    github.repository == 'openai/openai-node' &&
    github.event_name == 'push' &&
    github.ref == 'refs/heads/main' &&
    github.actor != 'dependabot[bot]'
    }}
    environment: ci
    permissions:
    contents: read
    steps:
    - uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4
    with:
    persist-credentials: false
    - name: Set up pnpm
    uses: pnpm/action-setup@f40ffcd9367d9f12939873eb1018b921a783ffaa # v4
    - name: Set up Node
    uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4
    with:
    node-version-file: '.nvmrc'
    cache: pnpm
    - uses: denoland/setup-deno@11b63cf76cfcafb4e43f97b6cad24d8e8438f62d # v1
    with:
    deno-version: v1.39.0
    - uses: oven-sh/setup-bun@0c5077e51419868618aeaa5fe8019c62421857d6 # v2
    with:
    bun-version: 1.3.14
    - name: Install dependencies
    run: |
    pnpm install --frozen-lockfile
    - name: Run live examples and ecosystem tests
    run: |
    if [ -z "$OPENAI_API_KEY" ]; then
    echo "OPENAI_API_KEY is required for live CI checks" >&2
    exit 1
    fi
    pnpm tsn examples/chat-completions/demo.ts
    pnpm tsn ecosystem-tests/cli.ts --live --verbose --parallel --jobs=4 --retry=3
    env:
    DISABLE_V8_COMPILE_CACHE: '1'
    OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
    PUPPETEER_EXECUTABLE_PATH: /usr/bin/google-chrome
    PUPPETEER_SKIP_DOWNLOAD: 'true'
    ecosystem_tests:
  • Credential-free ecosystem runner:
    async function defaultNodeRunner() {
    await installPackage();
    await run('npm', ['run', 'tsc']);
    if (state.live) {
    await run('npm', ['test']);
    }
    }
  • Release publication dependencies: https://github.com/openai/openai-node/blob/a3f004f24006473347c532b7cebd495277853dc1/.github/workflows/create-releases.yml#L84-L346

Why #2495 escaped

The added/manual verification covered normal Node CommonJS, Node ESM, native-browser ESM, and browser bundling. The packed-package test combined --conditions=browser only with ESM; it did not exercise CommonJS require('openai') under browser export conditions or Jest's resolver.

The existing CommonJS Jest fixtures would have exposed that missing combination, but their tests run only in the main-only live examples job. The credential-free ecosystem job did not execute them.

Suggested direction

Please audit the PR, main, merge-queue, release-PR, and publication coverage as a single matrix, then make the release policy explicit.

A likely approach:

  1. Split deterministic ecosystem startup/import checks from live API checks. Run the deterministic CommonJS/Jest/jsdom import path on PRs and merge groups without credentials.
  2. Keep genuinely live API coverage main-only if needed, but decide explicitly whether it is release-blocking or advisory. Anything treated as a main-branch release signal should have an explicit publication gate.
  3. Bind npm publication to successful required checks for the immutable release SHA, including the release-relevant examples/ecosystem checks. A reusable workflow or a workflow_run/check-conclusion gate would avoid racing an independent CI workflow.
  4. Add a small workflow-policy check or documented coverage table so a future main-only job cannot silently fall outside both PR protection and release protection.

Desired outcome

  • A regression like CommonJS + browser export conditions + Jest fails before merge.
  • A failing release-relevant main check prevents npm publication for that exact release SHA.
  • Maintainers can see which checks are required on PRs, merge queue, main, release PRs, and publication.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions