Onboarding: Generate real REST handlers for OpenAPI-discovered agents - #2760
Onboarding: Generate real REST handlers for OpenAPI-discovered agents#2760TalZaccai wants to merge 7 commits into
Conversation
Adds a buildRestHandler generator (the REST twin of the existing CLI handler generator) so agents scaffolded from a JSON OpenAPI 3 spec via parseOpenApiSpec produce an executeAction that makes a real HTTP call instead of returning a not-yet-implemented stub. - discoveryHandler.ts: capture ApiSurface.baseUrl (absolute/relative/ server-variable resolution, guarded, http/https only) and DiscoveredParameter.in; merge path-item-level params with operation-level params (op wins); skip \ path items/params (v1 inline-only scope). - restHandlerTemplate.ts + restHandler.template: per-action switch cases split params by .in (path/query/body; header/cookie unsupported v1), slash-normalized URL joining that preserves the server path, camelCase<->wire-name bracket-access fallback, scalar query/body serialization, 30s timeout, truncated/pretty-printed response handling. - scaffolderHandler.ts: guarded routing to buildRestHandler right after the CLI-detection block, only for schema-grammar/external-api patterns with a resolved baseUrl and HTTP actions; other patterns (websocket-bridge, native-platform, ...) keep their own builders. Tests (test/*.spec.ts, run via \ pm test\ / \ sx --test\): extraction+merge+baseUrl resolution from a checked-in OpenAPI fixture, approval-spread preservation of baseUrl/in, routing guard behavior, tsc type-check of generated output, and executeAction correctness against a local node:http server (GET path param, GET query param, POST JSON body). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…scheme leak
Addresses code review feedback on the parseOpenApiSpec REST handler generator:
- Fix 1 (BLOCKING): resolve local #/components/parameters/* $refs during
discovery merge instead of dropping them, so path params factored into
components (e.g. GitHub-style specs) are no longer silently lost and
substituted as literal "undefined" at runtime. Added a defense-in-depth
check in filterRestActions that excludes any action whose path template
still has an unresolved {placeholder} with no matching path param.
- Fix 2 (WARNING): gate request-body emission on a positive method allowlist
(POST/PUT/PATCH) instead of "!== DELETE", so a GET/HEAD action with a
requestBody never emits a body (which fetch() rejects at runtime).
- Fix 3 (hardening): re-validate the resolved protocol is http/https after
resolving a relative servers[0].url, so an absolute non-http(s) scheme
(ws://, ftp://, ...) can no longer leak through as a baseUrl.
Added regression tests for all three fixes (openApiExtraction.spec.ts,
restHandlerCodegenFixes.spec.ts) plus a new Tier-A deterministic end-to-end
test (restHandlerE2EBooks.spec.ts) that drives parseOpenApiSpec -> filterRestActions
-> buildRestHandler -> tsc compile -> real execution against a local node:http
server, covering a $ref-based path param and /v2 base-path preservation together.
Full suite: 30/30 node:test passing, tsc -b clean, prettier applied.
Also manually ran the full LLM-driven pipeline (discovery -> phraseGen ->
schemaGen -> grammarGen -> scaffold) against a books OpenAPI fixture with a
$ref path param, pointed at a local HTTP server with a /v2 base path
(Tier B, scratch driver, not committed): the generated handler correctly
performed GET /v2/books/book-42 (base path preserved, no "undefined") and
returned the real server response through createActionResultFromTextDisplay.
Deduplicate the filterRestActions doc block and trim comments in the REST handler generator and OpenAPI discovery path to describe code behavior concisely, without cross-file/external examples. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR upgrades the onboarding pipeline for OpenAPI-discovered integrations so that, when discovery can resolve an absolute HTTP(S) baseUrl, the scaffolder generates a deterministic fetch-based REST action handler (instead of a stub), enabling true end-to-end HTTP execution for generated agents.
Changes:
- Extend OpenAPI discovery to (a) resolve/persist a
baseUrland (b) record each parameter’s wire location (in: path/query/header/cookie/body), including inline resolution of local#/components/parameters/*refs. - Add a REST handler generator (
restHandler.template+buildRestHandler) and route scaffolding to it only for REST-shaped patterns whenapiSurface.baseUrlis available. - Add a
tsx --testtest runner and a suite of unit/integration/E2E tests covering discovery, preservation, routing, codegen correctness, compilation, and execution.
Show a summary per file
| File | Description |
|---|---|
| ts/pnpm-lock.yaml | Adds the tsx dependency to the workspace lockfile. |
| ts/packages/agents/onboarding/package.json | Adds test/test:local scripts using tsx --test and includes tsx in devDependencies. |
| ts/packages/agents/onboarding/src/scaffolder/scaffolderHandler.ts | Routes scaffolding to REST handler generation when apiSurface.baseUrl is present and the pattern is REST-appropriate. |
| ts/packages/agents/onboarding/src/scaffolder/restHandlerTemplate.ts | Introduces deterministic REST handler codegen and REST-action filtering for safe generation. |
| ts/packages/agents/onboarding/src/scaffolder/restHandler.template | New generated handler template implementing fetch requests with timeout, formatting, and error surfacing. |
| ts/packages/agents/onboarding/src/discovery/discoveryHandler.ts | Adds baseUrl + parameter in capture, local $ref param resolution, and exposes pure helpers for testing. |
| ts/packages/agents/onboarding/test/restRouting.spec.ts | Verifies buildHandler routing decisions between stub vs REST generator across patterns/baseUrl presence. |
| ts/packages/agents/onboarding/test/restHandlerExecution.spec.ts | Offline execution tests against a local HTTP server for path/query/body wiring. |
| ts/packages/agents/onboarding/test/restHandlerE2EBooks.spec.ts | End-to-end pipeline test: discovery → scaffolding → type-check → real local HTTP execution. |
| ts/packages/agents/onboarding/test/restHandlerCompile.spec.ts | Ensures generated handler type-checks via tsc --noEmit. |
| ts/packages/agents/onboarding/test/restHandlerCodegenFixes.spec.ts | Regression tests for codegen string correctness (path param substitution, no GET/DELETE bodies). |
| ts/packages/agents/onboarding/test/openApiExtraction.spec.ts | Unit tests for OpenAPI action extraction, param merging/ref resolution, and base URL resolution. |
| ts/packages/agents/onboarding/test/apiSurfacePreservation.spec.ts | Ensures baseUrl and parameter in survive discovery → approval persistence round-trip. |
| ts/packages/agents/onboarding/test/fixtures/openapi/book-api.json | Adds an OpenAPI fixture for extraction/base-url tests (relative server URL). |
| ts/packages/agents/onboarding/test/fixtures/openapi/book-api-absolute-server.json | Adds an OpenAPI fixture for persistence tests (absolute server URL). |
Review details
Files not reviewed (1)
- ts/pnpm-lock.yaml: Generated file
Comments suppressed due to low confidence (1)
ts/packages/agents/onboarding/src/scaffolder/restHandlerTemplate.ts:176
- There are two consecutive, duplicated JSDoc blocks describing filterRestActions. Keeping both is redundant and makes future edits error-prone; remove the extra block so there is only one doc comment.
/**
* Returns the subset of `actions` this generator can actually handle:
* actions from the parseOpenApiSpec arm with a recognized HTTP method and a
* `path`. Used by scaffolderHandler to decide whether to route to the REST
* generator at all.
*/
/**
- Files reviewed: 14/15 changed files
- Comments generated: 1
- Review effort level: Low
The scaffolded callRest helper passed body: requestBody (string | undefined) directly to fetch, which fails TS2379 under the repo's exactOptionalPropertyTypes, so a scaffolded REST agent would not compile in-workspace. Only set fetch's body when a request body exists. Adds a Fix 3 codegen regression test. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Extract mergeOperationParameters, extractBodyParameters, and buildActionForOperation helpers so extractOpenApiActions is just its two loops plus a push, bringing it under the CC 25 / Cog 30 CI gate. Behavior-preserving: identical output for local parameter \ resolution, path+op param merge (op overriding path), and request-body props as in: body. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Introduce a minimal structural OpenApi* type model (spec, path item, operation, parameter, schema, server) and thread it through resolveOpenApiBaseUrl, resolveLocalParameterRef, mergeOperationParameters, extractBodyParameters, buildActionForOperation, extractOpenApiActions, and the parsed-spec local in handleParseOpenApiSpec. Removes the OpenAPI extraction any usage (file no-explicit-any 21 -> 3, back under the base of 7) without disabling lint or undoing the helper extraction. Behavior is unchanged — the types describe the same parsed-JSON shape the code already read. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
| import { {{PASCAL_NAME}}Actions } from "./{{NAME}}Schema.js"; | ||
|
|
||
| const BASE_URL = {{BASE_URL}}; | ||
| const MAX_RESPONSE_CHARS = 4000; |
There was a problem hiding this comment.
Mabe move this to the aiclient and maybe spec it out based on model...right now a lot of these are just arbitrary and don't really match the selected mode. We should have a function or config that sets this that callers can use instead of embedding this constant all over the place.
There was a problem hiding this comment.
Agreed the magic 4000 shouldn't be embedded. Pulled it into a single named DEFAULT_MAX_RESPONSE_CHARS in agent-sdk (next to the shared truncateText) so it lives in one place and callers reference it. Full model-aware sizing (budget derived from the selected model) really belongs in the host/dispatcher truncating tool results — the generated REST handler has no model context — so I've tracked that as a follow-up rather than bake fake model-awareness into standalone codegen. Happy to pick it up there if you'd like it in this PR. (8503913)
Add DEFAULT_MAX_RESPONSE_CHARS and a reusable truncateText(text, maxChars?) to agent-sdk's action helpers, and have the generated REST handler template import truncateText instead of inlining its own MAX_RESPONSE_CHARS constant and truncate() function. Behavior is unchanged (same 4000-char cap and '…(truncated)' marker). Full model-aware sizing is intentionally deferred to the host/dispatcher, which knows the selected model; standalone codegen has no model context. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Summary
When the onboarding pipeline discovers an integration from an OpenAPI spec, the
scaffolder previously emitted only a stub action handler: the generated
agent could translate an utterance into an action, but the handler never called
the API. This PR adds a deterministic,
fetch-based REST handler generator soan OpenAPI-discovered agent issues real HTTP requests end to end.
What changed
Discovery (
parseOpenApiSpec) now captures the two things a REST handlerneeds:
servers[0].url, with server-variablesubstitution and relative-URL resolution against the spec's fetched
location; falls back to the spec origin when there is no
serversentry.Left unset when it can't resolve to a well-formed http/https URL.
in(path/query/header/cookie, or
bodyfor request-body fields). Path-leveland operation-level parameters are merged, and local
#/components/parameters/*$refs are resolved inline.ApiSurfaceand survive approval.Scaffolder routes to the new REST generator when the surface has a
resolved
baseUrland REST-shaped actions; otherwise it falls back to theexisting stub. Explicit non-REST patterns (websocket-bridge, native-platform)
keep their own builders.
Generated handler (from
restHandler.template) builds the request URLfrom path/query parameters, sends a JSON body for POST/PUT/PATCH, applies a
30s timeout, and returns truncated/pretty-printed responses with HTTP errors
surfaced.
Scope / v1 limitations
dropped. No auth headers. DELETE requests carry no body.
$refs (external files, deeper pointers) are skipped; an actionwhose path placeholder can't be resolved falls back to the stub handler.
Testing
Adds a
tsx --testrunner and 30 unit/integration tests: OpenAPI actionextraction, base-URL resolution, api-surface preservation, scaffolder REST
routing, codegen fixes, generated-handler compilation, and live
execution/E2E against the Open Library
/api/booksendpoint. All 30 pass.