ci: automated public-API spec conformance validation#988
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR introduces an automated spec conformance validation system that detects API drift between the library's public-API client and Ubiquiti's Integration OpenAPI specification. The system runs once per Protect release, reports endpoint/field/enum mismatches via CI, and maintains a version marker file tracking the last validated release. ChangesSpec Conformance Validation
Sequence Diagram(s)sequenceDiagram
participant Workflow as GitHub Actions
participant Fetch as fetch_openapi.py
participant Validate as validate_spec.py
participant Registry as Decorator Registry
participant Spec as OpenAPI Spec
participant Report as Drift Report
Workflow->>Fetch: --print-version
Fetch-->>Workflow: resolved version
Workflow->>Workflow: check marker & issues
alt version unchanged
Workflow-->>Workflow: exit (short-circuit)
end
Workflow->>Fetch: fetch spec for version
Fetch-->>Spec: download integration.json
Workflow->>Validate: run validation
Validate->>Registry: derive covered endpoints
Validate->>Spec: check spec against coverage
Validate->>Validate: compare model fields
Validate->>Validate: check enum values
Validate->>Report: format summary
Report-->>Workflow: markdown report + exit code
alt validation success
Workflow->>Workflow: open version-bump PR
else validation error
Workflow->>Workflow: open/update drift issue
Workflow->>Workflow: close older issues
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 🚀 New features to boost your workflow:
|
Collaborator
|
Hold — resequenced behind #989. Review of this PR's Once #989 lands, this PR should be reworked to:
Everything else here (workflow, marker, dedupe state machine, field/required/enum checks) stays as-is. |
This was referenced Jun 10, 2026
Collaborator
|
@bluetoothbot fix #990 is merged — the declarative
Everything else here (workflow, |
Collaborator
|
@bluetoothbot fix The
Keep the rest (workflow, |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds automated CI validation to detect drift between this library’s hand-maintained Public Integration API surface and Ubiquiti’s Integration OpenAPI spec, while ensuring the spec itself is never committed (only a validated-version marker is tracked).
Changes:
- Introduces
scripts/validate_spec.pyto run endpoint/model/required/enum conformance checks and emit a markdown report (non-zero exit on errors). - Adds a scheduled + manual GitHub Actions workflow to validate at most once per Protect release and open either a marker-bump PR (green) or a drift issue (red).
- Adds unit tests for all validation branches and wires the same checks into the existing opt-in schema conformance test when a spec is present.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
scripts/validate_spec.py |
Implements spec drift checks and report formatting used by both CI and tests. |
scripts/fetch_openapi.py |
Adds --print-version to resolve a target Protect version without downloading the deb. |
.github/workflows/spec-validation.yml |
New scheduled/manual workflow to run validation once per release and manage PR/issue outcomes. |
openapi/.validated-version |
Adds the tracked “last validated” version marker. |
tests/test_validate_spec.py |
New network-free unit tests covering validation logic branches. |
tests/test_public_schema_conformance.py |
Reuses validation helpers when a spec is present and adds a spec-free completeness test. |
tests/conftest.py |
Exposes scripts/ on sys.path so tests can import validate_spec. |
AGENTS.md |
Documents the new validation script, marker, and workflow behavior. |
.gitignore |
Keeps the fetched spec ignored while allowing the marker file to be committed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/spec-validation.yml:
- Line 42: Update the workflow to remove unpinned action references and disable
credential persistence: replace actions/checkout@v6 and actions/setup-python@v6
with specific commit SHA pins (use the corresponding official repo commit
hashes) and set persist-credentials: false for the checkout step so credentials
are not written to the runner; ensure both action usages (the checkout step and
the setup-python step) are pinned to commit hashes rather than version tags.
- Around line 111-112: The fetch command should protect the user input from
shell word-splitting; update the invocation of fetch_openapi.py to pass the
version argument as a single quoted string (e.g. --version '${{ inputs.version
}}') so the value of inputs.version is not subject to shell splitting or
globbing. Modify the line that calls "poetry run python scripts/fetch_openapi.py
--version ..." to wrap the expansion in quotes, referencing the inputs.version
expansion and the fetch_openapi.py invocation to locate the change.
- Around line 62-63: The workflow directly expands ${{ inputs.version }} into
the shell command causing potential shell injection; fix by first assigning the
input to a quoted shell variable (e.g., version="${{ inputs.version }}") and
then use the quoted variable ("$version") when calling scripts/fetch_openapi.py
to populate latest, and apply the same quoted-variable pattern to the other
occurrence that calls scripts/fetch_openapi.py (the block that sets latest/uses
inputs.version later).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 89753f9f-333a-4203-b6a7-c4b8ce7124df
📒 Files selected for processing (9)
.github/workflows/spec-validation.yml.gitignoreAGENTS.mdopenapi/.validated-versionscripts/fetch_openapi.pyscripts/validate_spec.pytests/conftest.pytests/test_public_schema_conformance.pytests/test_validate_spec.py
Contributor
Author
|
Rework complete —
Workflow, |
1 similar comment
Contributor
Author
|
Rework complete —
Workflow, |
Collaborator
|
@bluetoothbot refactor #988 — please address all 7 open review comments:
|
Add scripts/validate_spec.py — checks the public-API client against a fetched Integration OpenAPI spec (endpoint coverage, model fields both directions, required fields, enum values), reusing the resolution helpers shared with tests/test_public_schema_conformance.py and the model classes' _get_unifi_remaps() as the remap source of truth. Add .github/workflows/spec-validation.yml — daily cron + workflow_dispatch that runs the full validation at most once per Protect release, gated by a firmware-API check and the committed openapi/.validated-version marker; opens a marker-bump PR on green or one drift issue on red. The spec is fetched ephemerally and never committed (Ubiquiti IP). Validation logic is unit-tested network-free against in-memory mock specs in tests/test_validate_spec.py; the local hook in test_public_schema_conformance.py runs the same checks when a spec is present and skips when absent.
Replace the hand-maintained _ENDPOINT_TO_METHOD table with a coverage set
derived from the import-time @public_* decorator registry plus one recorded
example call per hand-written exception method (a request spy captures
(verb, path) then short-circuits before network I/O). Placeholder names are
normalized to a bare {} so registry/spec/recorded paths compare equal.
Add check_completeness: every public-API coroutine on ProtectApiClient must
be declarative or have a recorded example call, so a newly added method that
nobody wired up fails the suite instead of silently leaving its endpoint
uncovered.
RaHehl
previously approved these changes
Jun 18, 2026
RaHehl
force-pushed
the
koan/implement-987
branch
from
5109f7a to
a0a373b
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
tests/test_validate_spec.py (1)
1-6: ⚡ Quick winUse a terse single-line module docstring here.
The current module docstring is narrative-heavy; a short summary line matches repo conventions for test modules.
As per coding guidelines, Python modules should default to terse single-line docstrings and avoid explanatory rationale in comments/docstrings unless behavior is non-obvious.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_validate_spec.py` around lines 1 - 6, Replace the multi-line module docstring at the top of the file with a single-line terse docstring that provides a brief summary of the module's purpose. Remove the narrative explanations about hand-built specs and CI details, and condense the information into a concise one-liner that matches the repository's conventions for test module docstrings. The docstring should be enclosed in triple quotes on a single line.Source: Coding guidelines
tests/test_public_schema_conformance.py (1)
22-23: ⚡ Quick winPrefer
orjsonover stdlibjsonfor spec parsing in this test.This keeps parsing behavior consistent with the validator and matches the repo’s Protect-payload serialization guideline.
Proposed fix
diff --git a/tests/test_public_schema_conformance.py b/tests/test_public_schema_conformance.py @@ -import json +import orjson @@ - schemas = json.loads(_SPEC_PATH.read_text())["components"]["schemas"] + schemas = orjson.loads(_SPEC_PATH.read_bytes())["components"]["schemas"] @@ - spec = json.loads(_SPEC_PATH.read_text()) + spec = orjson.loads(_SPEC_PATH.read_bytes())As per coding guidelines, use
orjsonfor serialization/parsing Protect payloads instead of stdlibjson.Also applies to: 92-93, 101-102
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_public_schema_conformance.py` around lines 22 - 23, Replace the stdlib `json` import with `orjson` to ensure consistent parsing behavior with the validator and comply with repository serialization guidelines. Update the import statement at the top of the file to import `orjson` instead of `json`, and then replace all `json.loads()` calls throughout the file (including the occurrences at lines 92-93 and 101-102) with `orjson.loads()` to maintain consistency across the test file for spec parsing operations.Source: Coding guidelines
scripts/validate_spec.py (1)
2-24: ⚡ Quick winDocstrings are over-detailed for this repo’s Python style contract.
These docstrings include rationale and multi-line narrative where terse, single-line summaries are expected by default.
As per coding guidelines,
**/*.pyshould use terse single-line docstrings by default and avoid rationale/motivation prose unless behavior is non-obvious.Also applies to: 325-333
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/validate_spec.py` around lines 2 - 24, The module-level docstring in the file is excessively detailed with multi-line narrative and rationale when the repo's Python style guidelines require terse single-line summaries by default. Replace the verbose docstring block (the entire multi-line docstring from lines 2-24) with a concise single-line summary describing what the script does without rationale or detailed logic explanation. Apply the same condensing approach to the other docstring around lines 325-333 mentioned in the comment review.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/spec-validation.yml:
- Around line 92-93: Add explicit `--limit` parameters to all three `gh` list
commands used for dedupe and supersede logic in the workflow to ensure complete
result retrieval beyond default pagination limits. Specifically, add `--limit`
to the `gh issue list` call on lines 92-93, the `gh pr list` call on line 137,
and the `gh issue list` call on lines 172-174. Use a sufficiently high limit
value (such as 100 or greater) to capture all potential matches for accurate
deduplication and superseding logic.
In `@scripts/validate_spec.py`:
- Around line 350-351: The code at lines 350-351 silently skips schema
validation when a schema_name is not found in schemas, but this masks real drift
for explicitly tracked schemas. Distinguish between explicitly tracked schemas
in _MODEL_SCHEMAS and _ENUM_SCHEMAS versus untracked ones: for tracked schemas
that are missing from the current schemas dict, raise an error or warning to
surface the drift instead of silently continuing; only use continue for schemas
that are not explicitly tracked (private-only or untracked). Apply the same
logic to the similar code block referenced at lines 381-383.
---
Nitpick comments:
In `@scripts/validate_spec.py`:
- Around line 2-24: The module-level docstring in the file is excessively
detailed with multi-line narrative and rationale when the repo's Python style
guidelines require terse single-line summaries by default. Replace the verbose
docstring block (the entire multi-line docstring from lines 2-24) with a concise
single-line summary describing what the script does without rationale or
detailed logic explanation. Apply the same condensing approach to the other
docstring around lines 325-333 mentioned in the comment review.
In `@tests/test_public_schema_conformance.py`:
- Around line 22-23: Replace the stdlib `json` import with `orjson` to ensure
consistent parsing behavior with the validator and comply with repository
serialization guidelines. Update the import statement at the top of the file to
import `orjson` instead of `json`, and then replace all `json.loads()` calls
throughout the file (including the occurrences at lines 92-93 and 101-102) with
`orjson.loads()` to maintain consistency across the test file for spec parsing
operations.
In `@tests/test_validate_spec.py`:
- Around line 1-6: Replace the multi-line module docstring at the top of the
file with a single-line terse docstring that provides a brief summary of the
module's purpose. Remove the narrative explanations about hand-built specs and
CI details, and condense the information into a concise one-liner that matches
the repository's conventions for test module docstrings. The docstring should be
enclosed in triple quotes on a single line.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 448ebe62-bf2f-4e8f-a6af-83bd3b38e0b4
📒 Files selected for processing (9)
.github/workflows/spec-validation.yml.gitignoreAGENTS.mdopenapi/.validated-versionscripts/fetch_openapi.pyscripts/validate_spec.pytests/conftest.pytests/test_public_schema_conformance.pytests/test_validate_spec.py
✅ Files skipped from review due to trivial changes (1)
- openapi/.validated-version
🚧 Files skipped from review as they are similar to previous changes (3)
- .gitignore
- tests/conftest.py
- scripts/fetch_openapi.py
Collaborator
|
@bluetoothbot fix — please apply CodeRabbit's two actionable findings plus the orjson nitpick, and skip the docstring nitpicks:
Please leave the terse-docstring nitpicks — the conformance architecture is non-obvious, so the multi-line docstrings are justified. Keep the |
- add explicit --limit 200 to all gh issue/pr list calls so dedupe and supersede checks no longer rely on the CLI default of 30 - treat a tracked _MODEL_SCHEMAS/_ENUM_SCHEMAS entry absent from the spec as an error (real drift) instead of a silent skip - use orjson in tests/test_public_schema_conformance.py per repo convention
Contributor
Author
|
Applied the three actionable findings (skipped docstring nitpicks per your note):
|
1 similar comment
Contributor
Author
|
Applied the three actionable findings (skipped docstring nitpicks per your note):
|
RaHehl
previously approved these changes
Jun 19, 2026
RaHehl
approved these changes
Jun 19, 2026
RaHehl
approved these changes
Jun 19, 2026
RaHehl
left a comment
RaHehl
left a comment
Collaborator
There was a problem hiding this comment.
Re-approving after the shell-injection fix (version input now passed via env). All conformance checks green; SHA-pin declined for repo-convention/credential-push reasons.
RaHehl
approved these changes
Jun 19, 2026
RaHehl
left a comment
RaHehl
left a comment
Collaborator
There was a problem hiding this comment.
Re-approving after the shell-injection fix.
RaHehl
added a commit
that referenced
this pull request
Jun 19, 2026
…#1015) #1012 decorated get_alarm_hubs_public, get_alarm_hub_public and update_alarm_hub_public with @public_get/@public_patch, making them declarative. Their endpoints are now covered via the registry, so they must be removed from the recorded-example-call table. #1012 branched before #988 (the conformance test), so its CI never ran test_example_calls_match_nondeclarative_coroutines; the two PRs were each green against their base but the combination left main red. This restores the table to exactly the non-declarative public coroutines.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds automated drift detection between the hand-maintained public-API client and Ubiquiti's Integration OpenAPI spec, running at most once per Protect release. The spec itself is never committed (Ubiquiti IP) — only a bare
openapi/.validated-versionmarker is tracked.Closes #987
Changes
scripts/validate_spec.py— four conformance checks (endpoint coverage table, model fields both directions,requiredfields, enum values), reusing the resolution helpers shared withtests/test_public_schema_conformance.pyand each model's_get_unifi_remaps()as the remap source of truth. Prints a markdown summary; exits non-zero on any error..github/workflows/spec-validation.yml— daily cron +workflow_dispatch(optionalversion,dry_run). Queries the firmware-latest API first and short-circuits while the marker is current or a drift issue is already open, so the ~74 MB deb is fetched only when a release actually drifts. Opens a marker-bump PR on green or one drift issue on red (closing superseded ones).openapi/.validated-version(7.1.77) — the only committed artifact;.gitignorekeepsopenapi/integration.jsonignored while un-ignoring the marker.scripts/fetch_openapi.py—--print-versionflag (resolve version, no download) for the workflow's cheap steady-state check.tests/test_validate_spec.py— network-free unit tests over in-memory mock specs covering every check branch (new endpoint, removed field, missing-required, new enum, all-green).tests/test_public_schema_conformance.py— extended to run the same checks when a spec is present; still skips when absent (CI default).AGENTS.md— documents the validation script, marker, and workflow.Design note
The plan specified
required-vs-optional mismatches as errors, but this library deliberately models every public-API field optional (older firmware and partial/reference responses omit them — documented convention). A hard error would make validation perpetually red and unreachable for the green marker-bump path, socheck_requiredemits warnings instead. Rationale documented inline.Test plan
poetry run pytest tests/test_validate_spec.py— 23 pass, no network.poetry run pytest tests/test_public_schema_conformance.py— skips cleanly with no spec; runs the full checks when a spec is present.ruff check/ruff format --checkclean on all changed files.git check-ignore openapi/integration.jsonstill reports it ignored;openapi/.validated-versionis trackable.Quality Report
Changes: 9 files changed, 1067 insertions(+), 51 deletions(-)
Code scan: 3 issue(s) found
scripts/fetch_openapi.py:125— debug print statementscripts/validate_spec.py:445— debug print statementscripts/validate_spec.py:454— debug print statementTests: failed (FAILED)
Branch hygiene: 1 issue(s)
Generated by Kōan
Summary by CodeRabbit
Chores
New Features
Documentation
Tests