Skip to content

[FIX] Align OpenAPI spec with API validation - #1263

Open
kaihere14 wants to merge 2 commits into
CCExtractor:masterfrom
kaihere14:fix/openapi-spec-validation
Open

kaihere14 wants to merge 2 commits into
CCExtractor:masterfrom
kaihere14:fix/openapi-spec-validation

Conversation

@kaihere14

@kaihere14 kaihere14 commented Nov 6, 2026 •

Copy link
Copy Markdown

In raising this pull request, I confirm the following (please check boxes):

  • I have read and understood the contributors guide.
  • I have checked that another pull request for this purpose does not exist.
  • I have considered, and confirmed that this submission will be valuable to others.
  • I accept that this submission may not be used, and the pull request closed at the will of the maintainer.
  • I give this submission freely, and claim no ownership to its content.

My familiarity with the project is as follows (check one):

  • I have never used the project.
  • I have used the project briefly.
  • I have used the project extensively, but have not contributed previously.
  • I am an active contributor to the project.

Replaces #1242, which was pointed at the outdated GSoC-22-dev branch by mistake. This PR targets master, rebased on the current tip, and contains only the 2 intended commits.

Problem

Several parts of openapi-ci-api.yaml documented request values that the API rejects with a 400, so any client following the spec, including the spec's own examples, got errors. It started with GET /runs?status=, and a full pass over all 79 operations found the same mismatch elsewhere.

Filters the spec advertised but the API rejected

Endpoint Documented value API behaviour
GET /runs?status= pass, fail, incomplete 400. Only queued, running and canceled can be filtered on.
GET /runs/{run_id}/progress?status= queued 400. TestStatus has no queued value.
GET /runs/{run_id}/samples?status= skipped, running 400. derive_sample_status never produces either.
GET /runs/{run_id}/samples/{sample_id}/logs?cursor= any cursor 400 (Cannot mix cursor and offset pagination.). The route used offset pagination.

Smaller mismatches

  • validate_path_id rejects IDs above 2147483647, but no path ID in the spec had a maximum.
  • Several body fields reject an empty string (minLength: 1 in the schemas) without the spec saying so. The password fields enforce the configured 10–500 length range, which was also undocumented.
  • An unknown ccx_version returns 400, which was not documented.
  • GET /runs/{run_id}/artifacts?type= left out actual_output, although the API returns it and filters on it.

Responses that violated the spec

  • Run.status and RunSummary.status did not list error, although the API returns it (_check_completed_run_status).
  • POST /runs/{run_id}/restart returns action: restart, but RunActionResult.action only allowed cancel.
  • GET /auth/me returns 7 scopes, but scopes had maxItems: 6.

Why mostly the spec, not the code

pass, fail, error and incomplete are not stored in the database. They are computed per request from every sample's TestResult rows once a run completes, so they cannot be expressed as a SQL filter without deriving status for every run before pagination. Per-sample statuses work the same way. In each case the code already deliberately limits filters to values it can actually match, so the spec was the side that was wrong.

The code changed in three places:

  • The per-sample logs route now uses cursor pagination, as the spec documents and as /runs/{run_id}/logs already does.
  • The /progress 400 message is now built from TestStatus. It used to list queued and error as allowed, and both were rejected.
  • validate_path_id now covers the int-converted path IDs that skipped it, so oversized values return 400 instead of reaching the database.

Changes

openapi-ci-api.yaml

  • RunStatus enum narrowed to [queued, running, canceled], with example: running and a description explaining why outcome statuses cannot be filtered on.
  • Run.status and RunSummary.status: added error.
  • /runs/{run_id}/progress status: dropped queued and added a description.
  • /runs/{run_id}/samples status: narrowed to [pass, fail, missing_output, not_started], matching the /history filter.
  • maximum: 2147483647 added to all 17 integer path IDs and to the sample_id filter on /regression-tests.
  • minLength: 1 added to PATCH /auth/me name and current_password, PATCH /samples/{sample_id} tag items and version, POST /tags name, finalize version, and the password-reset mac. Both password fields now document the 10–500 length range.
  • ccx_version: documented the 400 for unknown versions.
  • Artifacts type filter: added actual_output.
  • RunActionResult.action: added restart.
  • CurrentUser.scopes: maxItems raised from 6 to 7.

Routes

  • mod_api/routes/errors_logs.py: get_sample_logs uses validate_cursor_pagination.
  • mod_api/routes/runs.py: the /progress status error lists the real TestStatus values.
  • mod_api/routes/{regression_tests,samples,auth,system}.py: validate_path_id added for output_id, variant_id, extra_id, token_id and the blocked-users user_id. An ID of 0 or one above 2147483647 now returns 400, matching every other ID route.

Tests

  • New tests/api/test_openapi_contract.py. It reads the spec, calls every GET operation with each documented enum value, boolean and pagination bound against the data BaseTestCase seeds, and fails on any 400 or 5xx. It runs with the existing nose2 suite, so it needs no new dependency.

Testing

Run on Python 3.14 after rebasing onto master at 02d0297:

  • The new test passes on this branch. Against this PR's first commit it fails on the /progress, /samples and sample-logs cases. Against master it also fails on /runs?status=pass|fail|incomplete.
  • The full nose2 suite passes (811 tests).
  • dodgy, isort, pydocstyle, mypy and pycodestyle pass with the CI configs.
  • The spec validates with openapi-spec-validator.

Not changed

  • Response enums that list more values than the API returns (ProgressEvent.status, RunSample.status, SampleHistoryEntry.status). Clients cannot break on these, so I left them alone.
  • /runs/{run_id}/logs defaults to 100 lines while the shared Limit parameter documents a default of 50. Nothing is rejected, so this is left for a separate change.

GET /runs only filters on statuses that map directly to the latest
TestProgress row (queued, running, canceled). pass, fail, error and
incomplete are not stored in the database: they are computed per
request from every sample's TestResult rows once a run completes, so
they cannot be used as a query filter. The RunStatus parameter
advertised all six values (with example: pass), so spec-conforming
clients got a 400. Narrow the enum and document why.

Run.status and RunSummary.status were missing "error", which the API
already returns when a completed run reported no results or fewer
samples than configured. Add it to both enums.
A full pass over all 79 operations found more places where the API
rejects a request the spec documents as valid:

- GET /runs/{run_id}/progress advertised status=queued, but TestStatus
  has no queued value, so it always returned 400. A queued run has no
  progress events to filter. Drop it from the enum, and build the 400
  message from TestStatus instead of listing queued and error, which
  were both rejected.
- GET /runs/{run_id}/samples advertised status=skipped and running,
  which derive_sample_status never produces and the route rejects.
  Narrow the enum to the four statuses the history filter already uses.
- GET /runs/{run_id}/samples/{sample_id}/logs documents cursor
  pagination, but the route used the offset decorator, which rejects
  any cursor. Switch it to validate_cursor_pagination.

Smaller mismatches in the same direction:

- validate_path_id rejects IDs above 2147483647, but no path ID in the
  spec had a maximum. Add it to all of them, and to the sample_id filter
  on /regression-tests. Apply validate_path_id to the int-converted IDs
  that skipped it (output_id, variant_id, extra_id, token_id, blocked
  user_id), where oversized values reached the database and raised.
- Document the minLength: 1 the schemas enforce on several body fields,
  and the configured 10-500 password bounds.
- Document the 400 for an unknown ccx_version.
- Add actual_output to the artifacts type filter; the API returns and
  filters on it.

Responses that broke the spec: restart returns action=restart, which
RunActionResult did not allow, and /auth/me returns seven scopes against
a maxItems of 6.

Add tests/api/test_openapi_contract.py, which calls every GET operation
with each documented enum, boolean and pagination bound value and fails
on a 400 or 5xx. It fails on all three bugs above against the previous
commit, and on the original /runs status bug against the one before.
@sonarqubecloud

sonarqubecloud Bot commented Nov 6, 2026

Copy link
Copy Markdown

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant