diff --git a/ts/packages/agentSdk/src/helpers/actionHelpers.ts b/ts/packages/agentSdk/src/helpers/actionHelpers.ts index 14d0224150..21f305f94a 100644 --- a/ts/packages/agentSdk/src/helpers/actionHelpers.ts +++ b/ts/packages/agentSdk/src/helpers/actionHelpers.ts @@ -459,3 +459,19 @@ export function actionResultToString(actionResult: ActionResult): string { return fields.join("\n"); } } + +// Default cap on characters of a tool/action response body echoed back to the +// dispatcher. Conservative v1 default; not yet model-aware — a model-derived +// budget belongs in the host where the selected model is known. Callers should +// reference this instead of inlining a magic number. +export const DEFAULT_MAX_RESPONSE_CHARS = 4000; + +// Truncate `text` to at most `maxChars`, appending a marker when clipped. +export function truncateText( + text: string, + maxChars: number = DEFAULT_MAX_RESPONSE_CHARS, +): string { + return text.length > maxChars + ? text.slice(0, maxChars) + "\n…(truncated)" + : text; +} diff --git a/ts/packages/agents/onboarding/package.json b/ts/packages/agents/onboarding/package.json index 1bdd298e07..8cdc77c489 100644 --- a/ts/packages/agents/onboarding/package.json +++ b/ts/packages/agents/onboarding/package.json @@ -42,6 +42,8 @@ "clean": "rimraf --glob dist *.tsbuildinfo *.done.build.log", "prettier": "prettier --check . --ignore-path ../../../.prettierignore", "prettier:fix": "prettier --write . --ignore-path ../../../.prettierignore", + "test": "npm run test:local", + "test:local": "tsx --test test/*.spec.ts", "tsc": "tsc -b" }, "dependencies": { @@ -64,6 +66,7 @@ "copyfiles": "^2.4.1", "prettier": "^3.5.3", "rimraf": "^6.0.1", + "tsx": "^4.21.0", "typescript": "~5.4.5", "ws": "^8.21.1" }, diff --git a/ts/packages/agents/onboarding/src/discovery/discoveryHandler.ts b/ts/packages/agents/onboarding/src/discovery/discoveryHandler.ts index 062cfa4c24..f82d03a073 100644 --- a/ts/packages/agents/onboarding/src/discovery/discoveryHandler.ts +++ b/ts/packages/agents/onboarding/src/discovery/discoveryHandler.ts @@ -49,6 +49,9 @@ export type DiscoveredParameter = { type: string; description?: string; required?: boolean; + // Where the parameter lives on the wire (OpenAPI `in`, or "body" for a + // request-body property). Only set by the parseOpenApiSpec arm. + in?: "path" | "query" | "header" | "cookie" | "body"; }; export type DiscoveredEntity = { @@ -66,6 +69,58 @@ export type ApiSurface = { approved?: boolean; approvedAt?: string; approvedActions?: string[]; + // Absolute http/https base URL resolved from the OpenAPI spec's + // `servers[0].url` (parseOpenApiSpec arm only). Unset when it cannot be + // resolved to a well-formed http/https URL (see resolveOpenApiBaseUrl). + baseUrl?: string; +}; + +// Minimal structural model of the subset of an OpenAPI 3 document this handler +// reads. Deliberately partial (every field optional) since specs are parsed +// from untrusted JSON — the extraction code guards each access rather than +// assuming a valid/complete document. +type OpenApiRef = { $ref?: string }; +type OpenApiSchema = { + type?: string; + description?: string; + properties?: Record; + required?: string[]; +} & OpenApiRef; +type OpenApiParameter = { + name?: string; + in?: string; + description?: string; + required?: boolean; + schema?: OpenApiSchema; +} & OpenApiRef; +type OpenApiRequestBody = { + content?: Record; +}; +type OpenApiOperation = { + operationId?: string; + summary?: string; + description?: string; + parameters?: OpenApiParameter[]; + requestBody?: OpenApiRequestBody; +}; +type OpenApiPathItem = { + parameters?: OpenApiParameter[]; + get?: OpenApiOperation; + post?: OpenApiOperation; + put?: OpenApiOperation; + patch?: OpenApiOperation; + delete?: OpenApiOperation; +} & OpenApiRef; +type OpenApiServer = { + url?: string; + variables?: Record; +}; +type OpenApiSpec = { + openapi?: string; + swagger?: string; + paths?: Record; + components?: { parameters?: Record }; + servers?: OpenApiServer[]; }; // TypeChat translator for structured CLI help extraction @@ -359,6 +414,219 @@ function isInternalAction(name: string): boolean { return false; } +// Resolve an OpenAPI 3 spec's base URL to an absolute http/https URL, or +// return undefined when it cannot be resolved deterministically (Swagger 2 +// host/basePath/schemes, malformed/local, or missing server variable +// defaults are all left unset — the REST handler generator then falls back +// to the stub handler rather than emitting a malformed request). +export function resolveOpenApiBaseUrl( + spec: OpenApiSpec, + specSource: string, + fetchedUrl?: string, +): string | undefined { + const specIsHttp = /^https?:\/\//i.test(specSource); + // Prefer the post-redirect URL (only meaningful when we actually fetched + // over http/https); otherwise fall back to the original specSource. + const specLocation = specIsHttp ? (fetchedUrl ?? specSource) : undefined; + + const rawServerUrl: string | undefined = spec?.servers?.[0]?.url; + + if (!rawServerUrl) { + // No `servers` entry — fall back to the origin of the spec's own + // location, but only when that location is itself http/https. + if (!specLocation) return undefined; + try { + return new URL(specLocation).origin; + } catch { + return undefined; + } + } + + // Substitute server variables ({var}) from servers[0].variables[var].default. + // If any referenced variable lacks a default, bail out rather than emit + // a malformed URL containing a literal "{var}". + const variables = spec.servers?.[0]?.variables ?? {}; + let substituted = rawServerUrl; + const varNames = [...rawServerUrl.matchAll(/\{([^}]+)\}/g)].map( + (m) => m[1], + ); + for (const varName of varNames) { + const def = variables?.[varName]?.default; + if (def === undefined || def === null) return undefined; + substituted = substituted.replaceAll(`{${varName}}`, String(def)); + } + + if (/^https?:\/\//i.test(substituted)) { + // Absolute — use as-is (preserving any path component). + try { + const u = new URL(substituted); + // Strip a trailing slash only; keep the rest of the path intact. + return u.toString().replace(/\/$/, ""); + } catch { + return undefined; + } + } + + // Relative server URL (e.g. "/v3") — resolve against the spec's own + // fetched location, only when that location is http/https. + if (!specLocation) return undefined; + try { + const resolved = new URL(substituted, specLocation); + // `substituted` might itself be an absolute non-http(s) URL (e.g. + // "ws://...", "ftp://...") that didn't match the earlier + // http(s)-prefix check; guard the http/https-only guarantee here + // too rather than relying solely on the regex test above. + if (resolved.protocol !== "http:" && resolved.protocol !== "https:") { + return undefined; + } + return resolved.toString().replace(/\/$/, ""); + } catch { + return undefined; + } +} + +// Resolve a local `#/components/parameters/` JSON-pointer reference +// against the (already-parsed) spec document. Only this narrow, common +// shape is supported (deterministic inline-OpenAPI-3 subset, v1) — any +// other pointer shape (external file refs, `#/definitions/...` from +// Swagger 2, deeper paths) returns undefined and the caller treats the +// parameter as unresolvable. +function resolveLocalParameterRef( + spec: OpenApiSpec, + ref: string, +): OpenApiParameter | undefined { + const match = /^#\/components\/parameters\/([^/]+)$/.exec(ref); + if (!match) return undefined; + const resolved = spec?.components?.parameters?.[match[1]]; + // A resolved entry could itself be a `$ref` (unlikely, but guard against + // infinite loops / unsupported indirection) — only accept a fully + // inline parameter object. + if (!resolved || resolved.$ref) return undefined; + return resolved; +} + +// Merge path-level and operation-level parameters, keyed by (in, name), with +// operation-level entries overriding path-level ones. Local +// `#/components/parameters/*` refs are resolved inline so a referenced +// parameter isn't dropped; any other `$ref` shape (external files, deeper +// pointers) stays unresolved and is skipped. +function mergeOperationParameters( + spec: OpenApiSpec, + pathLevelParams: OpenApiParameter[], + op: OpenApiOperation, +): DiscoveredParameter[] { + const opLevelParams: OpenApiParameter[] = Array.isArray(op.parameters) + ? op.parameters + : []; + const mergedByKey = new Map(); + for (const raw of [...pathLevelParams, ...opLevelParams]) { + if (!raw) continue; + const p = raw.$ref ? resolveLocalParameterRef(spec, raw.$ref) : raw; + if (!p || p.$ref || !p.name) continue; + mergedByKey.set(`${p.in ?? "query"}:${p.name}`, p); + } + return Array.from(mergedByKey.values()).map( + (p): DiscoveredParameter => + ({ + name: p.name, + type: p.schema?.type ?? "string", + description: p.description, + required: p.required ?? false, + in: p.in, + // The guard loop above guarantees `name` is set and `in` is a + // valid OpenAPI location; assert the mapping since the source + // fields are optional/loosely typed on the parsed spec. + }) as DiscoveredParameter, + ); +} + +// Map an operation's JSON request-body properties to `in: "body"` parameters. +function extractBodyParameters(op: OpenApiOperation): DiscoveredParameter[] { + const requestBody = op.requestBody?.content?.["application/json"]?.schema; + if (!requestBody?.properties) return []; + return Object.entries(requestBody.properties).map( + ([propName, propSchema]): DiscoveredParameter => + ({ + name: propName, + type: propSchema.type ?? "string", + description: propSchema.description, + required: requestBody.required?.includes(propName) ?? false, + in: "body", + }) as DiscoveredParameter, + ); +} + +// Build a single DiscoveredAction from one path/method/operation. +function buildActionForOperation( + spec: OpenApiSpec, + pathStr: string, + pathLevelParams: OpenApiParameter[], + method: string, + op: OpenApiOperation, + specSource: string, +): DiscoveredAction { + const name = + op.operationId ?? `${method}${pathStr.replace(/[^a-zA-Z0-9]/g, "_")}`; + const camelName = name.replace(/_([a-z])/g, (_: string, c: string) => + c.toUpperCase(), + ); + return { + name: camelName, + description: + op.summary ?? + op.description ?? + `${method.toUpperCase()} ${pathStr}`, + method: method.toUpperCase(), + path: pathStr, + parameters: [ + ...mergeOperationParameters(spec, pathLevelParams, op), + ...extractBodyParameters(op), + ], + sourceUrl: specSource, + }; +} + +const OPENAPI_HTTP_METHODS = ["get", "post", "put", "patch", "delete"] as const; + +// Extract discovered actions from a parsed OpenAPI 3 (or Swagger 2 — +// unsupported, will simply yield no actions since `spec.paths` items won't +// have the shape below) spec document. Pure/synchronous so it can be unit +// tested without going through the workspace-state-backed action handler. +export function extractOpenApiActions( + spec: OpenApiSpec, + specSource: string, +): DiscoveredAction[] { + const actions: DiscoveredAction[] = []; + const paths: Record = spec.paths ?? {}; + for (const [pathStr, pathItem] of Object.entries(paths)) { + // Deterministic inline-OpenAPI-3 subset only — a `$ref`'d path item + // (shared path referencing a component) is out of scope for v1. + if (pathItem?.$ref) continue; + + const pathLevelParams: OpenApiParameter[] = Array.isArray( + pathItem?.parameters, + ) + ? pathItem.parameters + : []; + + for (const method of OPENAPI_HTTP_METHODS) { + const op = pathItem?.[method]; + if (!op) continue; + actions.push( + buildActionForOperation( + spec, + pathStr, + pathLevelParams, + method, + op, + specSource, + ), + ); + } + } + return actions; +} + async function handleParseOpenApiSpec( integrationName: string, specSource: string, @@ -374,6 +642,9 @@ async function handleParseOpenApiSpec( // Fetch the spec (URL or file path) let specContent: string; + // Post-redirect URL when specSource is fetched over http/https; used to + // resolve a relative `servers[0].url` against the *actual* spec location. + let fetchedUrl: string | undefined; try { if ( specSource.startsWith("http://") || @@ -385,6 +656,7 @@ async function handleParseOpenApiSpec( error: `Failed to fetch spec: ${response.status} ${response.statusText}`, }; } + fetchedUrl = response.url || specSource; specContent = await response.text(); } else { const fs = await import("fs/promises"); @@ -396,7 +668,7 @@ async function handleParseOpenApiSpec( }; } - let spec: any; + let spec: OpenApiSpec; try { spec = JSON.parse(specContent); } catch { @@ -411,75 +683,15 @@ async function handleParseOpenApiSpec( } // Extract actions from OpenAPI paths - const actions: DiscoveredAction[] = []; - const paths = spec.paths ?? {}; - for (const [pathStr, pathItem] of Object.entries(paths) as [ - string, - any, - ][]) { - for (const method of [ - "get", - "post", - "put", - "patch", - "delete", - ] as const) { - const op = pathItem?.[method]; - if (!op) continue; - - const name = - op.operationId ?? - `${method}${pathStr.replace(/[^a-zA-Z0-9]/g, "_")}`; - const camelName = name.replace( - /_([a-z])/g, - (_: string, c: string) => c.toUpperCase(), - ); - - const parameters: DiscoveredParameter[] = (op.parameters ?? []).map( - (p: any) => ({ - name: p.name, - type: p.schema?.type ?? "string", - description: p.description, - required: p.required ?? false, - }), - ); - - // Also include request body fields as parameters - const requestBody = - op.requestBody?.content?.["application/json"]?.schema; - if (requestBody?.properties) { - for (const [propName, propSchema] of Object.entries( - requestBody.properties, - ) as [string, any][]) { - parameters.push({ - name: propName, - type: propSchema.type ?? "string", - description: propSchema.description, - required: - requestBody.required?.includes(propName) ?? false, - }); - } - } - - actions.push({ - name: camelName, - description: - op.summary ?? - op.description ?? - `${method.toUpperCase()} ${pathStr}`, - method: method.toUpperCase(), - path: pathStr, - parameters, - sourceUrl: specSource, - }); - } - } + const actions: DiscoveredAction[] = extractOpenApiActions(spec, specSource); + const resolvedBaseUrl = resolveOpenApiBaseUrl(spec, specSource, fetchedUrl); const surface: ApiSurface = { integrationName, discoveredAt: new Date().toISOString(), source: specSource, actions, + ...(resolvedBaseUrl !== undefined ? { baseUrl: resolvedBaseUrl } : {}), }; await writeArtifactJson( diff --git a/ts/packages/agents/onboarding/src/scaffolder/restHandler.template b/ts/packages/agents/onboarding/src/scaffolder/restHandler.template new file mode 100644 index 0000000000..352e8bfc81 --- /dev/null +++ b/ts/packages/agents/onboarding/src/scaffolder/restHandler.template @@ -0,0 +1,99 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. +// +// Auto-generated REST handler for {{NAME}} +// +// Generated deterministically from an OpenAPI 3 spec (parseOpenApiSpec arm). +// v1 limitations: no auth (no Authorization/api-key headers), scalar-only +// query/path params (no array/object/style/explode), header/cookie params +// are dropped when optional and rejected when required, DELETE requests +// never carry a body. + +import { + ActionContext, + AppAgent, + TypeAgentAction, +} from "@typeagent/agent-sdk"; +import { + createActionResultFromTextDisplay, + truncateText, +} from "@typeagent/agent-sdk/helpers/action"; +import { {{PASCAL_NAME}}Actions } from "./{{NAME}}Schema.js"; + +const BASE_URL = {{BASE_URL}}; + +function formatResponseBody(text: string): string { + try { + return truncateText(JSON.stringify(JSON.parse(text), null, 2)); + } catch { + return truncateText(text); + } +} + +async function callRest( + method: string, + path: string, + query: Record, + body: Record | undefined, +): Promise { + const trimmedBase = BASE_URL.replace(/\/$/, ""); + const normalizedPath = path.startsWith("/") ? path : `/${path}`; + const url = new URL(`${trimmedBase}${normalizedPath}`); + for (const [key, value] of Object.entries(query)) { + if (value === undefined || value === null) continue; + url.searchParams.set(key, String(value)); + } + + const headers: Record = { Accept: "application/json" }; + let requestBody: string | undefined; + if (body !== undefined) { + headers["Content-Type"] = "application/json"; + requestBody = JSON.stringify(body); + } + + const res = await fetch(url, { + method, + headers, + signal: AbortSignal.timeout(30_000), + ...(requestBody !== undefined ? { body: requestBody } : {}), + }); + const text = await res.text(); + if (!res.ok) { + throw new Error(`${res.status} ${res.statusText}: ${truncateText(text)}`); + } + return formatResponseBody(text); +} + +async function runAction( + action: TypeAgentAction<{{PASCAL_NAME}}Actions>, +): Promise { + const parameters = action.parameters as Record; + switch (action.actionName) { +{{SWITCH_CASES}} + default: + // Cast rather than reading `action.actionName` directly: when + // the switch above exhaustively covers every literal in the + // {{PASCAL_NAME}}Actions union, TS narrows `action` to `never` + // here, and `never` has no properties. + throw new Error( + `Unknown action: ${(action as { actionName: string }).actionName}`, + ); + } +} + +export function instantiate(): AppAgent { + return { + executeAction: async ( + action: TypeAgentAction<{{PASCAL_NAME}}Actions>, + context: ActionContext<{{PASCAL_NAME}}Actions>, + ) => { + try { + const output = await runAction(action); + return createActionResultFromTextDisplay(output); + } catch (e: unknown) { + const msg = e instanceof Error ? e.message : String(e); + return createActionResultFromTextDisplay(`Error: ${msg}`); + } + }, + }; +} diff --git a/ts/packages/agents/onboarding/src/scaffolder/restHandlerTemplate.ts b/ts/packages/agents/onboarding/src/scaffolder/restHandlerTemplate.ts new file mode 100644 index 0000000000..111880669a --- /dev/null +++ b/ts/packages/agents/onboarding/src/scaffolder/restHandlerTemplate.ts @@ -0,0 +1,213 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// Generates a fetch-based TypeScript action handler that issues HTTP requests +// against a base URL resolved from an OpenAPI 3 spec. Called by +// scaffolderHandler when the API surface has a resolved `baseUrl` and actions +// carrying an HTTP method and `path`. +// +// v1 limitations: +// - scalar path/query params only (no arrays/objects, no style/explode) +// - header/cookie params: required ones fail the action, optional ones are +// dropped +// - no auth (no Authorization / api-key headers) +// - DELETE requests never carry a body + +import type { DiscoveredAction } from "../discovery/discoveryHandler.js"; +import fs from "fs/promises"; +import path from "path"; +import { fileURLToPath } from "url"; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); + +// Resolve template from src/ relative to the package root. +// At runtime __dirname is dist/scaffolder/, so go up two levels to package root. +function templatePath(): string { + return path.resolve(__dirname, "../../src/scaffolder/restHandler.template"); +} + +const HTTP_METHODS_WITH_HANDLER = new Set([ + "GET", + "POST", + "PUT", + "PATCH", + "DELETE", +]); + +// Deterministic wire-name -> camelCase transform, matching how parameter +// names are renamed during SchemaGen, so `book_id`/`book-id` on the wire maps +// to the `bookId` populated in `action.parameters`. +function toCamel(wireName: string): string { + return wireName.replace(/[-_]+([A-Za-z0-9])/g, (_, c: string) => + c.toUpperCase(), + ); +} + +// Bracket-access expression reading a parameter by its camelCase runtime +// name, falling back to the original wire name in case the LLM (or a +// simple/no-op transform) preserved it verbatim. +function paramValueExpr(wireName: string): string { + const camel = toCamel(wireName); + if (camel === wireName) { + return `parameters[${JSON.stringify(wireName)}]`; + } + return `parameters[${JSON.stringify(camel)}] ?? parameters[${JSON.stringify(wireName)}]`; +} + +// Builds a JS expression for the request path: literal segments are +// JSON.stringify'd string literals, `{param}` placeholders become +// `encodeURIComponent(String())`, all joined with `+`. This +// avoids template-literal escaping concerns entirely (no backtick/`${` +// collision risk from spec-provided literal text). +function buildPathExpr( + pathTemplate: string, + pathParamNames: Set, +): string { + const parts: string[] = []; + let lastIndex = 0; + const re = /\{([^}]+)\}/g; + let m: RegExpExecArray | null; + while ((m = re.exec(pathTemplate)) !== null) { + const literal = pathTemplate.slice(lastIndex, m.index); + if (literal) parts.push(JSON.stringify(literal)); + const paramName = m[1]; + if (pathParamNames.has(paramName)) { + parts.push( + `encodeURIComponent(String(${paramValueExpr(paramName)}))`, + ); + } else { + // Path placeholder with no captured parameter (shouldn't + // normally happen for an inline spec) — leave the literal + // placeholder text in place rather than throw at generation time. + parts.push(JSON.stringify(m[0])); + } + lastIndex = re.lastIndex; + } + const tail = pathTemplate.slice(lastIndex); + if (tail || parts.length === 0) parts.push(JSON.stringify(tail)); + return parts.join(" + "); +} + +function buildSwitchCases(actions: DiscoveredAction[]): string { + const cases: string[] = []; + for (const action of actions) { + const method = (action.method ?? "GET").toUpperCase(); + const pathTemplate = action.path ?? "/"; + const pathParamNames = new Set( + [...pathTemplate.matchAll(/\{([^}]+)\}/g)].map((m) => m[1]), + ); + + const params = action.parameters ?? []; + const bodyParams = params.filter((p) => p.in === "body"); + const headerCookieParams = params.filter( + (p) => p.in === "header" || p.in === "cookie", + ); + const queryParams = params.filter( + (p) => + p.in === "query" || + (p.in === undefined && + !pathParamNames.has(p.name) && + !bodyParams.includes(p)), + ); + + const requiredUnsupported = headerCookieParams.find((p) => p.required); + if (requiredUnsupported) { + cases.push( + ` case ${JSON.stringify(action.name)}:\n` + + ` throw new Error(${JSON.stringify( + `Required parameter "${requiredUnsupported.name}" (${requiredUnsupported.in}) is not supported by the generated REST handler in v1.`, + )});`, + ); + continue; + } + // Optional header/cookie params are silently dropped (v1 limitation). + + const lines: string[] = []; + lines.push(` case ${JSON.stringify(action.name)}: {`); + lines.push( + ` const path = ${buildPathExpr(pathTemplate, pathParamNames)};`, + ); + lines.push(` const query: Record = {};`); + for (const p of queryParams) { + lines.push( + ` query[${JSON.stringify(p.name)}] = ${paramValueExpr(p.name)};`, + ); + } + if ( + ["POST", "PUT", "PATCH"].includes(method) && + bodyParams.length > 0 + ) { + lines.push(` const body: Record = {`); + for (const p of bodyParams) { + lines.push( + ` [${JSON.stringify(p.name)}]: ${paramValueExpr(p.name)},`, + ); + } + lines.push(` };`); + lines.push( + ` return await callRest(${JSON.stringify(method)}, path, query, body);`, + ); + } else { + lines.push( + ` return await callRest(${JSON.stringify(method)}, path, query, undefined);`, + ); + } + lines.push(` }`); + cases.push(lines.join("\n")); + } + return cases.join("\n"); +} + +/** + * Returns the subset of `actions` this generator can handle: those with a + * recognized HTTP method and a `path` whose every `{placeholder}` has a + * resolved path parameter. An action with an unresolved placeholder is + * excluded so the caller falls back to the stub handler instead of emitting a + * request with a literal "undefined" in the URL. + */ +export function filterRestActions( + actions: DiscoveredAction[] | undefined, +): DiscoveredAction[] { + if (!actions) return []; + return actions.filter((a) => { + if ( + !a.path || + !a.method || + !HTTP_METHODS_WITH_HANDLER.has(a.method.toUpperCase()) + ) { + return false; + } + const pathParamNames = new Set( + [...a.path.matchAll(/\{([^}]+)\}/g)].map((m) => m[1]), + ); + if (pathParamNames.size === 0) return true; + const resolvedPathParamNames = new Set( + (a.parameters ?? []) + .filter((p) => p.in === "path") + .map((p) => p.name), + ); + for (const placeholder of pathParamNames) { + if (!resolvedPathParamNames.has(placeholder)) return false; + } + return true; + }); +} + +export async function buildRestHandler( + name: string, + pascalName: string, + baseUrl: string, + actions: DiscoveredAction[], +): Promise { + const tpl = await fs.readFile(templatePath(), "utf-8"); + const switchCases = buildSwitchCases(actions); + // Use function-form replacers: string-form replace treats "$" + // sequences in the replacement as special (e.g. "$&", "$1"), which a + // generated base URL or switch-case body could plausibly contain. + return tpl + .replace(/\{\{NAME\}\}/g, () => name) + .replace(/\{\{PASCAL_NAME\}\}/g, () => pascalName) + .replace(/\{\{BASE_URL\}\}/g, () => JSON.stringify(baseUrl)) + .replace(/\{\{SWITCH_CASES\}\}/g, () => switchCases); +} diff --git a/ts/packages/agents/onboarding/src/scaffolder/scaffolderHandler.ts b/ts/packages/agents/onboarding/src/scaffolder/scaffolderHandler.ts index c554b9fd5c..0e03ef583d 100644 --- a/ts/packages/agents/onboarding/src/scaffolder/scaffolderHandler.ts +++ b/ts/packages/agents/onboarding/src/scaffolder/scaffolderHandler.ts @@ -21,6 +21,7 @@ import { } from "../lib/workspace.js"; import type { ApiSurface } from "../discovery/discoveryHandler.js"; import { buildCliHandler } from "./cliHandlerTemplate.js"; +import { buildRestHandler, filterRestActions } from "./restHandlerTemplate.js"; import { loadTemplate } from "./templateLoader.js"; import { generateAgentKeywordFiles } from "./agentKeywordFiles.js"; import type { KeywordSchemaTarget } from "./agentKeywordFiles.js"; @@ -726,7 +727,7 @@ function buildManifest( return manifest; } -async function buildHandler( +export async function buildHandler( name: string, pascalName: string, pattern: AgentPattern = "schema-grammar", @@ -741,6 +742,27 @@ async function buildHandler( return await buildCliHandler(name, pascalName, cliCommand, cliActions); } + // If discovery data has a resolved OpenAPI base URL, generate a real + // fetch-based REST handler for the patterns that model a direct REST + // integration (schema-grammar is the default when a caller doesn't + // specify a pattern). Explicit non-REST patterns (websocket-bridge, + // native-platform, etc.) keep their own dedicated builders even if an + // apiSurface happens to be attached. + if ( + apiSurface?.baseUrl && + (pattern === "schema-grammar" || pattern === "external-api") + ) { + const restActions = filterRestActions(apiSurface.actions); + if (restActions.length > 0) { + return await buildRestHandler( + name, + pascalName, + apiSurface.baseUrl, + restActions, + ); + } + } + switch (pattern) { case "external-api": return buildExternalApiHandler(name, pascalName); diff --git a/ts/packages/agents/onboarding/test/apiSurfacePreservation.spec.ts b/ts/packages/agents/onboarding/test/apiSurfacePreservation.spec.ts new file mode 100644 index 0000000000..eae742af79 --- /dev/null +++ b/ts/packages/agents/onboarding/test/apiSurfacePreservation.spec.ts @@ -0,0 +1,105 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// Verifies CHANGE 1/2 survive the full discovery -> approval round trip +// (parseOpenApiSpec writes api-surface.json, approveApiSurface re-reads and +// re-writes it via `{...surface, ...}`): baseUrl and per-parameter `in` +// must still be present in the approved artifact. Exercises the real +// executeDiscoveryAction handler end-to-end against the real per-integration +// workspace on disk (~/.typeagent/onboarding/), cleaned up afterward. + +import { test, after } from "node:test"; +import assert from "node:assert/strict"; +import fs from "fs/promises"; +import path from "path"; +import os from "os"; +import { fileURLToPath } from "url"; +import { executeDiscoveryAction } from "../src/discovery/discoveryHandler.js"; +import type { ApiSurface } from "../src/discovery/discoveryHandler.js"; +import { createWorkspace, readArtifactJson } from "../src/lib/workspace.js"; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); + +const INTEGRATION_NAME = `test-book-api-${process.pid}-${Date.now()}`; + +function getWorkspaceDir(): string { + return path.join( + os.homedir(), + ".typeagent", + "onboarding", + INTEGRATION_NAME, + ); +} + +after(async () => { + await fs.rm(getWorkspaceDir(), { recursive: true, force: true }); +}); + +test("baseUrl and parameter `in` survive parseOpenApiSpec -> approveApiSurface", async () => { + await createWorkspace({ integrationName: INTEGRATION_NAME }); + + const fixturePath = path.resolve( + __dirname, + "fixtures/openapi/book-api-absolute-server.json", + ); + + const parseResult = await executeDiscoveryAction( + { + actionName: "parseOpenApiSpec", + parameters: { + integrationName: INTEGRATION_NAME, + specSource: fixturePath, + }, + } as any, + {} as any, + ); + assert.ok( + !("error" in (parseResult as any)), + `parseOpenApiSpec failed: ${JSON.stringify(parseResult)}`, + ); + + const parsedSurface = await readArtifactJson( + INTEGRATION_NAME, + "discovery", + "api-surface.json", + ); + assert.equal(parsedSurface?.baseUrl, "https://api.example.com/v1"); + const getBook = parsedSurface?.actions.find((a) => a.name === "getBook"); + assert.equal( + getBook?.parameters?.find((p) => p.name === "book_id")?.in, + "path", + ); + + const approveResult = await executeDiscoveryAction( + { + actionName: "approveApiSurface", + parameters: { integrationName: INTEGRATION_NAME }, + } as any, + {} as any, + ); + assert.ok( + !("error" in (approveResult as any)), + `approveApiSurface failed: ${JSON.stringify(approveResult)}`, + ); + + const approvedSurface = await readArtifactJson( + INTEGRATION_NAME, + "discovery", + "api-surface.json", + ); + assert.equal( + approvedSurface?.baseUrl, + "https://api.example.com/v1", + "baseUrl must survive the approval spread", + ); + const approvedGetBook = approvedSurface?.actions.find( + (a) => a.name === "getBook", + ); + assert.equal( + approvedGetBook?.parameters?.find((p) => p.name === "book_id")?.in, + "path", + "parameter `in` must survive the approval spread", + ); + assert.equal(approvedSurface?.approved, true); +}); diff --git a/ts/packages/agents/onboarding/test/fixtures/openapi/book-api-absolute-server.json b/ts/packages/agents/onboarding/test/fixtures/openapi/book-api-absolute-server.json new file mode 100644 index 0000000000..f48918064f --- /dev/null +++ b/ts/packages/agents/onboarding/test/fixtures/openapi/book-api-absolute-server.json @@ -0,0 +1,25 @@ +{ + "openapi": "3.0.0", + "info": { + "title": "Book API", + "version": "1.0.0" + }, + "servers": [{ "url": "https://api.example.com/v1" }], + "paths": { + "/books/{book_id}": { + "parameters": [ + { + "name": "book_id", + "in": "path", + "required": true, + "schema": { "type": "string" } + } + ], + "get": { + "operationId": "get_book", + "summary": "Get a book", + "responses": { "200": { "description": "ok" } } + } + } + } +} diff --git a/ts/packages/agents/onboarding/test/fixtures/openapi/book-api.json b/ts/packages/agents/onboarding/test/fixtures/openapi/book-api.json new file mode 100644 index 0000000000..d7101e6e25 --- /dev/null +++ b/ts/packages/agents/onboarding/test/fixtures/openapi/book-api.json @@ -0,0 +1,85 @@ +{ + "openapi": "3.0.0", + "info": { + "title": "Book API", + "version": "1.0.0" + }, + "servers": [{ "url": "/v3" }], + "paths": { + "/books/{book_id}": { + "parameters": [ + { + "name": "book_id", + "in": "path", + "required": true, + "schema": { "type": "string" } + } + ], + "get": { + "operationId": "get_book", + "summary": "Get a book", + "parameters": [ + { + "name": "include_reviews", + "in": "query", + "required": false, + "schema": { "type": "boolean" } + } + ], + "responses": { "200": { "description": "ok" } } + }, + "put": { + "operationId": "update_book", + "summary": "Update a book", + "requestBody": { + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "title": { "type": "string" } + }, + "required": ["title"] + } + } + } + }, + "responses": { "200": { "description": "ok" } } + } + }, + "/books": { + "get": { + "operationId": "search_books", + "summary": "Search books", + "parameters": [ + { + "name": "limit", + "in": "query", + "required": false, + "schema": { "type": "integer" } + } + ], + "responses": { "200": { "description": "ok" } } + }, + "post": { + "operationId": "create_book", + "summary": "Create a book", + "requestBody": { + "content": { + "application/json": { + "schema": { + "type": "object", + "properties": { + "title": { "type": "string" }, + "author": { "type": "string" } + }, + "required": ["title"] + } + } + } + }, + "responses": { "201": { "description": "created" } } + } + } + } +} diff --git a/ts/packages/agents/onboarding/test/openApiExtraction.spec.ts b/ts/packages/agents/onboarding/test/openApiExtraction.spec.ts new file mode 100644 index 0000000000..0817e5a93f --- /dev/null +++ b/ts/packages/agents/onboarding/test/openApiExtraction.spec.ts @@ -0,0 +1,308 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// Deterministic unit tests for the parseOpenApiSpec arm's extraction and +// base-URL resolution logic (CHANGE 1 & CHANGE 2 of the REST-handler- +// generation feature). + +import { test, describe } from "node:test"; +import assert from "node:assert/strict"; +import fs from "fs/promises"; +import path from "path"; +import { fileURLToPath } from "url"; +import { + extractOpenApiActions, + resolveOpenApiBaseUrl, +} from "../src/discovery/discoveryHandler.js"; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); + +async function loadBookApiSpec(): Promise { + const raw = await fs.readFile( + path.resolve(__dirname, "fixtures/openapi/book-api.json"), + "utf-8", + ); + return JSON.parse(raw); +} + +describe("extractOpenApiActions", () => { + test("merges path-item-level params with operation-level params", async () => { + const spec = await loadBookApiSpec(); + const actions = extractOpenApiActions( + spec, + "https://api.example.com/openapi.json", + ); + + const getBook = actions.find((a) => a.name === "getBook"); + assert.ok(getBook, "expected a getBook action"); + assert.equal(getBook!.method, "GET"); + assert.equal(getBook!.path, "/books/{book_id}"); + const getBookParamNames = getBook!.parameters?.map((p) => p.name); + assert.ok(getBookParamNames?.includes("book_id")); + assert.ok(getBookParamNames?.includes("include_reviews")); + + const bookIdParam = getBook!.parameters?.find( + (p) => p.name === "book_id", + ); + assert.equal(bookIdParam?.in, "path"); + assert.equal(bookIdParam?.required, true); + + const includeReviewsParam = getBook!.parameters?.find( + (p) => p.name === "include_reviews", + ); + assert.equal(includeReviewsParam?.in, "query"); + }); + + test("operation-level param with the same (name, in) overrides the path-level one", () => { + const spec = { + openapi: "3.0.0", + paths: { + "/items/{id}": { + parameters: [ + { + name: "id", + in: "path", + required: true, + description: "path-level", + schema: { type: "string" }, + }, + ], + get: { + operationId: "get_item", + parameters: [ + { + name: "id", + in: "path", + required: true, + description: "operation-level override", + schema: { type: "string" }, + }, + ], + }, + }, + }, + }; + const actions = extractOpenApiActions(spec, "spec.json"); + const idParam = actions[0].parameters?.find((p) => p.name === "id"); + assert.equal(idParam?.description, "operation-level override"); + // Only one merged entry, not two. + assert.equal( + actions[0].parameters?.filter((p) => p.name === "id").length, + 1, + ); + }); + + test("captures request body properties with in: body", () => { + const spec = { + openapi: "3.0.0", + paths: { + "/books/{book_id}": { + put: { + operationId: "update_book", + requestBody: { + content: { + "application/json": { + schema: { + type: "object", + properties: { + title: { type: "string" }, + }, + required: ["title"], + }, + }, + }, + }, + }, + }, + }, + }; + const actions = extractOpenApiActions(spec, "spec.json"); + const titleParam = actions[0].parameters?.find( + (p) => p.name === "title", + ); + assert.equal(titleParam?.in, "body"); + assert.equal(titleParam?.required, true); + }); + + test("skips a $ref'd path item entirely (inline-only v1 scope)", () => { + const spec = { + openapi: "3.0.0", + paths: { + "/shared": { $ref: "#/components/pathItems/Shared" }, + "/real": { get: { operationId: "get_real" } }, + }, + }; + const actions = extractOpenApiActions(spec, "spec.json"); + assert.equal(actions.length, 1); + assert.equal(actions[0].name, "getReal"); + }); + + test("skips an unresolvable $ref'd parameter (no matching components.parameters entry)", () => { + const spec = { + openapi: "3.0.0", + paths: { + "/x": { + get: { + operationId: "get_x", + parameters: [{ $ref: "#/components/parameters/Foo" }], + }, + }, + }, + }; + const actions = extractOpenApiActions(spec, "spec.json"); + assert.equal(actions[0].parameters?.length, 0); + }); + + test("resolves a local #/components/parameters/* $ref path parameter inline (Fix 1 regression)", () => { + const spec = { + openapi: "3.0.0", + components: { + parameters: { + BookId: { + name: "bookId", + in: "path", + required: true, + schema: { type: "string" }, + }, + }, + }, + paths: { + "/books/{bookId}": { + get: { + operationId: "get_book", + parameters: [ + { $ref: "#/components/parameters/BookId" }, + ], + }, + }, + }, + }; + const actions = extractOpenApiActions(spec, "spec.json"); + assert.equal(actions.length, 1); + const bookIdParam = actions[0].parameters?.find( + (p) => p.name === "bookId", + ); + assert.ok( + bookIdParam, + "expected the $ref'd path parameter to be resolved and merged, not dropped", + ); + assert.equal(bookIdParam?.in, "path"); + assert.equal(bookIdParam?.required, true); + }); + + test("path-level $ref parameter resolves and merges the same as an inline path-level parameter", () => { + const spec = { + openapi: "3.0.0", + components: { + parameters: { + BookId: { + name: "bookId", + in: "path", + required: true, + schema: { type: "string" }, + }, + }, + }, + paths: { + "/books/{bookId}": { + parameters: [{ $ref: "#/components/parameters/BookId" }], + get: { operationId: "get_book" }, + delete: { operationId: "delete_book" }, + }, + }, + }; + const actions = extractOpenApiActions(spec, "spec.json"); + assert.equal(actions.length, 2); + for (const action of actions) { + const bookIdParam = action.parameters?.find( + (p) => p.name === "bookId", + ); + assert.ok( + bookIdParam, + `expected ${action.name} to carry the path-level $ref'd bookId parameter`, + ); + assert.equal(bookIdParam?.in, "path"); + } + }); +}); + +describe("resolveOpenApiBaseUrl", () => { + test("resolves a relative servers[0].url against the fetched spec location, preserving path", async () => { + const spec = await loadBookApiSpec(); + const baseUrl = resolveOpenApiBaseUrl( + spec, + "https://api.example.com/openapi.json", + "https://api.example.com/openapi.json", + ); + assert.equal(baseUrl, "https://api.example.com/v3"); + }); + + test("preserves an absolute server URL's path component", () => { + const spec = { servers: [{ url: "https://api.example.com/v1" }] }; + const baseUrl = resolveOpenApiBaseUrl( + spec, + "https://api.example.com/openapi.json", + ); + assert.equal(baseUrl, "https://api.example.com/v1"); + }); + + test("substitutes server variables from their default values", () => { + const spec = { + servers: [ + { + url: "https://{host}.example.com/{basePath}", + variables: { + host: { default: "api" }, + basePath: { default: "v2" }, + }, + }, + ], + }; + const baseUrl = resolveOpenApiBaseUrl(spec, "spec.json"); + assert.equal(baseUrl, "https://api.example.com/v2"); + }); + + test("leaves baseUrl unset when a server variable has no default", () => { + const spec = { + servers: [ + { + url: "https://{host}.example.com", + variables: {}, + }, + ], + }; + const baseUrl = resolveOpenApiBaseUrl(spec, "spec.json"); + assert.equal(baseUrl, undefined); + }); + + test("falls back to the fetched spec's origin when servers is absent", () => { + const baseUrl = resolveOpenApiBaseUrl( + {}, + "https://api.example.com/openapi.json", + "https://api.example.com/openapi.json", + ); + assert.equal(baseUrl, "https://api.example.com"); + }); + + test("leaves baseUrl unset for a local file spec source with a relative servers url", () => { + const spec = { servers: [{ url: "/v3" }] }; + const baseUrl = resolveOpenApiBaseUrl(spec, "./local-spec.json"); + assert.equal(baseUrl, undefined); + }); + + test("leaves baseUrl unset for a local file spec source with no servers entry", () => { + const baseUrl = resolveOpenApiBaseUrl({}, "./local-spec.json"); + assert.equal(baseUrl, undefined); + }); + + test("leaves baseUrl unset when servers[0].url is an absolute non-http(s) scheme (Fix 3 regression)", () => { + const spec = { servers: [{ url: "ws://api.example.com/v1" }] }; + const baseUrl = resolveOpenApiBaseUrl( + spec, + "https://api.example.com/openapi.json", + "https://api.example.com/openapi.json", + ); + assert.equal(baseUrl, undefined); + }); +}); diff --git a/ts/packages/agents/onboarding/test/restHandlerCodegenFixes.spec.ts b/ts/packages/agents/onboarding/test/restHandlerCodegenFixes.spec.ts new file mode 100644 index 0000000000..72d745b3ed --- /dev/null +++ b/ts/packages/agents/onboarding/test/restHandlerCodegenFixes.spec.ts @@ -0,0 +1,145 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// Targeted regression tests for two review fixes on buildRestHandler's +// generated source text (fast, no tsc/network/server needed): +// - Fix 1: a path parameter resolved from a local `#/components/parameters` +// $ref (already deref'd into DiscoveredAction.parameters by +// discoveryHandler.ts) must be substituted into the path expression via +// the actual parameter lookup, not silently degrade to a literal +// "undefined" segment. +// - Fix 2: a GET (or HEAD) action with body-typed parameters must never +// emit a `body` argument to callRest, since fetch throws for +// GET/HEAD requests carrying a body. +// - Fix 3: the callRest helper must only set fetch's `body` when a request +// body exists (conditional spread), never `body: requestBody` where +// requestBody is `string | undefined` — that fails TS2379 under the +// repo's `exactOptionalPropertyTypes`, so a scaffolded REST agent would +// not compile in-workspace. + +import { test } from "node:test"; +import assert from "node:assert/strict"; +import { buildRestHandler } from "../src/scaffolder/restHandlerTemplate.js"; +import type { DiscoveredAction } from "../src/discovery/discoveryHandler.js"; + +test("path parameter (as resolved from a local $ref by discoveryHandler) is substituted, never left as a literal placeholder or undefined", async () => { + // Shape identical to what extractOpenApiActions now produces once it + // resolves a `{"$ref": "#/components/parameters/BookId"}` path + // parameter inline. + const actions: DiscoveredAction[] = [ + { + name: "getBook", + method: "GET", + path: "/books/{bookId}", + parameters: [ + { + name: "bookId", + type: "string", + required: true, + in: "path", + }, + ], + }, + ]; + const source = await buildRestHandler( + "bookApi", + "BookApi", + "https://api.example.com/v1", + actions, + ); + + // Must read the actual parameter (bracket-access lookup), not emit the + // raw placeholder text or a bare `undefined` literal into the path. + assert.match(source, /parameters\["bookId"\]/); + assert.doesNotMatch(source, /"\{bookId\}"/); + assert.doesNotMatch(source, /\+\s*undefined\s*\+/); +}); + +test("GET action with body-typed parameters never emits a body (Fix 2 regression)", async () => { + const actions: DiscoveredAction[] = [ + { + name: "searchBooks", + method: "GET", + path: "/books/search", + parameters: [ + { + name: "query", + type: "string", + required: true, + in: "body", + }, + ], + }, + ]; + const source = await buildRestHandler( + "bookApi", + "BookApi", + "https://api.example.com/v1", + actions, + ); + + const caseMatch = /case "searchBooks": \{([\s\S]*?)\n {8}\}/.exec(source); + assert.ok(caseMatch, "expected a generated case for searchBooks"); + const caseBody = caseMatch![1]; + assert.doesNotMatch( + caseBody, + /const body/, + "GET actions must never construct a request body", + ); + assert.match( + caseBody, + /callRest\("GET", path, query, undefined\)/, + "GET actions must call callRest with an undefined body argument", + ); +}); + +test("HEAD-like semantics aside, DELETE with body-typed parameters still never emits a body", async () => { + const actions: DiscoveredAction[] = [ + { + name: "deleteBook", + method: "DELETE", + path: "/books/{bookId}", + parameters: [ + { name: "bookId", type: "string", required: true, in: "path" }, + { name: "reason", type: "string", required: false, in: "body" }, + ], + }, + ]; + const source = await buildRestHandler( + "bookApi", + "BookApi", + "https://api.example.com/v1", + actions, + ); + const caseMatch = /case "deleteBook": \{([\s\S]*?)\n {8}\}/.exec(source); + assert.ok(caseMatch, "expected a generated case for deleteBook"); + assert.doesNotMatch(caseMatch![1], /const body/); +}); + +test("callRest sets fetch's body only when present, never body: requestBody (Fix 3 regression)", async () => { + const actions: DiscoveredAction[] = [ + { + name: "getBook", + method: "GET", + path: "/books/{bookId}", + parameters: [ + { name: "bookId", type: "string", required: true, in: "path" }, + ], + }, + ]; + const source = await buildRestHandler( + "bookApi", + "BookApi", + "https://api.example.com/v1", + actions, + ); + + // Must NOT unconditionally pass `body: requestBody` (requestBody is + // `string | undefined`) — that fails TS2379 under exactOptionalPropertyTypes. + assert.doesNotMatch(source, /\n\s*body: requestBody,/); + // Must conditionally include body only when a request body exists. + assert.match( + source, + /\.\.\.\(requestBody !== undefined \? \{ body: requestBody \} : \{\}\)/, + ); +}); diff --git a/ts/packages/agents/onboarding/test/restHandlerCompile.spec.ts b/ts/packages/agents/onboarding/test/restHandlerCompile.spec.ts new file mode 100644 index 0000000000..f648032bc0 --- /dev/null +++ b/ts/packages/agents/onboarding/test/restHandlerCompile.spec.ts @@ -0,0 +1,143 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// Verifies the generator's output actually type-checks with `tsc` (CHANGE 3 +// acceptance criterion 2d). Writes a generated handler + a matching schema +// stub to a scratch folder nested inside the onboarding package (so +// module resolution finds the workspace's @typeagent/agent-sdk via the +// package's own node_modules symlink), then runs `tsc --noEmit` on it. + +import { test, after } from "node:test"; +import assert from "node:assert/strict"; +import { execFile } from "child_process"; +import { promisify } from "util"; +import fs from "fs/promises"; +import path from "path"; +import { fileURLToPath } from "url"; +import { buildRestHandler } from "../src/scaffolder/restHandlerTemplate.js"; +import type { DiscoveredAction } from "../src/discovery/discoveryHandler.js"; + +const execFileAsync = promisify(execFile); +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); + +const SCRATCH_DIR = path.resolve(__dirname, ".tmp-rest-handler-compile"); + +const actions: DiscoveredAction[] = [ + { + name: "getBook", + description: "Get a book", + method: "GET", + path: "/books/{book_id}", + parameters: [ + { name: "book_id", type: "string", required: true, in: "path" }, + { + name: "include_reviews", + type: "boolean", + required: false, + in: "query", + }, + ], + }, + { + name: "createBook", + description: "Create a book", + method: "POST", + path: "/books", + parameters: [ + { name: "title", type: "string", required: true, in: "body" }, + { name: "author", type: "string", required: false, in: "body" }, + ], + }, + { + name: "deleteBook", + description: "Delete a book", + method: "DELETE", + path: "/books/{book_id}", + parameters: [ + { name: "book_id", type: "string", required: true, in: "path" }, + ], + }, +]; + +function buildSchemaStub(pascalName: string): string { + const members = actions + .map( + (a) => + ` | {\n` + + ` actionName: ${JSON.stringify(a.name)};\n` + + ` parameters: Record;\n` + + ` }`, + ) + .join("\n"); + return ( + `// Copyright (c) Microsoft Corporation.\n` + + `// Licensed under the MIT License.\n\n` + + `export type ${pascalName}Actions =\n${members};\n` + ); +} + +after(async () => { + await fs.rm(SCRATCH_DIR, { recursive: true, force: true }); +}); + +test("buildRestHandler output type-checks with tsc", async () => { + await fs.rm(SCRATCH_DIR, { recursive: true, force: true }); + await fs.mkdir(SCRATCH_DIR, { recursive: true }); + + const name = "bookApi"; + const pascalName = "BookApi"; + const handlerSource = await buildRestHandler( + name, + pascalName, + "https://api.example.com/v1", + actions, + ); + + await fs.writeFile( + path.join(SCRATCH_DIR, `${name}ActionHandler.ts`), + handlerSource, + "utf-8", + ); + await fs.writeFile( + path.join(SCRATCH_DIR, `${name}Schema.ts`), + buildSchemaStub(pascalName), + "utf-8", + ); + await fs.writeFile( + path.join(SCRATCH_DIR, "tsconfig.json"), + JSON.stringify( + { + compilerOptions: { + target: "es2021", + lib: ["es2021"], + module: "node16", + moduleResolution: "node16", + types: ["node"], + esModuleInterop: true, + skipLibCheck: true, + strict: true, + noEmit: true, + }, + include: ["*.ts"], + }, + null, + 2, + ), + "utf-8", + ); + + const tscBin = path.resolve( + __dirname, + "../node_modules/typescript/bin/tsc", + ); + try { + await execFileAsync(process.execPath, [tscBin, "-p", SCRATCH_DIR], { + cwd: SCRATCH_DIR, + }); + } catch (err: any) { + assert.fail( + `Generated REST handler failed to type-check:\n${err.stdout ?? err.message}`, + ); + } +}); diff --git a/ts/packages/agents/onboarding/test/restHandlerE2EBooks.spec.ts b/ts/packages/agents/onboarding/test/restHandlerE2EBooks.spec.ts new file mode 100644 index 0000000000..2d310b650d --- /dev/null +++ b/ts/packages/agents/onboarding/test/restHandlerE2EBooks.spec.ts @@ -0,0 +1,296 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// Tier-A end-to-end onboarding pipeline check (offline/deterministic): runs +// the full parseOpenApiSpec discovery pipeline (including a $ref-resolved +// path parameter — the Fix 1 regression scenario) against an OpenAPI 3 +// "books" spec whose `servers[0].url` is an absolute URL pointing at a +// local node:http server WITH a base path ("/v2", to also exercise +// base-path preservation), then scaffolds a REST handler from the +// resulting ApiSurface, type-checks it, and executes it against that same +// local server for a GET-with-path-param, a GET-with-query, and a +// POST-with-JSON-body action. Asserts the server observed the correct +// method/URL/body for each request (base path preserved, $ref path param +// substituted — never "undefined" — query encoded, JSON body sent) and +// that real response data flows back through the handler's +// createActionResultFromTextDisplay result. + +import { test, after } from "node:test"; +import assert from "node:assert/strict"; +import { execFile } from "child_process"; +import { promisify } from "util"; +import fs from "fs/promises"; +import path from "path"; +import { fileURLToPath, pathToFileURL } from "url"; +import http from "node:http"; +import type { AddressInfo } from "node:net"; +import { + extractOpenApiActions, + resolveOpenApiBaseUrl, +} from "../src/discovery/discoveryHandler.js"; +import { + buildRestHandler, + filterRestActions, +} from "../src/scaffolder/restHandlerTemplate.js"; + +const execFileAsync = promisify(execFile); +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); +const SCRATCH_DIR = path.resolve(__dirname, ".tmp-rest-e2e-books"); + +after(async () => { + await fs.rm(SCRATCH_DIR, { recursive: true, force: true }); +}); + +type ObservedRequest = { method?: string; url?: string; body: string }; + +async function startBooksServer(): Promise<{ + baseOrigin: string; + observed: ObservedRequest[]; + close: () => Promise; +}> { + const observed: ObservedRequest[] = []; + const server = http.createServer((req, res) => { + const chunks: Buffer[] = []; + req.on("data", (c) => chunks.push(c)); + req.on("end", () => { + const body = Buffer.concat(chunks).toString("utf-8"); + observed.push({ method: req.method, url: req.url, body }); + + if (req.method === "GET" && req.url?.startsWith("/v2/books/")) { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ id: "book-42", title: "Dune" })); + return; + } + if (req.method === "GET" && req.url?.startsWith("/v2/books")) { + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ results: ["Dune", "Foundation"] })); + return; + } + if (req.method === "POST" && req.url === "/v2/books") { + res.writeHead(201, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ id: "book-99", created: true })); + return; + } + res.writeHead(404); + res.end("not found"); + }); + }); + await new Promise((resolve) => + server.listen(0, "127.0.0.1", resolve), + ); + const { port } = server.address() as AddressInfo; + return { + baseOrigin: `http://127.0.0.1:${port}`, + observed, + close: () => + new Promise((resolve) => server.close(() => resolve())), + }; +} + +function buildBooksSpec(baseOrigin: string) { + return { + openapi: "3.0.0", + info: { title: "Books API", version: "1.0.0" }, + // Absolute server URL WITH a base path ("/v2") — exercises base-path + // preservation end to end, independent of the (local, non-http) + // spec source. + servers: [{ url: `${baseOrigin}/v2` }], + components: { + parameters: { + BookId: { + name: "bookId", + in: "path", + required: true, + schema: { type: "string" }, + }, + }, + }, + paths: { + "/books/{bookId}": { + // Path-level $ref parameter — the Fix 1 regression scenario: + // must resolve inline rather than silently drop, or the + // generated handler would substitute the literal string + // "undefined" into the request URL. + parameters: [{ $ref: "#/components/parameters/BookId" }], + get: { + operationId: "get_book", + summary: "Get a book by id", + responses: { "200": { description: "ok" } }, + }, + }, + "/books": { + get: { + operationId: "search_books", + summary: "Search books", + parameters: [ + { + name: "limit", + in: "query", + required: false, + schema: { type: "integer" }, + }, + ], + responses: { "200": { description: "ok" } }, + }, + post: { + operationId: "create_book", + summary: "Create a book", + requestBody: { + content: { + "application/json": { + schema: { + type: "object", + properties: { + title: { type: "string" }, + }, + required: ["title"], + }, + }, + }, + }, + responses: { "201": { description: "created" } }, + }, + }, + }, + }; +} + +test("Tier A: parseOpenApiSpec discovery + buildRestHandler scaffolding + real local-HTTP execution for a books API", async (t) => { + const { baseOrigin, observed, close } = await startBooksServer(); + t.after(() => close()); + + // --- Discovery phase (parseOpenApiSpec arm) --- + const specSource = "./local-books-spec.json"; // local, non-http source + const spec = buildBooksSpec(baseOrigin); + + const baseUrl = resolveOpenApiBaseUrl(spec, specSource, specSource); + assert.equal( + baseUrl, + `${baseOrigin}/v2`, + "expected the absolute server URL (with its /v2 base path) to be captured verbatim", + ); + + const actions = extractOpenApiActions(spec, specSource); + assert.equal(actions.length, 3); + + const getBook = actions.find((a) => a.name === "getBook"); + assert.ok(getBook, "expected a getBook action"); + const bookIdParam = getBook!.parameters?.find((p) => p.name === "bookId"); + assert.ok( + bookIdParam, + "expected the $ref'd path-level bookId parameter to have been resolved and merged onto get_book", + ); + assert.equal(bookIdParam?.in, "path"); + assert.equal(bookIdParam?.required, true); + + const searchBooks = actions.find((a) => a.name === "searchBooks"); + const limitParam = searchBooks?.parameters?.find((p) => p.name === "limit"); + assert.equal(limitParam?.in, "query"); + + const createBook = actions.find((a) => a.name === "createBook"); + const titleParam = createBook?.parameters?.find((p) => p.name === "title"); + assert.equal(titleParam?.in, "body"); + + // --- Scaffolding phase (buildRestHandler) --- + const restActions = filterRestActions(actions); + assert.equal( + restActions.length, + 3, + "all three actions should be eligible for REST generation (the $ref path param must have resolved, or getBook would have been excluded by the Fix-1 safety net)", + ); + + await fs.rm(SCRATCH_DIR, { recursive: true, force: true }); + await fs.mkdir(SCRATCH_DIR, { recursive: true }); + const handlerSource = await buildRestHandler( + "booksApi", + "BooksApi", + baseUrl!, + restActions, + ); + const handlerPath = path.join(SCRATCH_DIR, "booksApiActionHandler.ts"); + await fs.writeFile(handlerPath, handlerSource, "utf-8"); + await fs.writeFile( + path.join(SCRATCH_DIR, "booksApiSchema.ts"), + "export type BooksApiActions = { actionName: string; parameters: Record };\n", + "utf-8", + ); + + // --- Compile check --- + await fs.writeFile( + path.join(SCRATCH_DIR, "tsconfig.json"), + JSON.stringify( + { + compilerOptions: { + target: "es2021", + lib: ["es2021"], + module: "node16", + moduleResolution: "node16", + types: ["node"], + esModuleInterop: true, + skipLibCheck: true, + strict: true, + noEmit: true, + }, + include: ["*.ts"], + }, + null, + 2, + ), + "utf-8", + ); + const tscBin = path.resolve( + __dirname, + "../node_modules/typescript/bin/tsc", + ); + try { + await execFileAsync(process.execPath, [tscBin, "-p", SCRATCH_DIR], { + cwd: SCRATCH_DIR, + }); + } catch (err: any) { + assert.fail( + `Generated books REST handler failed to type-check:\n${err.stdout ?? err.message}`, + ); + } + + // --- Execution phase: real HTTP calls against the local server --- + const fileUrl = pathToFileURL(handlerPath); + fileUrl.search = `t=${Date.now()}-${Math.random()}`; + const mod = await import(fileUrl.href); + const agent = mod.instantiate(); + + // 1. GET with a $ref-resolved path param. + const getResult: any = await agent.executeAction( + { actionName: "getBook", parameters: { bookId: "book-42" } }, + {} as any, + ); + assert.equal(observed.length, 1); + assert.equal(observed[0].method, "GET"); + assert.equal( + observed[0].url, + "/v2/books/book-42", + "expected the /v2 base path preserved and the $ref-resolved bookId substituted (not 'undefined')", + ); + assert.doesNotMatch(observed[0].url ?? "", /undefined/); + assert.match(getResult.displayContent as string, /"title": "Dune"/); + + // 2. GET with a query param. + await agent.executeAction( + { actionName: "searchBooks", parameters: { limit: 2 } }, + {} as any, + ); + assert.equal(observed.length, 2); + assert.equal(observed[1].method, "GET"); + assert.equal(observed[1].url, "/v2/books?limit=2"); + + // 3. POST with a JSON body. + const postResult: any = await agent.executeAction( + { actionName: "createBook", parameters: { title: "Foundation" } }, + {} as any, + ); + assert.equal(observed.length, 3); + assert.equal(observed[2].method, "POST"); + assert.equal(observed[2].url, "/v2/books"); + assert.deepEqual(JSON.parse(observed[2].body), { title: "Foundation" }); + assert.match(postResult.displayContent as string, /"created": true/); +}); diff --git a/ts/packages/agents/onboarding/test/restHandlerExecution.spec.ts b/ts/packages/agents/onboarding/test/restHandlerExecution.spec.ts new file mode 100644 index 0000000000..b8e4e53653 --- /dev/null +++ b/ts/packages/agents/onboarding/test/restHandlerExecution.spec.ts @@ -0,0 +1,212 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// Deterministic, offline execution test (CHANGE 3 acceptance criterion 2e): +// generates a REST handler with buildRestHandler, points it at a local +// node:http server on an ephemeral port, and exercises GET-with-path-param, +// GET-with-query, and POST-with-JSON-body — asserting the server observed +// the expected method/URL/body and that the handler surfaced the server's +// response back through createActionResultFromTextDisplay. + +import { test, after } from "node:test"; +import assert from "node:assert/strict"; +import fs from "fs/promises"; +import path from "path"; +import { fileURLToPath, pathToFileURL } from "url"; +import http from "node:http"; +import type { AddressInfo } from "node:net"; +import { buildRestHandler } from "../src/scaffolder/restHandlerTemplate.js"; +import type { DiscoveredAction } from "../src/discovery/discoveryHandler.js"; + +const __filename = fileURLToPath(import.meta.url); +const __dirname = path.dirname(__filename); +const SCRATCH_DIR = path.resolve(__dirname, ".tmp-rest-handler-exec"); + +after(async () => { + await fs.rm(SCRATCH_DIR, { recursive: true, force: true }); +}); + +type ObservedRequest = { + method?: string; + url?: string; + body: string; +}; + +async function startServer( + handleRequest: ( + req: http.IncomingMessage, + res: http.ServerResponse, + body: string, + ) => void, +): Promise<{ baseUrl: string; close: () => Promise }> { + const server = http.createServer((req, res) => { + const chunks: Buffer[] = []; + req.on("data", (c) => chunks.push(c)); + req.on("end", () => { + handleRequest(req, res, Buffer.concat(chunks).toString("utf-8")); + }); + }); + await new Promise((resolve) => + server.listen(0, "127.0.0.1", resolve), + ); + const { port } = server.address() as AddressInfo; + return { + baseUrl: `http://127.0.0.1:${port}`, + close: () => + new Promise((resolve) => server.close(() => resolve())), + }; +} + +async function loadHandler( + baseUrl: string, + actions: DiscoveredAction[], +): Promise<{ instantiate: () => { executeAction: Function } }> { + await fs.rm(SCRATCH_DIR, { recursive: true, force: true }); + await fs.mkdir(SCRATCH_DIR, { recursive: true }); + const source = await buildRestHandler( + "bookApi", + "BookApi", + baseUrl, + actions, + ); + const handlerPath = path.join(SCRATCH_DIR, "bookApiActionHandler.ts"); + await fs.writeFile(handlerPath, source, "utf-8"); + // The generated handler imports "./bookApiSchema.js" purely for a type, + // which is erased at emit time — an empty type-only stub is enough for + // tsx's on-the-fly transpilation (no type-checking at runtime). + await fs.writeFile( + path.join(SCRATCH_DIR, "bookApiSchema.ts"), + "export type BookApiActions = { actionName: string; parameters: Record };\n", + "utf-8", + ); + // Cache-bust: each test loads a freshly written file at the same path. + // pathToFileURL is required on Windows, where raw absolute paths + // (e.g. "D:/...") are not valid ESM specifiers. + const fileUrl = pathToFileURL(handlerPath); + fileUrl.search = `t=${Date.now()}-${Math.random()}`; + const mod = await import(fileUrl.href); + return mod; +} + +test("GET with a path parameter hits the right URL and returns the server's JSON", async () => { + const observed: ObservedRequest[] = []; + const { baseUrl, close } = await startServer((req, res, body) => { + observed.push({ method: req.method, url: req.url, body }); + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ id: "42", title: "Dune" })); + }); + try { + const actions: DiscoveredAction[] = [ + { + name: "getBook", + method: "GET", + path: "/books/{book_id}", + parameters: [ + { + name: "book_id", + type: "string", + required: true, + in: "path", + }, + ], + }, + ]; + const { instantiate } = await loadHandler(baseUrl, actions); + const agent = instantiate(); + const result: any = await agent.executeAction( + { + actionName: "getBook", + parameters: { bookId: "42" }, + }, + {} as any, + ); + + assert.equal(observed.length, 1); + assert.equal(observed[0].method, "GET"); + assert.equal(observed[0].url, "/books/42"); + assert.match(result.displayContent as string, /"title": "Dune"/); + } finally { + await close(); + } +}); + +test("GET with a query parameter is sent as a query string", async () => { + const observed: ObservedRequest[] = []; + const { baseUrl, close } = await startServer((req, res, body) => { + observed.push({ method: req.method, url: req.url, body }); + res.writeHead(200, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ results: [] })); + }); + try { + const actions: DiscoveredAction[] = [ + { + name: "searchBooks", + method: "GET", + path: "/books", + parameters: [ + { + name: "limit", + type: "integer", + required: false, + in: "query", + }, + ], + }, + ]; + const { instantiate } = await loadHandler(baseUrl, actions); + const agent = instantiate(); + await agent.executeAction( + { actionName: "searchBooks", parameters: { limit: 5 } }, + {} as any, + ); + + assert.equal(observed.length, 1); + assert.equal(observed[0].method, "GET"); + assert.equal(observed[0].url, "/books?limit=5"); + } finally { + await close(); + } +}); + +test("POST sends a JSON body and the server's method/body are observed correctly", async () => { + const observed: ObservedRequest[] = []; + const { baseUrl, close } = await startServer((req, res, body) => { + observed.push({ method: req.method, url: req.url, body }); + res.writeHead(201, { "Content-Type": "application/json" }); + res.end(JSON.stringify({ id: "99" })); + }); + try { + const actions: DiscoveredAction[] = [ + { + name: "createBook", + method: "POST", + path: "/books", + parameters: [ + { + name: "title", + type: "string", + required: true, + in: "body", + }, + ], + }, + ]; + const { instantiate } = await loadHandler(baseUrl, actions); + const agent = instantiate(); + const result: any = await agent.executeAction( + { + actionName: "createBook", + parameters: { title: "Foundation" }, + }, + {} as any, + ); + + assert.equal(observed.length, 1); + assert.equal(observed[0].method, "POST"); + assert.equal(observed[0].url, "/books"); + assert.deepEqual(JSON.parse(observed[0].body), { title: "Foundation" }); + assert.match(result.displayContent as string, /"id": "99"/); + } finally { + await close(); + } +}); diff --git a/ts/packages/agents/onboarding/test/restRouting.spec.ts b/ts/packages/agents/onboarding/test/restRouting.spec.ts new file mode 100644 index 0000000000..8aa75f89c4 --- /dev/null +++ b/ts/packages/agents/onboarding/test/restRouting.spec.ts @@ -0,0 +1,98 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +// Tests for CHANGE 4 — guarded routing of the REST handler generator in +// scaffolderHandler.ts's buildHandler. REST generation must only kick in +// when an apiSurface has a resolved baseUrl AND the pattern is one that +// models a direct REST integration (schema-grammar default, external-api); +// explicit non-REST patterns must keep their own dedicated builders even +// when an apiSurface happens to be attached. + +import { test, describe } from "node:test"; +import assert from "node:assert/strict"; +import { buildHandler } from "../src/scaffolder/scaffolderHandler.js"; +import type { ApiSurface } from "../src/discovery/discoveryHandler.js"; + +function makeSurface(baseUrl?: string): ApiSurface { + return { + integrationName: "bookApi", + discoveredAt: new Date().toISOString(), + source: "https://api.example.com/openapi.json", + actions: [ + { + name: "getBook", + description: "Get a book", + method: "GET", + path: "/books/{book_id}", + parameters: [ + { + name: "book_id", + type: "string", + required: true, + in: "path", + }, + ], + sourceUrl: "https://api.example.com/openapi.json", + }, + ], + ...(baseUrl !== undefined ? { baseUrl } : {}), + }; +} + +describe("buildHandler REST routing", () => { + test("routes to the REST generator for the default (schema-grammar) pattern when baseUrl is set", async () => { + const output = await buildHandler( + "bookApi", + "BookApi", + "schema-grammar", + makeSurface("https://api.example.com/v3"), + ); + assert.match(output, /callRest\(/); + assert.match(output, /"getBook"/); + }); + + test("routes to the REST generator for the external-api pattern when baseUrl is set", async () => { + const output = await buildHandler( + "bookApi", + "BookApi", + "external-api", + makeSurface("https://api.example.com/v3"), + ); + assert.match(output, /callRest\(/); + }); + + test("does NOT route to REST for websocket-bridge even when baseUrl is set", async () => { + const output = await buildHandler( + "bookApi", + "BookApi", + "websocket-bridge", + makeSurface("https://api.example.com/v3"), + ); + assert.doesNotMatch(output, /callRest\(/); + }); + + test("does NOT route to REST for native-platform even when baseUrl is set", async () => { + const output = await buildHandler( + "bookApi", + "BookApi", + "native-platform", + makeSurface("https://api.example.com/v3"), + ); + assert.doesNotMatch(output, /callRest\(/); + }); + + test("falls back to the schema-grammar stub when no baseUrl is present", async () => { + const output = await buildHandler( + "bookApi", + "BookApi", + "schema-grammar", + makeSurface(undefined), + ); + assert.doesNotMatch(output, /callRest\(/); + }); + + test("falls back to the schema-grammar stub when there is no apiSurface at all", async () => { + const output = await buildHandler("bookApi", "BookApi"); + assert.doesNotMatch(output, /callRest\(/); + }); +}); diff --git a/ts/pnpm-lock.yaml b/ts/pnpm-lock.yaml index 15c833a57b..7369980e03 100644 --- a/ts/pnpm-lock.yaml +++ b/ts/pnpm-lock.yaml @@ -2992,6 +2992,9 @@ importers: rimraf: specifier: ^6.0.1 version: 6.0.1 + tsx: + specifier: ^4.21.0 + version: 4.21.0 typescript: specifier: ~5.4.5 version: 5.4.5