From 656a6559898a725dbf09d1b44e86878be42c498c Mon Sep 17 00:00:00 2001 From: Eunseo Lee Date: Thu, 1 Oct 2026 17:27:50 +0900 Subject: [PATCH 1/3] fix(ci): run the tests/build guards pytest was silently skipping `uv run pytest -q` collected 515 tests and not one of them came from `tests/build/`. pytest's default `norecursedirs` carries the bare pattern `build`, which matches by basename, so recursing from `testpaths = ["tests"]` skipped the directory whole. Every regen guard in there -- the `.fernignore` check, the README sync, the CLI<->SDK contract -- has never gated a PR, and guards that were red the day they were written merged to main on green CI. Drop `build` from `norecursedirs` and keep the `./build/` artifact tree out by path with `--ignore=build` instead. Collection goes 515 -> 937. That turns main red, because one of the newly-live guards is red and has been since it landed: `reference.md` documents `**request:** WorkflowRunStartIn` on three methods whose generated clients take flattened `script_text`/`source_language` kwargs, so the documented call raises TypeError before a request is sent. Replace the three hand-named assertions with a parse of every documented method, compared against `inspect.signature` in both directions, and record what the generator currently gets wrong in `_REFERENCE_DEFECTS` with reasons -- same contract as `_INTENTIONALLY_UNEXPOSED`, so the regen that fixes the generator deletes the excuse. Generalizing finds six mismatches, not three: `workspace_members.update_member_role`, `update_invite_role` and `workflows.runs_summary` (`from` documented, `from_` generated, so the documented call is not even syntactically valid) were wrong the same way and nothing said so. Co-Authored-By: Claude Opus 5 (1M context) --- AGENTS.md | 1 + pyproject.toml | 12 ++- tests/build/test_sdk_contract.py | 147 +++++++++++++++++++++++++++---- tests/unit/test_collection.py | 45 ++++++++++ 4 files changed, 186 insertions(+), 19 deletions(-) create mode 100644 tests/unit/test_collection.py diff --git a/AGENTS.md b/AGENTS.md index 520bda5..225c026 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -126,6 +126,7 @@ Comments must **never**: - **Editing `src/onepin/` outside `_cli/`** → lost on next Fern regen. Change the OpenAPI spec instead. - **`ruff check .`** → silently skips `_cli/` due to the prefix exclude. Use the scoped invocation. - **Adding tests under `src/onepin/tests/`** → not collected. Put project tests in `tests/`. +- **Dropping the `norecursedirs` override in `pyproject.toml`** → `tests/build/` stops being collected, because pytest's default list contains the bare pattern `build`. Every regen guard in there goes silently dead while CI stays green. `tests/unit/test_collection.py` guards it. - **Forgetting `scripts/post_fern.sh`** after a manual Fern regen → `py.typed` markers vanish; downstream type-checkers stop treating `onepin` as typed. - **`twine delete`** on a bad release → irrevocable. Use `twine yank` (see RUNBOOK). - **Changing the CLI surface** (e.g. adding the `skill` group) → regenerate the README block (`python scripts/gen_cli_docs.py`) and the manifest snapshot (`UPDATE_SNAPSHOT=1 pytest tests/cli/test_cli_manifest.py`), or `test_readme_in_sync.py` / `test_cli_manifest.py` fail. diff --git a/pyproject.toml b/pyproject.toml index 2abde2e..be3171d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -125,7 +125,17 @@ ignore_errors = true [tool.pytest.ini_options] asyncio_mode = "auto" -addopts = "-q --strict-markers" +# `--ignore=build` pairs with `norecursedirs` below: it keeps the `./build/` tree that +# `python -m build` leaves behind out of collection, which the stock `norecursedirs` used to +# cover for free. +addopts = "-q --strict-markers --ignore=build" # Restrict collection to top-level tests/ — Fern-generated SDK ships its own # tests under src/onepin/tests/ with markers we don't register. testpaths = ["tests"] +# pytest's *default* norecursedirs contains the bare pattern `build`, which matches by +# basename — so it skipped `tests/build/` along with the artifact dir, and every guard in +# there (`test_sdk_contract.py`, `test_cli_preserved.py`) silently never ran under +# `pytest -q`. They only ran where regen.yml names the path explicitly, which is why red +# guards reached main through green PRs. This is the stock list with `build` dropped; +# `tests/unit/test_collection.py` fails if the directory stops being collected again. +norecursedirs = ["*.egg", ".*", "_darcs", "CVS", "dist", "node_modules", "venv", "{arch}"] diff --git a/tests/build/test_sdk_contract.py b/tests/build/test_sdk_contract.py index 829c9d1..e94a137 100644 --- a/tests/build/test_sdk_contract.py +++ b/tests/build/test_sdk_contract.py @@ -19,8 +19,12 @@ half is hand-written, so it lands nowhere. Nothing was red while ``GET /voices`` grew ``age``/``category``/``accent`` and ``voices list`` kept offering four of its eight filters for months -- the discovery path was a user saying the agent behaved oddly, not a test. Since -``.github/workflows/regen.yml`` already runs ``tests/build`` on every generated PR, asserting -the reverse direction here turns that silence into a failing check on the regen PR itself. +``ci.yml`` runs the whole suite on every PR, including the regen PRs, asserting the reverse +direction here turns that silence into a failing check on the regen PR itself. + +That last sentence is load-bearing and was false for a while: ``tests/build`` is only reachable +because ``pyproject.toml`` drops ``build`` from pytest's default ``norecursedirs``. See +``tests/unit/test_collection.py``. Both directions are then repeated for the hand-written composites, which are not TABLE rows and which every parametrized check above therefore skips. See :data:`_COMPOSITE_CALLS`. @@ -379,22 +383,129 @@ def test_composite_calls_covers_every_sdk_call_site() -> None: assert not unused, f"_COMPOSITE_CALLS lists {sorted(unused)}, which composites.py no longer calls — drop it" +# === generated reference.md vs the generated clients ===================================== +# +# reference.md is written by the same generator, in the same run, from the same spec as the +# code it documents, so the two cannot disagree without one of them being wrong. They do: the +# "⚙️ Parameters" table is built from a pre-transform view of the method while the code — and +# the "🔌 Usage" snippet in the same section — use the post-transform one. A reader who follows +# the table gets a TypeError before a request is ever sent. +# +# Parametrizing over every documented method, rather than naming the few known-bad ones, is +# the difference between catching this class of defect and catching one instance of it. + +_REFERENCE = Path(__file__).parents[2] / "src" / "onepin" / "reference.md" +_PARAMS_HEADING = "#### ⚙️ Parameters" +_SECTION_RE = re.compile(r"
client\.([\w.]*?)(?:]+>)?(\w+)\(") +_PARAM_RE = re.compile(r"\*\*(\w+):\*\*") + + +def _reference_sections() -> list[tuple[str, tuple[str, ...]]]: + """Every documented method as (dotted SDK path, the params its table lists).""" + text = _REFERENCE.read_text(encoding="utf-8") + sections = [] + for match in _SECTION_RE.finditer(text): + owner, name = match.group(1).rstrip("."), match.group(2) + body = text[match.start() : text.index("
", match.start())] + # Only the parameter table. Descriptions use `**Bold:**` for their own subheadings + # (`**Pagination:**`, `**Deprecated:**`), which are prose, not parameters. + heading = body.find(_PARAMS_HEADING) + documented = tuple(_PARAM_RE.findall(body[heading:])) if heading >= 0 else () + sections.append((f"{owner}.{name}" if owner else name, documented)) + return sections + + +_REFERENCE_SECTIONS = _reference_sections() + +_INLINED_BODY = ( + "Generator documents the pre-inlining wrapper model while generating a method that takes " + "the body's fields as flattened kwargs — no generated method accepts `request` at all." +) +_RESERVED_WORD = ( + "Generator documents the wire name while generating `from_`, because `from` is a Python " + "keyword — the documented call is not even syntactically valid." +) + +# Documented-vs-generated mismatches, keyed by dotted SDK path, valued by param -> reason. +# +# Same contract as _INTENTIONALLY_UNEXPOSED, and recorded here rather than fixed in place for a +# reason beyond taste: reference.md is regenerated on every regen and is not in `.fernignore`, +# so a local correction survives exactly until the next PR. The fix belongs in the generator — +# which `fern/generators.yml` pins to `latest`, so this list doubles as the record of what the +# current unpinned generator gets wrong. +_REFERENCE_DEFECTS: dict[str, dict[str, str]] = { + "workflows.estimate_workflow": dict.fromkeys(("request", "script_text", "source_language"), _INLINED_BODY), + "workflows.preview_run": dict.fromkeys(("request", "script_text", "source_language"), _INLINED_BODY), + "workflows.runs.start": dict.fromkeys(("request", "script_text", "source_language"), _INLINED_BODY), + "workspace_members.update_member_role": dict.fromkeys(("request", "role"), _INLINED_BODY), + "workspace_members.update_invite_role": dict.fromkeys(("request", "role"), _INLINED_BODY), + "workflows.runs_summary": dict.fromkeys(("from", "from_"), _RESERVED_WORD), +} + + +def _reference_mismatch(dotted: str, documented: tuple[str, ...]) -> tuple[set[str], set[str]]: + """(params the table invents, params the table omits) for one documented method.""" + accepted = set(inspect.signature(_resolve_method(OnePinClient(token="op_live_test"), dotted)).parameters) + accepted.discard("self") + return set(documented) - accepted, accepted - set(documented) + + @pytest.mark.parametrize( - "method_path", - ("workflows.estimate_workflow", "workflows.preview_run", "workflows.runs.start"), + ("dotted", "documented"), + _REFERENCE_SECTIONS, + ids=[dotted for dotted, _ in _REFERENCE_SECTIONS], ) -def test_run_input_reference_matches_flattened_signatures(method_path: str) -> None: - """The generated reference must expose the same flattened run-input kwargs as the clients.""" - reference = (Path(__file__).parents[2] / "src" / "onepin" / "reference.md").read_text(encoding="utf-8") - method_name = method_path.rsplit(".", 1)[-1] - owner = method_path.removesuffix(f".{method_name}") - marker = rf"
client\.{re.escape(owner)}(?:\.]+>)?{method_name}\(\.\.\.\)" - match = re.search(marker, reference) - assert match is not None, f"reference.md has no section for {method_path}" - section = reference[match.start() : reference.index("
", match.start())] - - assert "**script_text:**" in section, f"{method_path} reference omits script_text" - assert "**source_language:**" in section, f"{method_path} reference omits source_language" - assert "**request:** `WorkflowRunStartIn`" not in section, ( - f"{method_path} reference still documents the stale wrapped request model" +def test_reference_params_match_signature(dotted: str, documented: tuple[str, ...]) -> None: + """Each parameter table must name exactly the params its method takes. + + Both directions are drift, but they are not equally bad: a param the table invents is a + documented call that raises, while one it omits is a capability users cannot discover. + """ + invented, omitted = _reference_mismatch(dotted, documented) + excused = set(_REFERENCE_DEFECTS.get(dotted, {})) + + assert not invented - excused, ( + f"{dotted}: reference.md documents {sorted(invented - excused)}, which the method does not " + f"accept — following the docs raises TypeError. Fix the generator (reference.md is " + f"regenerated, so editing it here is lost), or record it in _REFERENCE_DEFECTS." + ) + assert not omitted - excused, ( + f"{dotted}: the method accepts {sorted(omitted - excused)} but reference.md's parameter " + f"table omits it. Same two options as above." ) + + +def test_reference_sections_are_discoverable() -> None: + """Guard the parse itself. + + The check above is parametrized over whatever `_reference_sections` finds, so a generator + that reshapes the surrounding markup would empty the parametrization and leave the suite + green by vacuity — the exact silence this file exists to end. Anchor on the shape instead. + """ + sections = dict(_REFERENCE_SECTIONS) + + assert len(sections) >= 80, f"only {len(sections)} methods parsed out of reference.md — markup likely changed" + assert "workflows.runs.start" in sections, "reference.md parse no longer finds a known method path" + tableless = sorted(dotted for dotted, params in sections.items() if not params) + assert not tableless, f"{tableless} parsed with no parameters — the {_PARAMS_HEADING!r} heading likely moved" + + +def test_reference_defect_allowlist_has_no_stale_entries() -> None: + """The allowlist may only name mismatches that are still mismatches. + + This is what fails on the regen that fixes the generator: the defect is gone, the excuse is + not, and the excuse has to go with it. + """ + documented_by_path = dict(_REFERENCE_SECTIONS) + + for dotted, reasons in _REFERENCE_DEFECTS.items(): + assert dotted in documented_by_path, f"_REFERENCE_DEFECTS names {dotted}, which reference.md does not document" + assert reasons, f"{dotted}: allowlist entry is empty — drop the key instead" + invented, omitted = _reference_mismatch(dotted, documented_by_path[dotted]) + stale = set(reasons) - (invented | omitted) + assert not stale, ( + f"{dotted}: allowlist still excuses {sorted(stale)}, which reference.md and the client " + f"now agree on — delete those entries" + ) + for param, reason in reasons.items(): + assert reason.strip(), f"{dotted}.{param}: allowlisted with no reason" diff --git a/tests/unit/test_collection.py b/tests/unit/test_collection.py new file mode 100644 index 0000000..694339a --- /dev/null +++ b/tests/unit/test_collection.py @@ -0,0 +1,45 @@ +"""Guard: the default pytest invocation must actually collect ``tests/build/``. + +Everything in ``tests/build/`` is a gate on the Fern regen pipeline — the ``.fernignore`` +guard, the README sync, the CLI<->SDK contract. All of it was dead for months: pytest's +default ``norecursedirs`` carries the bare pattern ``build``, which matches by basename, so +recursing from ``testpaths = ["tests"]`` skipped the directory whole. ``pytest -q`` reported a +green suite that had never imported those files, and guards that were red the day they were +written merged to main on green CI. + +``pyproject.toml`` drops ``build`` from ``norecursedirs`` and ignores the artifact tree by path +instead. That is one word in a config file, so this asserts the *behaviour* from outside: run +collection the way CI runs it and look for a test that lives in there. +""" + +from __future__ import annotations + +import subprocess +import sys +from pathlib import Path + +_REPO_ROOT = Path(__file__).parents[2] + +# Not a module this file imports: the point is what a *separate* pytest process, configured +# only by pyproject.toml, decides to collect. +_CANARIES = ( + "tests/build/test_sdk_contract.py", + "tests/build/test_cli_preserved.py", +) + + +def test_default_collection_includes_tests_build() -> None: + result = subprocess.run( + [sys.executable, "-m", "pytest", "--collect-only", "-q", "-p", "no:cacheprovider"], + capture_output=True, + text=True, + cwd=_REPO_ROOT, + ) + assert result.returncode == 0, result.stdout + result.stderr + + missing = [canary for canary in _CANARIES if canary not in result.stdout] + assert not missing, ( + f"`pytest --collect-only` does not reach {missing} — tests/build/ is being skipped " + f"again, so none of its regen guards gate anything. Check `norecursedirs` in " + f"pyproject.toml (pytest's default value contains `build`)." + ) From 77c1b2ec61144d1eeeff324349f3cf40ddb2779c Mon Sep 17 00:00:00 2001 From: Eunseo Lee Date: Thu, 1 Oct 2026 17:28:03 +0900 Subject: [PATCH 2/3] feat(ci): report regen guards on the PR and autofix CLI drift The guards ran only on the dev path, which opens no PR, so the failures landed where there was nothing to fix forward from; the PR-opening path skipped them entirely. Guards ran where no PR existed, PRs existed where no guards ran, and the comment claiming ci.yml covered the PR side was false until the previous commit. Run them on both paths without failing the run, and post the verdict onto the PR as a comment -- including when it is clean, since "ran" being indistinguishable from "skipped" is what hid two weeks of red guards. Hand the one mechanical failure class to an agent: SDK->CLI drift, a generated param no CLI flag reaches. BE->SDK is automatic so the param lands in the client with nobody in the loop, while the hand-written CLI never follows. The agent adds the `Opt(...)`, regenerates the README block and the manifest snapshot, adds a test and pushes to the PR branch. It is barred from allowlisting a guard, weakening a test or touching generated files, and raises the `Cmd(gate=)` question for a human rather than deciding it. Reference-mismatch and renamed-method failures are explicitly out of its scope. Also close superseded `auto:sdk-regen` PRs. Each regen is a whole-SDK snapshot of one spec commit rather than an increment, so three open at once is three ways to merge the SDK backwards. Scoped to the label, the `fern-regen/` branch prefix, and a number below the PR just opened, so a human PR cannot be caught and a concurrent newer regen is never closed by an older run. The merge gate is ci.yml, not this workflow. Autofix is skipped with a notice until ANTHROPIC_API_KEY is configured; `allowed_bots` names the pipeline App rather than `*` because the repo is public. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/regen.yml | 234 +++++++++++++++++++++++++++++++++--- AGENTS.md | 2 + 2 files changed, 221 insertions(+), 15 deletions(-) diff --git a/.github/workflows/regen.yml b/.github/workflows/regen.yml index ef8c689..f4520f3 100644 --- a/.github/workflows/regen.yml +++ b/.github/workflows/regen.yml @@ -10,6 +10,11 @@ name: Regenerate SDK # environment=dev -> generation smoke-check only, NO PR # workflow_dispatch -> manual regen PR from a given spec-repo SHA (default: main HEAD) # +# A guard failure never blocks the PR. `tests/build` runs on both paths and reports into the +# PR as a comment; when the failure is SDK -> CLI drift, the `autofix` job hands it to an agent +# that adds the missing flag on the PR branch. The merge is gated by ci.yml, not by this file. +# `autofix` is skipped, with a notice, when ANTHROPIC_API_KEY is not configured. +# # Requires a GitHub App (org secrets PIPELINE_APP_ID / PIPELINE_APP_PRIVATE_KEY) and the # repo secret SPEC_REPO (name of the backend repo). Fetches public-spec.yaml -- the # narrowed customer-facing export; NOT api-spec.yaml, which is the backend's full @@ -42,6 +47,9 @@ jobs: outputs: environment: ${{ steps.vars.outputs.env }} source_sha: ${{ steps.spec.outputs.resolved_sha }} + pr_number: ${{ steps.cpr.outputs.pull-request-number }} + pr_branch: fern-regen/${{ steps.spec.outputs.resolved_sha }} + guards_failed: ${{ steps.guards.outputs.failed }} steps: - name: Resolve trigger inputs id: vars @@ -157,23 +165,35 @@ jobs: echo "CLI preserved + non-bare + shaped client confirmed." - name: Build/CLI guard tests - # Only on the dev smoke-check path, which opens no PR and so has no other place - # for these to run. On the PR-opening paths this step is deliberately skipped: - # ci.yml runs the very same `tests/build` on the regen PR, so gating here is - # duplicate coverage with a strictly worse failure mode. `tests/build` mixes two - # severities -- CLI->SDK breakage (a renamed/dropped method, which crashes the CLI) - # and SDK->CLI drift (a new param with no flag yet, which breaks nothing) -- and a - # failure of either kind used to run BEFORE "Open regen PR" and take the PR down - # with it. That deleted the spec update entirely: no branch, no diff, nothing to - # fix forward from, just a red run. Which is the exact silence test_sdk_contract.py - # was written to end. The regen PR is a proposal, not a release; gate the merge - # (branch protection + ci.yml), not the proposal. The pre-PR shell guard above - # still blocks the catastrophic case -- a clobbered _cli/ or a bare client. - if: steps.vars.outputs.open_pr != 'true' + id: guards + # Runs on BOTH paths and fails neither. The regen PR is a proposal, not a release, so + # a guard failure is information *about* the proposal: it is posted onto the PR below, + # and handed to the autofix job when it is the mechanical kind. Blocking PR creation on + # it -- the shape this step used to have -- deleted the spec update entirely: no branch, + # no diff, nothing to fix forward from, just a red run. + # + # What gates the merge is ci.yml, which runs this same `tests/build` on the PR. That was + # the intent all along but not the behaviour: pytest's default `norecursedirs` carries + # the pattern `build`, so ci.yml silently never collected the directory, and the step + # here was the only place these ran. `pyproject.toml` drops that pattern now. + # + # The hard gate stays the shell guard above: a clobbered `_cli/` or a bare client is not + # a proposal worth reviewing, it is a broken generation. run: | + set -euo pipefail pip install uv uv sync --all-extras - uv run pytest tests/build -q + set +e + uv run pytest tests/build -q > guards.log 2>&1 + status=$? + set -e + tail -60 guards.log + { + echo "failed=$([ "$status" -eq 0 ] && echo false || echo true)" + echo 'summary<> "$GITHUB_OUTPUT" - name: Record source spec sha (.spec-sha — consumed by promote-prod.yml) if: steps.vars.outputs.open_pr == 'true' @@ -207,6 +227,7 @@ jobs: fi - name: Open regen PR + id: cpr if: steps.vars.outputs.open_pr == 'true' uses: peter-evans/create-pull-request@v8 with: @@ -231,9 +252,192 @@ jobs: Review the diff, let CI run, then merge to ship the new SDK surface. labels: auto:sdk-regen + - name: Close superseded regen PRs + # Each regen PR is a whole-SDK snapshot of one spec commit, not an increment on the last + # one, so N of them open at once is N ways to regress the SDK by merging the wrong one. + # Only the newest can be right. Scoped three ways so a human PR can never be caught: + # the label this workflow applies, the branch prefix it owns, and a number lower than + # the PR just opened (so a concurrent newer regen is never closed by an older run). + if: steps.cpr.outputs.pull-request-number != '' + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + REPO: ${{ github.repository }} + NEW_PR: ${{ steps.cpr.outputs.pull-request-number }} + run: | + set -euo pipefail + gh pr list --repo "$REPO" --state open --label auto:sdk-regen \ + --json number,headRefName --jq '.[] | "\(.number)\t\(.headRefName)"' > open-regens.tsv + while IFS=$'\t' read -r num branch; do + [ -n "${num:-}" ] || continue + [ "$num" -lt "$NEW_PR" ] || continue + case "$branch" in fern-regen/*) ;; *) continue ;; esac + echo "Closing #$num ($branch), superseded by #$NEW_PR" + gh pr comment "$num" --repo "$REPO" \ + --body "Superseded by #${NEW_PR}, which regenerates the SDK from a newer spec commit. Each regen PR is a full snapshot rather than an increment, so this one can only take the SDK backwards now. Closing and deleting the branch." + gh pr close "$num" --repo "$REPO" --delete-branch + done < open-regens.tsv + rm -f open-regens.tsv + + - name: Report guard results on the regen PR + # The PR is where the diff is, so it is where the verdict belongs -- including when the + # verdict is "clean", which is the difference between a check that ran and one that was + # skipped. That difference is exactly what went unnoticed for two weeks. + if: steps.cpr.outputs.pull-request-number != '' + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + REPO: ${{ github.repository }} + PR: ${{ steps.cpr.outputs.pull-request-number }} + FAILED: ${{ steps.guards.outputs.failed }} + SUMMARY: ${{ steps.guards.outputs.summary }} + RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }} + run: | + set -euo pipefail + if [ "$FAILED" != "true" ]; then + printf '%s\n' "**Regen guards: clean.** \`tests/build\` passed against the freshly generated SDK." \ + "" "[run log]($RUN_URL)" > comment.md + else + { + printf '%s\n' "**Regen guards: failing.** \`tests/build\` is red against the freshly generated SDK:" "" + printf '```\n%s\n```\n' "$SUMMARY" + printf '%s\n' "" \ + "This does not block the PR -- the diff is here to fix forward from. \`ci.yml\` runs the same suite as a required check, so the merge is gated on it being green." \ + "" "[run log]($RUN_URL)" + } > comment.md + fi + gh pr comment "$PR" --repo "$REPO" --body-file comment.md + rm -f comment.md + - name: Smoke-check note (dev — no PR) if: steps.vars.outputs.open_pr != 'true' - run: echo "::notice::dev smoke-check passed — generation clean, CLI preserved. No PR (publish gated on prod)." + run: | + if [ "${{ steps.guards.outputs.failed }}" = "true" ]; then + echo "::warning::dev smoke-check: tests/build is red. No PR on this path, so there is nothing to fix forward from here — the prod regen PR is where this gets handled." + else + echo "::notice::dev smoke-check passed — generation clean, CLI preserved. No PR (publish gated on prod)." + fi + + autofix: + # Close the SDK -> CLI half of the pipeline, which is the half that has no automation. + # BE -> SDK is automatic (the spec regenerates the client), so a new API parameter lands in + # the generated code with nobody in the loop; the hand-written CLI never follows, and the + # gap sat unnoticed for months at a time. The work to close it is mechanical and fully + # specified by the failing test -- add the flag, regenerate the docs, update the snapshot -- + # so it is work an agent can do and a human can review in a diff. + # + # Deliberately NOT given the other failure classes. A generator defect (reference.md + # documenting params the client does not accept) is unfixable in this repo: the file is + # regenerated and not in `.fernignore`. And silencing a guard by allowlisting it is the one + # move that must stay human -- an agent that can turn its own alarm off is how two weeks of + # red guards reached main on green CI. + needs: regenerate + if: needs.regenerate.outputs.guards_failed == 'true' && needs.regenerate.outputs.pr_number != '' + runs-on: ubuntu-latest + timeout-minutes: 30 + permissions: + contents: write + pull-requests: write + issues: write + id-token: write + actions: read + steps: + - name: Check for the Claude credential + id: key + env: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + run: | + if [ -z "${ANTHROPIC_API_KEY:-}" ]; then + echo "::notice::ANTHROPIC_API_KEY is not configured; skipping autofix. The guard results are still on the PR." + echo "present=false" >> "$GITHUB_OUTPUT" + else + echo "present=true" >> "$GITHUB_OUTPUT" + fi + + - name: Mint GitHub App token + id: app-token + if: steps.key.outputs.present == 'true' + uses: actions/create-github-app-token@v1 + with: + app-id: ${{ secrets.PIPELINE_APP_ID }} + private-key: ${{ secrets.PIPELINE_APP_PRIVATE_KEY }} + owner: ${{ github.repository_owner }} + repositories: ${{ github.event.repository.name }} + + - name: Checkout the regen PR branch + if: steps.key.outputs.present == 'true' + uses: actions/checkout@v7 + with: + ref: ${{ needs.regenerate.outputs.pr_branch }} + token: ${{ steps.app-token.outputs.token }} + # Credentials stay persisted here, unlike the regenerate job: the agent commits and + # pushes with the git CLI rather than through create-pull-request. + # hatch-vcs needs the history + tags or the wheel the guard tests build gets a bogus + # version and trips the version-sanity assertions. + fetch-depth: 0 + fetch-tags: true + + - name: Let Claude close the drift + if: steps.key.outputs.present == 'true' + uses: anthropics/claude-code-action@v1 + env: + GH_TOKEN: ${{ steps.app-token.outputs.token }} + with: + anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} + # Authenticating as the pipeline App (not GITHUB_TOKEN) is what makes ci.yml re-run on + # the agent's push -- the same reason the regenerate job uses it to open the PR. + github_token: ${{ steps.app-token.outputs.token }} + # This workflow is triggered by repository_dispatch from the backend, so the actor is + # always this App and the action's human-actor check would reject every run. Named + # explicitly rather than `*`: the repo is public, where `*` lets any external App in. + allowed_bots: "onepin-pipeline-bot[bot]" + claude_args: | + --model claude-sonnet-5 + --max-turns 40 + --allowedTools "Read,Edit,Write,Glob,Grep,Bash" + prompt: | + `tests/build` is failing on regen PR #${{ needs.regenerate.outputs.pr_number }}, + which you have checked out. Read AGENTS.md first — it is authoritative here. + + Run `uv sync --all-extras && uv run pytest tests/build -q` to see the failures. + Read them from that output, not from anything quoted to you. + + Fix EXACTLY ONE class of failure: SDK -> CLI drift, which is + `test_no_unexposed_sdk_params` or `test_no_unexposed_composite_sdk_params` reporting + that the generated client accepts a parameter no CLI flag reaches. The fix is to + expose it: + + 1. Add an `Opt(...)` to the command's row in `src/onepin/_cli/_spec.py`, matching the + style and `transform=` of the neighbouring filters. For a composite, add the + option in `src/onepin/_cli/commands/composites.py` and the param to + `_COMPOSITE_CALLS` in the test. + 2. The CLI surface changed, so regenerate the docs and the snapshot or two other + tests fail: `uv run python scripts/gen_cli_docs.py` and + `UPDATE_SNAPSHOT=1 uv run pytest tests/cli/test_cli_manifest.py`. + 3. Add a test under `tests/cli/` that exercises the new flag. PRs are gated on 90% + diff coverage. + 4. Verify before you finish: `uv run ruff check src/onepin/_cli tests`, + `uv run ruff format src/onepin/_cli tests`, `uv run mypy -p onepin._cli`, + `uv run pytest -q`. All must pass. + 5. Commit to this branch with a `feat(cli):` subject describing the flag. + + Never do any of the following. Each one hides the problem instead of fixing it, and + a human has to make the call: + - Adding an entry to `_INTENTIONALLY_UNEXPOSED`, `_COMPOSITE_UNEXPOSED` or + `_REFERENCE_DEFECTS`. Deciding NOT to expose something is a product decision. + - Weakening, skipping, deleting or `xfail`-ing any test. + - Editing anything under `src/onepin/` outside `_cli/`, including `reference.md`. + It is Fern-generated and your edit is lost on the next regen. + + If the failures are NOT drift — a `test_reference_params_match_signature` mismatch, a + renamed or dropped SDK method, anything else — do not attempt a fix. Post a PR + comment with `gh pr comment` explaining what the failure is and why it needs a human, + and stop. + + One more thing to raise rather than decide: if the parameter you exposed is a filter + whose absence changes what the response MEANS, an API older than the parameter + ignores it and answers 200 with an ungated result, so nothing errors and the rows + silently describe something else. AGENTS.md calls this out; `--buildable` needed a + `Cmd(gate=)` handler in `_gates.py` for it. Say in your PR comment whether you think + this parameter needs one, and ask for a human decision — do not add a gate yourself. notify-failure: if: failure() diff --git a/AGENTS.md b/AGENTS.md index 225c026..4927fc3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,6 +30,8 @@ Files under `src/onepin/` (excluding `_cli/`) are generated by [Fern](https://bu **If you need to change generated behavior, change the upstream OpenAPI spec.** Generation runs *in this repo*: `.github/workflows/regen.yml` fetches the shaped spec and runs `fern generate --local` (config in `fern/`), then opens a regen PR. +A regen PR is a proposal, not a release. `regen.yml` never blocks PR creation on `tests/build`: it runs the suite, posts the verdict onto the PR as a comment, and closes any older `auto:sdk-regen` PR it supersedes (each regen is a whole-SDK snapshot, so two open at once is a way to merge the SDK backwards). When the failure is SDK→CLI drift — a generated param no CLI flag reaches — the `autofix` job hands it to an agent that adds the `Opt(...)`, regenerates the README block and the manifest snapshot, and pushes to the PR branch. The agent is explicitly barred from allowlisting a guard or touching generated files; **the merge gate is `ci.yml`, not `regen.yml`.** Autofix needs the `ANTHROPIC_API_KEY` secret and is skipped with a notice when it is absent. + `src/onepin/.fernignore` lists the hand-rolled paths Fern preserves across regeneration (`_cli/`, `_version_gate.py`, `py.typed`). After every regen, `scripts/post_fern.sh` restores the PEP 561 markers (CI runs it automatically; local regens require running it by hand). `tests/build/test_cli_preserved.py` guards both. If a patch on a generated file becomes unavoidable, add it to `src/onepin/.fernignore` with a comment explaining why, and document the patch in this file under a new "Active patches" section. From adb302cadf77506bb97acda7b25847f4521b1bae Mon Sep 17 00:00:00 2001 From: Eunseo Lee Date: Thu, 1 Oct 2026 17:40:07 +0900 Subject: [PATCH 3/3] fix(ci): authenticate autofix with the org CLAUDE_CODE_OAUTH_TOKEN `ANTHROPIC_API_KEY` does not exist anywhere in this org, so autofix would have been skipped indefinitely waiting for a secret nobody had added. `CLAUDE_CODE_OAUTH_TOKEN` is already an org secret visible to this repo, so the job works on the next regen with no setup, and bills the subscription rather than the API. The tradeoff is that an OAuth token is tied to the account that ran `claude setup-token`, where an API key is not. Revoking or expiring it turns the autofix job red rather than skipped -- contained, since the guard verdict is posted by the regenerate job either way. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/regen.yml | 13 ++++++++----- AGENTS.md | 2 +- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/.github/workflows/regen.yml b/.github/workflows/regen.yml index f4520f3..9aae7f2 100644 --- a/.github/workflows/regen.yml +++ b/.github/workflows/regen.yml @@ -13,7 +13,7 @@ name: Regenerate SDK # A guard failure never blocks the PR. `tests/build` runs on both paths and reports into the # PR as a comment; when the failure is SDK -> CLI drift, the `autofix` job hands it to an agent # that adds the missing flag on the PR branch. The merge is gated by ci.yml, not by this file. -# `autofix` is skipped, with a notice, when ANTHROPIC_API_KEY is not configured. +# `autofix` is skipped, with a notice, when CLAUDE_CODE_OAUTH_TOKEN is not configured. # # Requires a GitHub App (org secrets PIPELINE_APP_ID / PIPELINE_APP_PRIVATE_KEY) and the # repo secret SPEC_REPO (name of the backend repo). Fetches public-spec.yaml -- the @@ -343,10 +343,10 @@ jobs: - name: Check for the Claude credential id: key env: - ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + CLAUDE_CODE_OAUTH_TOKEN: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} run: | - if [ -z "${ANTHROPIC_API_KEY:-}" ]; then - echo "::notice::ANTHROPIC_API_KEY is not configured; skipping autofix. The guard results are still on the PR." + if [ -z "${CLAUDE_CODE_OAUTH_TOKEN:-}" ]; then + echo "::notice::CLAUDE_CODE_OAUTH_TOKEN is not configured; skipping autofix. The guard results are still on the PR." echo "present=false" >> "$GITHUB_OUTPUT" else echo "present=true" >> "$GITHUB_OUTPUT" @@ -381,7 +381,10 @@ jobs: env: GH_TOKEN: ${{ steps.app-token.outputs.token }} with: - anthropic_api_key: ${{ secrets.ANTHROPIC_API_KEY }} + # The org already provides this, so autofix needs no new secret. It bills the + # subscription rather than the API. Note it is tied to the account that ran + # `claude setup-token`: revoke or expire that and this job starts failing. + claude_code_oauth_token: ${{ secrets.CLAUDE_CODE_OAUTH_TOKEN }} # Authenticating as the pipeline App (not GITHUB_TOKEN) is what makes ci.yml re-run on # the agent's push -- the same reason the regenerate job uses it to open the PR. github_token: ${{ steps.app-token.outputs.token }} diff --git a/AGENTS.md b/AGENTS.md index 4927fc3..429fed1 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -30,7 +30,7 @@ Files under `src/onepin/` (excluding `_cli/`) are generated by [Fern](https://bu **If you need to change generated behavior, change the upstream OpenAPI spec.** Generation runs *in this repo*: `.github/workflows/regen.yml` fetches the shaped spec and runs `fern generate --local` (config in `fern/`), then opens a regen PR. -A regen PR is a proposal, not a release. `regen.yml` never blocks PR creation on `tests/build`: it runs the suite, posts the verdict onto the PR as a comment, and closes any older `auto:sdk-regen` PR it supersedes (each regen is a whole-SDK snapshot, so two open at once is a way to merge the SDK backwards). When the failure is SDK→CLI drift — a generated param no CLI flag reaches — the `autofix` job hands it to an agent that adds the `Opt(...)`, regenerates the README block and the manifest snapshot, and pushes to the PR branch. The agent is explicitly barred from allowlisting a guard or touching generated files; **the merge gate is `ci.yml`, not `regen.yml`.** Autofix needs the `ANTHROPIC_API_KEY` secret and is skipped with a notice when it is absent. +A regen PR is a proposal, not a release. `regen.yml` never blocks PR creation on `tests/build`: it runs the suite, posts the verdict onto the PR as a comment, and closes any older `auto:sdk-regen` PR it supersedes (each regen is a whole-SDK snapshot, so two open at once is a way to merge the SDK backwards). When the failure is SDK→CLI drift — a generated param no CLI flag reaches — the `autofix` job hands it to an agent that adds the `Opt(...)`, regenerates the README block and the manifest snapshot, and pushes to the PR branch. The agent is explicitly barred from allowlisting a guard or touching generated files; **the merge gate is `ci.yml`, not `regen.yml`.** Autofix authenticates with the org-level `CLAUDE_CODE_OAUTH_TOKEN` secret (subscription billing, no repo secret to add) and is skipped with a notice when it is absent. `src/onepin/.fernignore` lists the hand-rolled paths Fern preserves across regeneration (`_cli/`, `_version_gate.py`, `py.typed`). After every regen, `scripts/post_fern.sh` restores the PEP 561 markers (CI runs it automatically; local regens require running it by hand). `tests/build/test_cli_preserved.py` guards both.