Issue #3620616: The HTTP surface does not match its published contract
Fixes the findings on #3620616, one commit each. Read them as commits: each says which side of the disagreement is authoritative and why, and names the failure its test reproduces.
Two findings did not survive verification and are answered in their commits rather than "fixed":
listInstances()newest-first.EntityStorageBase::loadMultiple()re-keys its result to the IDs it was passed, so the query's sort order does reach the caller whatever the static cache held. The code and the documents are both right. What was missing is a pin, since the one pagination test compares canonically; that pin is added.- The 422 branch on
getVariablesis reachable, not dead: the controller depends on the interface, and the remote adapter maps a transport failure or a non-2xx from the far side ontoOrchestraClientException. Removing it would have turned that into a 500. What was actually wrong is thatgetInstance(),listInstances()andgetVariables()declared no@throwswhile every implementation of them can raise, which is what makes the branch look dead. The interface now declares them.
One finding is narrower than reported: following a redirect on an API call is an integrity problem, not a bearer-token leak, because Guzzle strips Authorization across origins and a scheme change counts as cross-origin. The token request IS a credential leak, since the client secret travels in the body and a 307 carries it. Both now refuse redirects.
One defect not on the issue is fixed here too: docs/openapi.yaml was not valid YAML. Six lines put an unquoted comma-bearing scalar inside a flow mapping, which Symfony refuses outright and a lenient parser mis-reads into a truncated description plus three invented schema keys. Nothing had ever read the file, so OpenApiDocumentTest now does.
Verification
Every fix has a test that fails against unmodified code. The test-only changes job shows sixteen of them failing, each for its own reason. Four tests pass there by design, because they pin a contract rather than change behaviour: the newest-first order, the two page-size numbers, the reachability of the 422 branch, and the permission-denial coverage.
The last four commits come from an audit of this branch before merge, and two of them fix defects introduced by the branch itself: a shape check that rejected a valid response containing a variable named "0", and a body decoder whose docblock was ahead of its code. A third removes reflection from a test setUp() that was making a whole class error under test-only changes (12 tests, 0 assertions) instead of failing two tests for their own reasons.
Note for review
getInstance() now returns only live tokens, so a caller that was reading terminal ones sees fewer. That matches what the OpenAPI document and the interface always said, and it is the one behavioural change here that a downstream consumer of the @api interface could notice.
This merge request was prepared with the assistance of an AI agent (Claude). The changes and the wording were reviewed by me before pushing, and accountability for the content is mine.