Fail fast on a missing or misnamed OpenAPI path parameter - #31
Draft
jimador wants to merge 1 commit into
Draft
Conversation
`resolvePath` skipped substitution silently when a path parameter's value
was null, leaving the literal placeholder in the built URI. A learned
streaming API's `getShow` operation (`GET /shows/{id}`) hit this in
production: called without `id`, the tool sent `GET .../shows/{id}` to
the remote as-is, which 404'd every time. Nothing in the response or the
logs pointed at the missing argument -- it just looked like the API had
no data.
That request could never have succeeded. OpenAPI 3 requires `in: path`
parameters to be `required: true`, so a missing one isn't optional data,
it's a call to the wrong URL. `resolvePath` now throws as soon as a
declared path parameter has no value, naming the parameter, the
operation, and the argument keys that were actually supplied -- enough
for the caller (often an LLM re-driving the call) to correct itself
instead of retrying the same dead end.
Query and header parameters are untouched; this only tightens path
parameters, which the spec already marks required.
Three new tests in OpenApiOperationToolTest cover a missing `id`, a
misnamed key (`imdbId` instead of `id`), and confirm a correctly
supplied call still builds the substituted URL unchanged. 164 tests
green in embabel-api-client.
Signed-off-by: James Dunnam <7660553+jimador@users.noreply.github.com>
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.
Problem
OpenApiOperationTool.resolvePathskips substitution silently when a pathparameter's value is null, leaving the literal placeholder in the built URI.
A learned streaming API's
getShowoperation (GET /shows/{id}) hit this ina downstream consumer: called without
id, the tool sentGET .../shows/{id}to the remote as-is, which 404'd every time. Nothing in the response or the
logs pointed at the missing argument — it read as "the API has no data."
That request can never succeed. OpenAPI 3 requires
in: pathparameters to berequired: true, so a missing one isn't optional data, it's a call to thewrong URL.
Fix
resolvePaththrows as soon as a declared path parameter has no value, namingthe parameter, the operation, and the argument keys that were actually
supplied — enough for the caller (often an LLM re-driving the call) to correct
itself instead of retrying the same dead end.
Query and header parameters are untouched; this only tightens path parameters,
which the spec already marks required.
Tests
Three new cases in
OpenApiOperationToolTest: missingid, misnamed key(
imdbIdinstead ofid), and a correctly supplied call still building thesubstituted URL unchanged. Red-first: the missing-
idcase was confirmedsending the percent-encoded literal (
%7Bid%7D) to the mock before the fix.164 tests green in
embabel-api-client.