From d6942f45aa94a05d1981844b34508773390cdab6 Mon Sep 17 00:00:00 2001 From: James Dunnam <7660553+jimador@users.noreply.github.com> Date: Fri, 14 Aug 2026 17:40:21 -0400 Subject: [PATCH] Fail fast on a missing or misnamed OpenAPI path parameter `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> --- .../client/openapi/OpenApiOperationTool.kt | 20 ++++- .../openapi/OpenApiOperationToolTest.kt | 73 +++++++++++++++++++ 2 files changed, 91 insertions(+), 2 deletions(-) diff --git a/embabel-api-client/src/main/kotlin/com/embabel/agent/api/client/openapi/OpenApiOperationTool.kt b/embabel-api-client/src/main/kotlin/com/embabel/agent/api/client/openapi/OpenApiOperationTool.kt index 39db515..6d61b0d 100644 --- a/embabel-api-client/src/main/kotlin/com/embabel/agent/api/client/openapi/OpenApiOperationTool.kt +++ b/embabel-api-client/src/main/kotlin/com/embabel/agent/api/client/openapi/OpenApiOperationTool.kt @@ -218,13 +218,29 @@ class OpenApiOperationTool( return declared + bodyProps } + /** + * Substitute every `{param}` placeholder in the path with the caller's argument. + * + * OpenAPI 3 requires path parameters to be `required: true`, so a missing or + * misnamed one is not optional data -- it's a call that cannot possibly reach + * the right resource. We throw here rather than let the literal placeholder + * survive into the URI: a request for `/shows/{id}` sent as-is is guaranteed + * to 404, and that 404 tells the caller nothing about which argument was + * missing. Naming the parameter and the keys actually supplied turns a dead + * end into something the caller (often an LLM) can correct and retry. + */ private fun resolvePath(path: String, params: Map): String { var resolved = path pathParameterNames().forEach { paramName -> val value = params[paramName] - if (value != null) { - resolved = resolved.replace("{$paramName}", value.toString()) + if (value == null) { + throw IllegalArgumentException( + "Missing required path parameter '$paramName' for operation " + + "'${operation.operationId ?: path}' ($httpMethod $path). " + + "Provided arguments: ${params.keys}", + ) } + resolved = resolved.replace("{$paramName}", value.toString()) } return resolved } diff --git a/embabel-api-client/src/test/kotlin/com/embabel/agent/api/client/openapi/OpenApiOperationToolTest.kt b/embabel-api-client/src/test/kotlin/com/embabel/agent/api/client/openapi/OpenApiOperationToolTest.kt index 6c50d17..2217a97 100644 --- a/embabel-api-client/src/test/kotlin/com/embabel/agent/api/client/openapi/OpenApiOperationToolTest.kt +++ b/embabel-api-client/src/test/kotlin/com/embabel/agent/api/client/openapi/OpenApiOperationToolTest.kt @@ -1447,6 +1447,79 @@ class OpenApiOperationToolTest { } } + // ====================================================================== + // Path parameter validation + // ====================================================================== + + @Nested + inner class PathParameterValidationTests { + + /** + * A learned streaming API's `getShow` operation declares `GET /shows/{id}`. + * Calling it without `id` used to leave the literal `{id}` in the built + * URI, which the remote 404'd on every time -- a call that could never + * succeed, surfacing as an opaque HTTP error instead of naming the + * missing argument. + */ + private fun getShowOperation() = Operation().apply { + operationId = "getShow" + parameters = listOf( + Parameter().apply { + name = "id" + `in` = "path" + required = true + schema = StringSchema() + }, + ) + } + + @Test + fun `missing path parameter fails fast instead of sending the literal placeholder`() { + val (tool, server) = createToolWithMock( + PathItem.HttpMethod.GET, "/shows/{id}", + operation = getShowOperation(), + ) + + val result = tool.call("{}") + + assertInstanceOf(Tool.Result.Error::class.java, result) + val error = result as Tool.Result.Error + assertTrue(error.message.contains("id"), "Error should name the missing parameter 'id': ${error.message}") + server.verify() // no request expectations registered -- a request would fail verification + } + + @Test + fun `misnamed path parameter fails fast and lists the provided keys`() { + val (tool, server) = createToolWithMock( + PathItem.HttpMethod.GET, "/shows/{id}", + operation = getShowOperation(), + ) + + val result = tool.call("""{"imdbId": "tt1234567"}""") + + assertInstanceOf(Tool.Result.Error::class.java, result) + val error = result as Tool.Result.Error + assertTrue(error.message.contains("id"), "Error should name the missing parameter 'id': ${error.message}") + assertTrue(error.message.contains("imdbId"), "Error should list the provided key 'imdbId': ${error.message}") + server.verify() // no request expectations registered -- a request would fail verification + } + + @Test + fun `correctly supplied path parameter still builds the substituted URL`() { + val (tool, server) = createToolWithMock( + PathItem.HttpMethod.GET, "/shows/{id}", + operation = getShowOperation(), + ) + server.expect(requestTo("https://api.example.com/shows/tt1234567")) + .andExpect(method(HttpMethod.GET)) + .andRespond(withSuccess("""{"id": "tt1234567"}""", MediaType.APPLICATION_JSON)) + + val result = tool.call("""{"id": "tt1234567"}""") + assertIsText(result, """{"id": "tt1234567"}""") + server.verify() + } + } + // ====================================================================== // URI building // ======================================================================