From 31c17a54145af670ef19ef11d585480341ce05a9 Mon Sep 17 00:00:00 2001 From: Brandon Date: Mon, 3 Aug 2026 21:16:57 +0000 Subject: [PATCH 1/4] Regenerate generated/ from the spec `make generate` produces a small diff against the committed tree, so `generated/` had been hand-edited rather than regenerated (the datamodel-codegen header still read `timestamp: 2026-07-10`, from before the API-token fields were added in #445). Nothing was functionally wrong; the diff is docstrings and formatting: - generated/model.py: black splits the two long `description=` strings on the ApiToken/ApiTokenCreateResponse `last_used_at` and `token_ttl` fields. - api_token.py, api_token_create_response.py: the `token_ttl` argument was missing from the `__init__`/`_from_openapi_data` docstrings, and `last_used_at`'s docstring still had the pre-nullable type and a truncated description. This is the output of running `make generate` and nothing else. Co-Authored-By: Claude Opus 5 --- .../model/api_token.py | 4 +++- .../model/api_token_create_response.py | 4 +++- generated/model.py | 22 ++++++++++++++----- 3 files changed, 23 insertions(+), 7 deletions(-) diff --git a/generated/groundlight_openapi_client/model/api_token.py b/generated/groundlight_openapi_client/model/api_token.py index e967be45..dfd9f69d 100644 --- a/generated/groundlight_openapi_client/model/api_token.py +++ b/generated/groundlight_openapi_client/model/api_token.py @@ -140,7 +140,7 @@ def _from_openapi_data(cls, name, raw_key_snippet, created_at, last_used_at, *ar name (str): An nickname for the API token. This name must be unique for this user. raw_key_snippet (str): Since we're storing hashed keys, it can be useful to see the raw prefix snippet of the token. created_at (datetime): When was this token created? - last_used_at (datetime): The most recent time this API token was used. (Helpful for detecting suspicious activity). + last_used_at (datetime, none_type): The most recent time this API token was used. (Helpful for detecting suspicious activity). Null if the token has never been used. Keyword Args: _check_type (bool): if True, values for parameters in openapi_types @@ -174,6 +174,7 @@ def _from_openapi_data(cls, name, raw_key_snippet, created_at, last_used_at, *ar through its discriminator because we passed in _visited_composed_classes = (Animal,) expires_at (datetime, none_type): When does this token expire? If Null, the token never expires.. [optional] # noqa: E501 + token_ttl (int, none_type): Identity token lifetime policy in whole seconds. Null means tokens minted under this identity never expire (no rotation).. [optional] # noqa: E501 """ _check_type = kwargs.pop("_check_type", True) @@ -265,6 +266,7 @@ def __init__(self, name, *args, **kwargs): # noqa: E501 through its discriminator because we passed in _visited_composed_classes = (Animal,) expires_at (datetime, none_type): When does this token expire? If Null, the token never expires.. [optional] # noqa: E501 + token_ttl (int, none_type): Identity token lifetime policy in whole seconds. Null means tokens minted under this identity never expire (no rotation).. [optional] # noqa: E501 """ _check_type = kwargs.pop("_check_type", True) diff --git a/generated/groundlight_openapi_client/model/api_token_create_response.py b/generated/groundlight_openapi_client/model/api_token_create_response.py index 17acd71c..b9270a1a 100644 --- a/generated/groundlight_openapi_client/model/api_token_create_response.py +++ b/generated/groundlight_openapi_client/model/api_token_create_response.py @@ -145,7 +145,7 @@ def _from_openapi_data( name (str): An nickname for the API token. This name must be unique for this user. raw_key_snippet (str): Since we're storing hashed keys, it can be useful to see the raw prefix snippet of the token. created_at (datetime): When was this token created? - last_used_at (datetime): The most recent time this API token was used. (Helpful for detecting suspicious activity). + last_used_at (datetime, none_type): The most recent time this API token was used. (Helpful for detecting suspicious activity). Null if the token has never been used. raw_key (str): The full API token secret. Returned only once, when the token is created. Keyword Args: @@ -180,6 +180,7 @@ def _from_openapi_data( through its discriminator because we passed in _visited_composed_classes = (Animal,) expires_at (datetime, none_type): When does this token expire? If Null, the token never expires.. [optional] # noqa: E501 + token_ttl (int, none_type): Identity token lifetime policy in whole seconds. Null means tokens minted under this identity never expire (no rotation).. [optional] # noqa: E501 """ _check_type = kwargs.pop("_check_type", True) @@ -272,6 +273,7 @@ def __init__(self, name, *args, **kwargs): # noqa: E501 through its discriminator because we passed in _visited_composed_classes = (Animal,) expires_at (datetime, none_type): When does this token expire? If Null, the token never expires.. [optional] # noqa: E501 + token_ttl (int, none_type): Identity token lifetime policy in whole seconds. Null means tokens minted under this identity never expire (no rotation).. [optional] # noqa: E501 """ _check_type = kwargs.pop("_check_type", True) diff --git a/generated/model.py b/generated/model.py index 4659a06e..6d86a74e 100644 --- a/generated/model.py +++ b/generated/model.py @@ -1,6 +1,6 @@ # generated by datamodel-codegen: # filename: public-api.yaml -# timestamp: 2026-07-10T01:01:06+00:00 +# timestamp: 2026-08-03T21:13:28+00:00 from __future__ import annotations @@ -38,14 +38,20 @@ class ApiToken(BaseModel): created_at: datetime = Field(..., description="When was this token created?") last_used_at: Optional[datetime] = Field( ..., - description="The most recent time this API token was used. (Helpful for detecting suspicious activity). Null if the token has never been used.", + description=( + "The most recent time this API token was used. (Helpful for detecting suspicious activity). Null if the" + " token has never been used." + ), ) expires_at: Optional[datetime] = Field( None, description="When does this token expire? If Null, the token never expires." ) token_ttl: Optional[int] = Field( None, - description="Identity token lifetime policy in whole seconds. Null means tokens minted under this identity never expire (no rotation).", + description=( + "Identity token lifetime policy in whole seconds. Null means tokens minted under this identity never expire" + " (no rotation)." + ), ) @@ -64,14 +70,20 @@ class ApiTokenCreateResponse(BaseModel): created_at: datetime = Field(..., description="When was this token created?") last_used_at: Optional[datetime] = Field( ..., - description="The most recent time this API token was used. (Helpful for detecting suspicious activity). Null if the token has never been used.", + description=( + "The most recent time this API token was used. (Helpful for detecting suspicious activity). Null if the" + " token has never been used." + ), ) expires_at: Optional[datetime] = Field( None, description="When does this token expire? If Null, the token never expires." ) token_ttl: Optional[int] = Field( None, - description="Identity token lifetime policy in whole seconds. Null means tokens minted under this identity never expire (no rotation).", + description=( + "Identity token lifetime policy in whole seconds. Null means tokens minted under this identity never expire" + " (no rotation)." + ), ) raw_key: str = Field(..., description="The full API token secret. Returned only once, when the token is created.") From 2c04c403124a9c5fb381790972b6499cf4fc04c0 Mon Sep 17 00:00:00 2001 From: Brandon Date: Mon, 3 Aug 2026 21:16:57 +0000 Subject: [PATCH 2/4] Enforce that generated/ is what `make generate` produces `generated/` is excluded from linting (LINT_PATHS), so nothing caught hand-edits to generated code. The cost shows up later: the next person to run `make generate` gets a large unexplained diff mixed into their own PR. test_codegen/ re-runs each half of `make generate` into a scratch tree and diffs it against what is committed, failing with the actual diff and naming `make generate`: - datamodel-codegen (generated/model.py) is pure Python and already a dependency, so that half always runs. Its `timestamp:` header line varies per run and is normalized. - openapi-generator-cli needs node and java, so that half skips with a reason when they are missing -- except when REQUIRE_OPENAPI_GENERATOR is set, which the new test-codegen CI job does, so the check cannot quietly become a no-op where it is the only place it reliably runs. Notes on why the tests look the way they do: - They live outside test/ because test/conftest.py builds a Groundlight() client in pytest_configure, so everything under test/ needs GROUNDLIGHT_API_TOKEN to collect. These checks need no API access, and requiring a token would mean they never ran. `make test` depends on the new `make test-codegen` target, so a contributor running the suite gets them without knowing they exist. - The scratch tree has to be inside the repo: datamodel-codegen (which formats its own output) and black both find our [tool.black] settings by walking up from the paths they write, so generating into /tmp silently yields black's default 88-column style. - The openapi half regenerates over a *copy* of the committed tree rather than into an empty directory, because openapi-generator leaves existing files alone (the stubs under generated/test/) and lists only what it wrote in .openapi-generator/FILES. - The generator command lines are parsed out of the Makefile rather than duplicated, so changing how the SDK is generated cannot leave these tests checking the old way. Co-Authored-By: Claude Opus 5 --- .github/workflows/cicd.yaml | 35 +++ .gitignore | 5 + DEVELOPING.md | 12 + Makefile | 13 +- .../test_generated_matches_generator.py | 242 ++++++++++++++++++ 5 files changed, 304 insertions(+), 3 deletions(-) create mode 100644 test_codegen/test_generated_matches_generator.py diff --git a/.github/workflows/cicd.yaml b/.github/workflows/cicd.yaml index 4ae76fb9..385cd453 100644 --- a/.github/workflows/cicd.yaml +++ b/.github/workflows/cicd.yaml @@ -65,6 +65,41 @@ jobs: # This is associated with the "sdk-test-prod" user, credentials on 1password GROUNDLIGHT_API_TOKEN: ${{ secrets.GROUNDLIGHT_API_TOKEN_PROD }} + # Check that the committed `generated/` tree is what `make generate` produces. This gets its own + # job because it is the only test that needs node and java (for openapi-generator-cli) and the + # only one that needs no API token. REQUIRE_OPENAPI_GENERATOR makes the openapi-generator half + # fail rather than skip, so a missing toolchain here can't quietly turn the check into a no-op -- + # this is the one place it reliably runs. + test-codegen: + runs-on: ubuntu-latest + steps: + - name: get code + uses: actions/checkout@v4 + - name: install python + uses: actions/setup-python@v4 + with: + python-version: ${{ env.PYTHON_VERSION }} + - name: install poetry + uses: snok/install-poetry@v1 + with: + version: ${{ env.POETRY_VERSION }} + - name: Setup npm + uses: actions/setup-node@v3 + with: + node-version: 21 + cache: npm + - name: install java (openapi-generator-cli runs a jar) + uses: actions/setup-java@v4 + with: + distribution: temurin + java-version: "17" + - name: install dependencies and the SDK generator + run: make install-generator + - name: check generated/ is up to date + env: + REQUIRE_OPENAPI_GENERATOR: "1" + run: make test-codegen + # Check that the docs build. (No broken links, etc.) test-docs: runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index 6be72b11..c5018146 100644 --- a/.gitignore +++ b/.gitignore @@ -162,6 +162,11 @@ cython_debug/ node_modules/ +# Scratch trees written by test_codegen/ while re-running the SDK generators. They live inside +# the repo because datamodel-codegen and black find our black settings by walking up from the +# files they write; the tests clean them up unless a run is killed part-way. +codegen-scratch-*/ + *.swp **/.python-version diff --git a/DEVELOPING.md b/DEVELOPING.md index 43604682..ef41bbeb 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -67,6 +67,18 @@ Then you can generate the code by running: make generate ``` +Never hand-edit anything under `generated/` — always change the spec (or the generator +invocation) and re-run `make generate`. `make test` enforces this with the checks in +[test_codegen/](test_codegen), which re-run the generators and diff the result against what is +committed; you can run just those checks, without an API token, with: + +```shell +make test-codegen +``` + +The openapi-generator half of that check needs node and java, so it skips (with a reason) if you +have not run `make install-generator`. It always runs in CI, in the `test-codegen` job. + ### Linters Linters help us find issues before runtime. We're currently using: diff --git a/Makefile b/Makefile index 6c5331f3..f52cd38c 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,4 @@ -.PHONY: apidocs docs-comprehensive generate html install install-dev install-extras install-generator install-lint install-pre-commit test test-4edge test-integ test-local help +.PHONY: apidocs docs-comprehensive generate html install install-dev install-extras install-generator install-lint install-pre-commit test test-4edge test-codegen test-integ test-local help help: ## Print all targets with their descriptions @grep -E '^[a-zA-Z_-]+:.*$$' $(MAKEFILE_LIST) | awk 'BEGIN {FS = ":.*?## "}; {if (NF == 1) {printf "\033[36m%-30s\033[0m %s\n", $$1, ""} else {printf "\033[36m%-30s\033[0m %s\n", $$1, $$2}}' @@ -46,7 +46,14 @@ PROFILING_ARGS = \ --durations 25 \ --durations-min 0.1 -test: install ## Run tests against the prod API (needs GROUNDLIGHT_API_TOKEN) +# Checks that `generated/` is what `make generate` would produce. Kept out of `test/` (and given +# its own target) because everything under `test/` needs a GROUNDLIGHT_API_TOKEN to even collect, +# and this needs no API access. TEST_ARGS is deliberately not passed through: a filter meant for +# the real suite would match nothing here and fail the run. +test-codegen: install ## Check that generated/ matches what `make generate` produces (no API token needed) + ${PYTEST} test_codegen + +test: install test-codegen ## Run tests against the prod API (needs GROUNDLIGHT_API_TOKEN) ${PYTEST} ${PROFILING_ARGS} ${TEST_ARGS} ${CLOUD_FILTERS} test test-4edge: install ## Run tests against the prod API via the edge-endpoint (needs GROUNDLIGHT_API_TOKEN) @@ -68,7 +75,7 @@ test-docs-integ: install-extras ## Run the example code and tests in our docs a GROUNDLIGHT_ENDPOINT="https://api.integ.groundlight.ai/" ${PYTEST} --markdown-docs ${TEST_ARGS} docs README.md # Adjust which paths we lint -LINT_PATHS="src test bin samples" +LINT_PATHS="src test test_codegen bin samples" lint: install-lint ## Run linter to check formatting and style ./code-quality/lint ${LINT_PATHS} diff --git a/test_codegen/test_generated_matches_generator.py b/test_codegen/test_generated_matches_generator.py new file mode 100644 index 00000000..754e6671 --- /dev/null +++ b/test_codegen/test_generated_matches_generator.py @@ -0,0 +1,242 @@ +"""Checks that the committed `generated/` tree is exactly what `make generate` produces. + +`generated/` is excluded from linting (see `LINT_PATHS` in the Makefile), so nothing in CI +notices when generated code is hand-edited instead of regenerated. Nothing is usually +functionally wrong when that happens -- the cost is that the next person to run +`make generate` gets a large unexplained diff mixed into their own PR. These tests re-run +the generators into a scratch tree and diff the result against what is committed. + +They live outside `test/` on purpose: `test/conftest.py` constructs a `Groundlight()` client +in `pytest_configure`, so everything under `test/` needs a GROUNDLIGHT_API_TOKEN before +collection even starts. These checks need no API access, and putting them where a token is +required would mean they never ran. +""" + +import difflib +import os +import re +import shlex +import shutil +import subprocess +import sys +import tempfile +from pathlib import Path +from typing import Dict, Iterator, List, Optional + +import pytest + +REPO_ROOT = Path(__file__).resolve().parent.parent +GENERATED_DIR = REPO_ROOT / "generated" +MAKEFILE = REPO_ROOT / "Makefile" +OPENAPI_GENERATOR_CLI = REPO_ROOT / "node_modules" / ".bin" / "openapi-generator-cli" + +# Set this (CI does) to turn "openapi-generator-cli is not available here" from a skip into a +# failure. A check that quietly skips everywhere is worse than no check, because it reads as +# coverage. +REQUIRE_GENERATOR_ENV_VAR = "REQUIRE_OPENAPI_GENERATOR" + +FIX_HINT = "Run `make generate` and commit the result, rather than hand-editing `generated/`." + +# datamodel-codegen stamps the time of the run into its file header, so this one line +# legitimately differs on every run and has to be normalized away. +CODEGEN_TIMESTAMP_RE = re.compile(r"^#\s+timestamp:.*$", re.MULTILINE) + +# Failure messages carry the real diff, but a whole-tree regeneration diff can be enormous. +MAX_DIFF_LINES = 400 + + +def _generate_recipe_commands() -> List[str]: + """The shell commands in the Makefile's `generate` target, with continuations joined. + + These are parsed out of the Makefile rather than duplicated here so that changing how the + SDK is generated cannot leave these tests silently checking the old way of generating it. + """ + lines = MAKEFILE.read_text().splitlines() + starts = [i for i, line in enumerate(lines) if re.match(r"^generate:", line)] + assert len(starts) == 1, f"expected exactly one `generate:` target in {MAKEFILE}, found {len(starts)}" + + commands: List[str] = [] + for line in lines[starts[0] + 1 :]: + if line.startswith("#"): # a comment column-0 inside a recipe does not end it + continue + if not line.startswith("\t"): # first non-recipe line ends the recipe + break + body = line.lstrip("\t").strip() + if commands and commands[-1].endswith("\\"): + commands[-1] = f"{commands[-1][:-1].strip()} {body}" + else: + commands.append(body) + assert commands, f"found no commands in the `generate` target of {MAKEFILE}" + return commands + + +def _recipe_command(tool: str) -> List[str]: + """The `make generate` command line that runs `tool`, split into argv.""" + matches = [command for command in _generate_recipe_commands() if tool in command] + assert len(matches) == 1, f"expected exactly one `{tool}` command in `make generate`, found {matches}" + return shlex.split(matches[0]) + + +def _retarget(command: List[str], old: str, new: str) -> List[str]: + """Point a generator command at the scratch tree instead of the committed one.""" + assert old in command, f"expected `{old}` as an argument in `make generate`'s command: {command}" + return [new if argument == old else argument for argument in command] + + +def _venv_command(command: List[str]) -> List[str]: + """Rewrite `poetry run ...` to call `` from the environment running these tests. + + We are already inside the poetry environment, so this runs the same versions `make generate` + would while avoiding a nested `poetry run`. + """ + if command[:2] == ["poetry", "run"]: + command = command[2:] + executable = Path(sys.executable).parent / command[0] + if not executable.exists(): + found = shutil.which(command[0]) + assert found, f"`{command[0]}` is not installed in this environment -- run `make install`" + executable = Path(found) + return [str(executable), *command[1:]] + + +def _run(command: List[str], what: str) -> None: + result = subprocess.run(command, cwd=REPO_ROOT, capture_output=True, text=True, check=False) + if result.returncode != 0: + pytest.fail( + f"Could not run {what} (exit {result.returncode}), so we cannot tell whether `generated/` is" + f" up to date.\ncommand: {shlex.join(command)}\nstdout:\n{result.stdout}\nstderr:\n{result.stderr}" + ) + + +def _run_black(paths: List[Path]) -> None: + """Format `paths` the way `make generate`'s final `black .` would.""" + if not paths: + return + command = _venv_command(_recipe_command("black")) + assert command[-1] == ".", f"expected `make generate` to end with `black .`, got {command}" + # Files are passed explicitly instead of handing black the scratch directory: black skips + # gitignored paths when it walks a directory, and the scratch directory is gitignored. + _run([*command[:-1], *(str(path) for path in paths)], "black") + + +@pytest.fixture(name="scratch_dir") +def fixture_scratch_dir() -> Iterator[Path]: + """A scratch directory for generator output, inside the repo. + + It has to be inside the repo: both datamodel-codegen (which formats its own output) and + black find the project's `[tool.black]` settings by walking up from the paths they write, so + generating into /tmp silently produces black's default 88-column style and every comparison + below fails for the wrong reason. + """ + with tempfile.TemporaryDirectory(dir=REPO_ROOT, prefix="codegen-scratch-") as scratch: + yield Path(scratch) + + +def _normalize(source: str) -> str: + return CODEGEN_TIMESTAMP_RE.sub("# timestamp: ", source) + + +def _file_diff(relative_path: str, committed: str, regenerated: str) -> List[str]: + return list( + difflib.unified_diff( + committed.splitlines(), + regenerated.splitlines(), + fromfile=f"generated/{relative_path} (committed)", + tofile=f"generated/{relative_path} (freshly generated)", + lineterm="", + ) + ) + + +def _truncated(diff_lines: List[str]) -> str: + if len(diff_lines) <= MAX_DIFF_LINES: + return "\n".join(diff_lines) + hidden = len(diff_lines) - MAX_DIFF_LINES + return "\n".join([*diff_lines[:MAX_DIFF_LINES], f"... {hidden} more diff lines suppressed ..."]) + + +def _tree_contents(root: Path) -> Dict[str, str]: + return { + path.relative_to(root).as_posix(): path.read_text(errors="replace") + for path in root.rglob("*") + if path.is_file() and "__pycache__" not in path.parts + } + + +def test_generated_model_matches_datamodel_codegen(scratch_dir: Path) -> None: + """`generated/model.py` must be exactly what `make generate`'s datamodel-codegen call writes. + + This half of the generator is pure Python and already a project dependency, so it always runs. + """ + regenerated = scratch_dir / "model.py" + command = _retarget(_venv_command(_recipe_command("datamodel-codegen")), "generated/model.py", str(regenerated)) + _run(command, "datamodel-codegen") + _run_black([regenerated]) + + committed_text = _normalize((GENERATED_DIR / "model.py").read_text()) + regenerated_text = _normalize(regenerated.read_text()) + if committed_text != regenerated_text: + diff = _truncated(_file_diff("model.py", committed_text, regenerated_text)) + pytest.fail( + "generated/model.py is not what datamodel-codegen produces -- it looks hand-edited." + f" {FIX_HINT}\n(The `timestamp:` header line is normalized out, so it is not the" + f" cause of this diff.)\n\n{diff}" + ) + + +def _openapi_generator_unavailable_reason() -> Optional[str]: + """Why the openapi-generator half cannot run here, or None if it can.""" + if not shutil.which("node"): + return "node is not installed, and openapi-generator-cli needs it" + if not OPENAPI_GENERATOR_CLI.exists(): + return f"{OPENAPI_GENERATOR_CLI.relative_to(REPO_ROOT)} is missing -- run `make install-generator`" + if not shutil.which("java"): + return "openapi-generator-cli runs a Java jar, and `java` is not on PATH" + return None + + +def test_generated_client_matches_openapi_generator(scratch_dir: Path) -> None: + """Everything openapi-generator owns under `generated/` must match a fresh run. + + Skipped when node/java/the generator CLI are unavailable, since neither is guaranteed on a + contributor's machine -- but CI sets REQUIRE_OPENAPI_GENERATOR so that it cannot skip there. + """ + reason = _openapi_generator_unavailable_reason() + if reason: + if os.environ.get(REQUIRE_GENERATOR_ENV_VAR): + pytest.fail( + f"{REQUIRE_GENERATOR_ENV_VAR} is set, so this check is not allowed to skip here, but {reason}." + ) + pytest.skip(f"{reason}. (Set {REQUIRE_GENERATOR_ENV_VAR}=1 to make this a failure instead of a skip.)") + + # Regenerate over a *copy* of the committed tree rather than into an empty directory: + # openapi-generator leaves already-existing files alone (the per-model stubs under + # generated/test/, for instance) and records only the files it wrote in + # .openapi-generator/FILES, so generating from empty yields a legitimately different + # manifest. Copying first reproduces what `make generate` actually does. + regenerated_dir = scratch_dir / "generated" + shutil.copytree(GENERATED_DIR, regenerated_dir, ignore=shutil.ignore_patterns("__pycache__")) + _run(_retarget(_recipe_command("openapi-generator-cli"), "./generated", str(regenerated_dir)), "openapi-generator") + _run_black(sorted(regenerated_dir.rglob("*.py"))) + + committed = _tree_contents(GENERATED_DIR) + regenerated = _tree_contents(regenerated_dir) + # model.py comes from datamodel-codegen, not from openapi-generator; the test above owns it. + committed.pop("model.py", None) + regenerated.pop("model.py", None) + + missing = sorted(set(regenerated) - set(committed)) + differing = sorted(path for path in regenerated if path in committed and committed[path] != regenerated[path]) + if not missing and not differing: + return + + report = ["generated/ is not what openapi-generator produces -- it looks hand-edited.", FIX_HINT, ""] + if missing: + report += ["Files the generator produces that are not committed:", *(f" generated/{p}" for p in missing), ""] + if differing: + report += ["Committed files that differ from a fresh run:", *(f" generated/{p}" for p in differing), ""] + diff_lines: List[str] = [] + for relative_path in differing: + diff_lines += _file_diff(relative_path, committed[relative_path], regenerated[relative_path]) + report.append(_truncated(diff_lines)) + pytest.fail("\n".join(report)) From a039e62d8110ce3103d32860cbc2e25bde2f8218 Mon Sep 17 00:00:00 2001 From: Auto-format Bot Date: Mon, 3 Aug 2026 22:30:05 +0000 Subject: [PATCH 3/4] Automatically reformatting code --- test_codegen/test_generated_matches_generator.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/test_codegen/test_generated_matches_generator.py b/test_codegen/test_generated_matches_generator.py index 754e6671..3fe0c107 100644 --- a/test_codegen/test_generated_matches_generator.py +++ b/test_codegen/test_generated_matches_generator.py @@ -204,9 +204,7 @@ def test_generated_client_matches_openapi_generator(scratch_dir: Path) -> None: reason = _openapi_generator_unavailable_reason() if reason: if os.environ.get(REQUIRE_GENERATOR_ENV_VAR): - pytest.fail( - f"{REQUIRE_GENERATOR_ENV_VAR} is set, so this check is not allowed to skip here, but {reason}." - ) + pytest.fail(f"{REQUIRE_GENERATOR_ENV_VAR} is set, so this check is not allowed to skip here, but {reason}.") pytest.skip(f"{reason}. (Set {REQUIRE_GENERATOR_ENV_VAR}=1 to make this a failure instead of a skip.)") # Regenerate over a *copy* of the committed tree rather than into an empty directory: From f241ca439371c31fb8f59aa208c326f109acd1c1 Mon Sep 17 00:00:00 2001 From: Brandon Date: Tue, 4 Aug 2026 17:10:59 +0000 Subject: [PATCH 4/4] Run the test-codegen CI job on node 22 The job failed on `node-version: 21` with ERR_REQUIRE_ESM: openapi-generator-cli 2.x `require()`s proxy-agent, which is ESM-only, and node 21 has no require(esm) support. The docs jobs pin 21 happily because nothing there loads the generator. Reproduced locally against this exact spec and node_modules, with `REQUIRE_OPENAPI_GENERATOR=1 make test-codegen`: node 18.19.1 ERR_REQUIRE_ESM (fails, same stack as CI) node 20.19.0 passes node 21.7.3 ERR_REQUIRE_ESM (fails, same stack as CI) node 22.14.0 passes which matches require(esm) landing in 20.19 and 22.12 and never in 21.x. Pinned the job to 22 and wrote the requirement into DEVELOPING.md, since `make generate` itself fails the same way for anyone on node 21. Co-Authored-By: Claude Opus 5 --- .github/workflows/cicd.yaml | 5 ++++- DEVELOPING.md | 4 +++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/cicd.yaml b/.github/workflows/cicd.yaml index 385cd453..de331c94 100644 --- a/.github/workflows/cicd.yaml +++ b/.github/workflows/cicd.yaml @@ -86,7 +86,10 @@ jobs: - name: Setup npm uses: actions/setup-node@v3 with: - node-version: 21 + # Not 21 like the docs jobs: openapi-generator-cli 2.x require()s proxy-agent, which is + # ESM-only, so it dies with ERR_REQUIRE_ESM on any node without require(esm) support. + # Verified 2026-08-04: 18.19.1 and 21.7.3 fail, 20.19.0 and 22.14.0 work. + node-version: 22 cache: npm - name: install java (openapi-generator-cli runs a jar) uses: actions/setup-java@v4 diff --git a/DEVELOPING.md b/DEVELOPING.md index ef41bbeb..4fc46997 100644 --- a/DEVELOPING.md +++ b/DEVELOPING.md @@ -55,7 +55,9 @@ it will run them, add the changes, and then ask you to try committing again with The auto-generated SDK code is in the [generated/](generated) directory. Most of the time, you won't need to generate code. But if the API specification changes, you may need to generate SDK code. To re-generate the client code, you'll need to [install npm](https://github.com/nvm-sh/nvm#intro) -first. Then you can install the code generator by running: +first, plus a Java runtime (openapi-generator-cli runs a jar). Use node 20.19+ or 22+ — the +generator CLI fails with `ERR_REQUIRE_ESM` on older node, and on node 21. Then you can install the +code generator by running: ```shell make install-generator