From cffc52bfb187469578d980c28578e5dee50c0ca4 Mon Sep 17 00:00:00 2001 From: MuHamza30 Date: Mon, 10 Aug 2026 15:57:46 +0500 Subject: [PATCH 01/12] feat: add `apimatic plugin generate` command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Uploads the build directory, polls generation status until a terminal state, then downloads a Claude Code context plugin into `/plugin` (or as `plugin.zip` with `--zip`). Unlike portal and SDK generation, this endpoint reports completion as a status body rather than a 302, and the run is bounded — a generation that never reaches a terminal status gives up after five minutes instead of polling forever. `PluginService` therefore keeps its own poll loop; deduping it against `portal-service.ts` is left to a follow-up. Deferred languages are reported as information, not failure. With `version` defaulting to v3 and C# the only generator emitting a plugin today, a partial result is the common case rather than the exception. Scoped to a `plugin-config.json` that already exists; creating one, and recording published SDKs into it from `sdk publish`, follow separately. Co-Authored-By: Claude Opus 5 (1M context) --- package.json | 3 + src/actions/plugin/generate.ts | 93 +++++ src/commands/plugin/generate.ts | 50 +++ src/infrastructure/service-error.ts | 9 +- src/infrastructure/services/plugin-service.ts | 262 ++++++++++++++ src/prompts/plugin/generate.ts | 83 +++++ src/types/plugin-context.ts | 29 ++ src/types/plugin/generation-status.ts | 51 +++ test/actions/plugin/generate.test.ts | 168 +++++++++ test/commands/examples-parse.test.ts | 1 + .../services/plugin-service.test.ts | 327 ++++++++++++++++++ test/types/plugin-context.test.ts | 63 ++++ 12 files changed, 1138 insertions(+), 1 deletion(-) create mode 100644 src/actions/plugin/generate.ts create mode 100644 src/commands/plugin/generate.ts create mode 100644 src/infrastructure/services/plugin-service.ts create mode 100644 src/prompts/plugin/generate.ts create mode 100644 src/types/plugin-context.ts create mode 100644 src/types/plugin/generation-status.ts create mode 100644 test/actions/plugin/generate.test.ts create mode 100644 test/infrastructure/services/plugin-service.test.ts create mode 100644 test/types/plugin-context.test.ts diff --git a/package.json b/package.json index 288228a9..1b821ab1 100644 --- a/package.json +++ b/package.json @@ -146,6 +146,9 @@ "auth": { "description": "Login using your APIMatic credentials, or view your authentication status." }, + "plugin": { + "description": "Generate context plugins that teach AI coding assistants to use your SDKs." + }, "portal": { "description": "Generate, download and serve an API Documentation portal for your APIs." }, diff --git a/src/actions/plugin/generate.ts b/src/actions/plugin/generate.ts new file mode 100644 index 00000000..4c5fa655 --- /dev/null +++ b/src/actions/plugin/generate.ts @@ -0,0 +1,93 @@ +import { withDirPath } from '../../infrastructure/tmp-extensions.js'; +import { PluginService } from '../../infrastructure/services/plugin-service.js'; +import { ServiceError } from '../../infrastructure/service-error.js'; +import { PluginGeneratePrompts } from '../../prompts/plugin/generate.js'; +import { BuildContext } from '../../types/build-context.js'; +import { CommandMetadata } from '../../types/common/command-metadata.js'; +import { DirectoryPath } from '../../types/file/directoryPath.js'; +import { PluginContext } from '../../types/plugin-context.js'; +import { TempContext } from '../../types/temp-context.js'; +import { ActionResult } from '../action-result.js'; + +export class PluginGenerateAction { + private readonly prompts: PluginGeneratePrompts = new PluginGeneratePrompts(); + private readonly pluginService: PluginService = new PluginService(); + private readonly configDir: DirectoryPath; + private readonly commandMetadata: CommandMetadata; + private readonly authKey: string | null; + + constructor(configDir: DirectoryPath, commandMetadata: CommandMetadata, authKey: string | null = null) { + this.configDir = configDir; + this.commandMetadata = commandMetadata; + this.authKey = authKey; + } + + public readonly execute = async ( + buildDirectory: DirectoryPath, + pluginDirectory: DirectoryPath, + force: boolean, + zipPlugin: boolean, + displayMessages: boolean = true + ): Promise => { + if (buildDirectory.isEqual(pluginDirectory)) { + this.prompts.directoryCannotBeSame(pluginDirectory); + return ActionResult.failed(); + } + + const buildContext = new BuildContext(buildDirectory); + if (!(await buildContext.validate())) { + this.prompts.srcDirectoryEmpty(buildDirectory); + return ActionResult.failed(); + } + + const pluginContext = new PluginContext(pluginDirectory); + if (!force && (await pluginContext.exists()) && !(await this.prompts.overwritePlugin(pluginDirectory))) { + this.prompts.pluginDirectoryNotEmpty(); + return ActionResult.cancelled(); + } + + return await withDirPath(async (tempDirectory) => { + const tempContext = new TempContext(tempDirectory); + const buildZipPath = await tempContext.zip(buildDirectory); + + const response = await this.prompts.generatePlugin( + this.pluginService.generatePlugin(buildZipPath, this.configDir, this.commandMetadata, this.authKey) + ); + + if (response.isErr()) { + this.reportGenerationError(response.error); + return ActionResult.failed(); + } + + const tempPluginZipPath = await tempContext.save(response.value.plugin); + await pluginContext.save(tempPluginZipPath, zipPlugin); + + if (response.value.deferred.length > 0) { + this.prompts.languagesDeferred(response.value.deferred); + } + + if (displayMessages) { + this.prompts.pluginGenerated(pluginDirectory); + } + + return ActionResult.success(); + }); + }; + + private reportGenerationError(error: ServiceError) { + const pluginConfigErrors = error.getError('pluginConfig'); + if (pluginConfigErrors) { + this.prompts.pluginConfigInvalid(pluginConfigErrors); + return; + } + + const sdkRepoErrors = error.getError('sdkRepos'); + if (sdkRepoErrors) { + this.prompts.noBuildableLanguages(sdkRepoErrors); + this.prompts.nextStepsPublishSdks(); + return; + } + + this.prompts.pluginGenerationError(error.errorMessage); + } +} diff --git a/src/commands/plugin/generate.ts b/src/commands/plugin/generate.ts new file mode 100644 index 00000000..c5b1f067 --- /dev/null +++ b/src/commands/plugin/generate.ts @@ -0,0 +1,50 @@ +import { Command, Flags } from '@oclif/core'; +import { PluginGenerateAction } from '../../actions/plugin/generate.js'; +import { CommandMetadata } from '../../types/common/command-metadata.js'; +import { DirectoryPath } from '../../types/file/directoryPath.js'; +import { FlagsProvider } from '../../types/flags-provider.js'; +import { format, intro, outro } from '../../prompts/format.js'; + +export default class PluginGenerate extends Command { + static readonly summary = 'Generate a Claude Code context plugin for your published SDKs.'; + + static readonly description = + 'Generate a context plugin that teaches an AI coding assistant how to use your SDKs. Requires an input directory containing a `src` directory with a `plugin-config.json`. Running without one creates the file so that `apimatic sdk publish` can record each SDK it publishes.'; + + static readonly cmdTxt = format.cmd('apimatic', 'plugin', 'generate'); + + static readonly examples = [ + PluginGenerate.cmdTxt, + `${PluginGenerate.cmdTxt} ${format.flag('input', '"./"')} ${format.flag('destination', '"./plugin"')}` + ]; + + static flags = { + zip: Flags.boolean({ + default: false, + description: 'Download the generated plugin as a .zip archive' + }), + ...FlagsProvider.input, + ...FlagsProvider.destination('plugin', 'plugin'), + ...FlagsProvider.force, + ...FlagsProvider.authKey + }; + + async run(): Promise { + const { + flags: { input, destination, force, zip: zipPlugin, 'auth-key': authKey } + } = await this.parse(PluginGenerate); + + const workingDirectory = DirectoryPath.createInput(input); + const buildDirectory = workingDirectory.join('src'); + const pluginDirectory = destination ? new DirectoryPath(destination) : workingDirectory.join('plugin'); + const commandMetadata: CommandMetadata = { + commandName: PluginGenerate.id, + shell: this.config.shell + }; + + intro('Generate Context Plugin'); + const action = new PluginGenerateAction(new DirectoryPath(this.config.configDir), commandMetadata, authKey); + const result = await action.execute(buildDirectory, pluginDirectory, force, zipPlugin); + outro(result); + } +} diff --git a/src/infrastructure/service-error.ts b/src/infrastructure/service-error.ts index d6af9802..9ed5e1b0 100644 --- a/src/infrastructure/service-error.ts +++ b/src/infrastructure/service-error.ts @@ -10,7 +10,8 @@ export enum ServiceErrorCode { InvalidResponse = "INVALID_RESPONSE", UnAuthorized = "UNAUTHORIZED", BadRequest = "BAD_REQUEST", - Forbidden = "FORBIDDEN" + Forbidden = "FORBIDDEN", + Timeout = "TIMEOUT" } export class ServiceError { @@ -32,6 +33,12 @@ export class ServiceError { static notFound(customMessage: string): ServiceError { return new ServiceError(ServiceErrorCode.NotFound, customMessage, {}); } + static timeout(customMessage: string): ServiceError { + return new ServiceError(ServiceErrorCode.Timeout, customMessage, {}); + } + static serverError(customMessage: string): ServiceError { + return new ServiceError(ServiceErrorCode.ServerError, customMessage, {}); + } static unauthorizedWithHint(apiMessage: string | null): ServiceError { // Both remedies name the full `auth login` command: the key is supplied to // that command, not to whichever one hit the 401 — most of them don't accept diff --git a/src/infrastructure/services/plugin-service.ts b/src/infrastructure/services/plugin-service.ts new file mode 100644 index 00000000..1e4c9364 --- /dev/null +++ b/src/infrastructure/services/plugin-service.ts @@ -0,0 +1,262 @@ +import axios from 'axios'; +import FormData from 'form-data'; +import { err, ok, Result } from 'neverthrow'; +import { AuthInfo, getAuthInfo } from '../../client-utils/auth-manager.js'; +import { CommandMetadata } from '../../types/common/command-metadata.js'; +import { DirectoryPath } from '../../types/file/directoryPath.js'; +import { FilePath } from '../../types/file/filePath.js'; +import { + GeneratedPluginResult, + isInFlight, + PluginGenerationInitiatedResponse, + PluginGenerationStatus, + PluginGenerationStatusResponse +} from '../../types/plugin/generation-status.js'; +import { discardStreamBody } from '../../utils/utils.js'; +import { envInfo } from '../env-info.js'; +import { FileService } from '../file-service.js'; +import { handleServiceError, ServiceError } from '../service-error.js'; + +const STATUS_POLL_INTERVAL_MS = 3000; +const GENERATION_TIMEOUT_MS = 5 * 60 * 1000; + +interface ProblemDetailsBody { + title?: string; + detail?: string; + errors?: Record; +} + +export class PluginService { + private readonly apiBaseUrl = 'https://api.apimatic.io' as const; + private readonly fileService = new FileService(); + private readonly statusPollIntervalMs: number; + private readonly generationTimeoutMs: number; + + constructor( + statusPollIntervalMs: number = STATUS_POLL_INTERVAL_MS, + generationTimeoutMs: number = GENERATION_TIMEOUT_MS + ) { + this.statusPollIntervalMs = statusPollIntervalMs; + this.generationTimeoutMs = generationTimeoutMs; + } + + public async generatePlugin( + buildPath: FilePath, + configDir: DirectoryPath, + commandMetadata: CommandMetadata, + authKey: string | null + ): Promise> { + const authInfo: AuthInfo | null = await getAuthInfo(configDir.toString()); + // `auth logout` blanks config.json rather than deleting it, so a logged-out user + // still has a non-null AuthInfo with an empty key — check the key, not the object. + const token = authKey || authInfo?.authKey; + if (!token) { + return err(ServiceError.unauthorizedWithHint(null)); + } + + const initiated = await this.initiateGeneration(buildPath, commandMetadata, token); + if (initiated.isErr()) { + return err(initiated.error); + } + + const generationId = initiated.value.id; + const completed = await pollUntilCompleted(this.statusPollIntervalMs, this.generationTimeoutMs, () => + this.getGenerationStatus(generationId, commandMetadata.shell, token) + ); + if (completed.isErr()) { + return err(completed.error); + } + + const download = await this.downloadPlugin(generationId, commandMetadata.shell, token); + if (download.isErr()) { + return err(download.error); + } + + return ok({ plugin: download.value, deferred: completed.value.deferred ?? [] }); + } + + private async initiateGeneration( + buildPath: FilePath, + commandMetadata: CommandMetadata, + token: string + ): Promise> { + const buildFileStream = await this.fileService.getStream(buildPath); + + try { + const formData = new FormData(); + formData.append('file', buildFileStream); + + const response = await this.axiosInstance(commandMetadata.shell, token).post('/plugin', formData, { + headers: formData.getHeaders(), + params: { origin: `APIMATIC CLI ${commandMetadata.commandName}` }, + responseType: 'json' + }); + + const id = (response.data as PluginGenerationInitiatedResponse | undefined)?.id; + return id ? ok({ id }) : err(ServiceError.InvalidResponse); + } catch (error) { + return err(mapProblemDetails(error) ?? handleServiceError(error)); + } finally { + buildFileStream.close(); + } + } + + private async getGenerationStatus( + generationId: string, + shell: string, + token: string + ): Promise> { + try { + const response = await this.axiosInstance(shell, token).get(`/plugin/${generationId}/status`, { + headers: { Accept: 'application/json' }, + // This endpoint reports completion as a status body, never a redirect. Refusing to + // follow one surfaces a contract change immediately instead of polling to the timeout. + maxRedirects: 0, + validateStatus: () => true + }); + + if (response.status === 200) { + return ok(response.data as PluginGenerationStatusResponse); + } + + // `validateStatus` above stops axios throwing, so nothing reaches the catch block. + if (response.status === 401) { + return err(ServiceError.unauthorizedWithHint(null)); + } + if (response.status === 404) { + return err(ServiceError.NotFound); + } + if (response.status === 500) { + return err(ServiceError.ServerError); + } + + return err(ServiceError.InvalidResponse); + } catch (error) { + return err(handleServiceError(error)); + } + } + + private async downloadPlugin( + generationId: string, + shell: string, + token: string + ): Promise> { + try { + const response = await this.axiosInstance(shell, token).get(`/plugin/${generationId}/download`, { + responseType: 'stream' + }); + return ok(response.data as NodeJS.ReadableStream); + } catch (error) { + // The body of a failed streamed response is itself a stream; leaving it open hangs the CLI. + if (axios.isAxiosError(error)) { + discardStreamBody(error.response?.data); + } + return err(handleServiceError(error)); + } + } + + private axiosInstance(shell: string, apiKey: string) { + return axios.create({ + baseURL: envInfo.getBaseUrl() ?? this.apiBaseUrl, + headers: { + 'User-Agent': envInfo.getUserAgent(shell), + Authorization: `X-Auth-Key ${apiKey}` + } + }); + } +} + +const TIMED_OUT_MESSAGE = 'Plugin generation timed out after 5 minutes.'; +const UNKNOWN_STATUS_MESSAGE = 'Unable to determine generation status. Please try again.'; + +type PollDecision = { kind: 'continue' } | { kind: 'done' } | { kind: 'error'; error: ServiceError }; + +/** + * Context plugin generation reports completion as a status body rather than a redirect, and + * unlike the portal and SDK endpoints it is bounded: a run that never reaches a terminal + * status gives up rather than polling forever. + */ +async function pollUntilCompleted( + pollIntervalMs: number, + timeoutMs: number, + fetchStatus: () => Promise> +): Promise> { + const startedAt = Date.now(); + + for (;;) { + await new Promise((resolve) => setTimeout(resolve, pollIntervalMs)); + + if (Date.now() - startedAt >= timeoutMs) { + return err(ServiceError.timeout(TIMED_OUT_MESSAGE)); + } + + const statusResult = await fetchStatus(); + if (statusResult.isErr()) { + return err(statusResult.error); + } + + const decision = classifyPluginStatus(statusResult.value); + if (decision.kind === 'done') { + return ok(statusResult.value); + } + if (decision.kind === 'error') { + return err(decision.error); + } + } +} + +function classifyPluginStatus({ status, errors }: PluginGenerationStatusResponse): PollDecision { + if (isInFlight(status)) { + return { kind: 'continue' }; + } + if (status === PluginGenerationStatus.Completed) { + return { kind: 'done' }; + } + if (status === PluginGenerationStatus.Failed) { + return { kind: 'error', error: ServiceError.ServerError }; + } + if (status === PluginGenerationStatus.ValidationError) { + const validationErrors = errors ?? {}; + return { + kind: 'error', + error: ServiceError.badRequest(formatValidationErrors(validationErrors), validationErrors) + }; + } + // `Unknown`, and anything a newer backend adds, ends the run rather than polling to the timeout. + return { kind: 'error', error: ServiceError.serverError(UNKNOWN_STATUS_MESSAGE) }; +} + +const formatValidationErrors = (errors: Record): string => { + const messages = Object.values(errors).flat(); + return 'One or more validation errors occurred.' + (messages.length ? '\n- ' + messages.join('\n- ') : ''); +}; + +/** + * `handleServiceError` only reads ProblemDetails off the SDK's typed errors, so a raw axios + * 400/403/404 would otherwise collapse into a generic server error and lose its message. + * Unlike the SDK path this also falls back to `detail`, which io uses when it sends no + * `errors` map. + */ +function mapProblemDetails(error: unknown): ServiceError | undefined { + if (!axios.isAxiosError(error)) { + return undefined; + } + + const body = error.response?.data as ProblemDetailsBody | undefined; + if (typeof body !== 'object' || body === null) { + return undefined; + } + + const errors = body.errors ?? {}; + const firstMessage = Object.values(errors).flat()[0] ?? body.detail; + const title = body.title ?? 'Request failed.'; + const message = firstMessage ? `${title}\n- ${firstMessage}` : title; + + if (error.response?.status === 400) { + return ServiceError.badRequest(message, errors); + } + if (error.response?.status === 403) { + return ServiceError.forbidden(message); + } + return undefined; +} diff --git a/src/prompts/plugin/generate.ts b/src/prompts/plugin/generate.ts new file mode 100644 index 00000000..7824e031 --- /dev/null +++ b/src/prompts/plugin/generate.ts @@ -0,0 +1,83 @@ +import { confirm, isCancel, log } from '@clack/prompts'; +import { Result } from 'neverthrow'; +import { ServiceError } from '../../infrastructure/service-error.js'; +import { DirectoryPath } from '../../types/file/directoryPath.js'; +import { DeferralReason, DeferredLanguage, GeneratedPluginResult } from '../../types/plugin/generation-status.js'; +import { format as f } from '../format.js'; +import { noteWrapped, withSpinner } from '../prompt.js'; + +const PLUGIN_CONFIG_FILE = 'plugin-config.json'; + +/** The wire carries the reason's name; the sentence that explains it lives here. */ +const DEFERRAL_DETAIL: Record = { + [DeferralReason.NoPluginGenerator]: 'has no context plugin generator', + [DeferralReason.TargetsV3]: 'targets v3, which this generator does not build' +}; + +export class PluginGeneratePrompts { + public generatePlugin(fn: Promise>) { + return withSpinner('Generating Context Plugin', 'Plugin generated successfully.', 'Plugin Generation failed.', fn); + } + + public async overwritePlugin(directory: DirectoryPath): Promise { + const overwrite = await confirm({ + message: `The destination ${f.path(directory)} is not empty, do you want to overwrite?`, + initialValue: false + }); + + if (isCancel(overwrite)) { + return false; + } + + return overwrite; + } + + public directoryCannotBeSame(directory: DirectoryPath) { + const message = `The ${f.var('src')} and ${f.var('plugin')} directories must be different. Current value: ${f.path( + directory + )}`; + log.error(message); + } + + public srcDirectoryEmpty(directory: DirectoryPath) { + const message = `The ${f.var('src')} directory is either empty or invalid: ${f.path(directory)}`; + log.error(message); + } + + public pluginDirectoryNotEmpty() { + log.error('Please enter a different destination folder or remove the existing files and try again.'); + } + + public pluginGenerationError(error: string) { + log.error(error); + } + + public pluginConfigInvalid(messages: string[]) { + const message = `Your ${f.var(PLUGIN_CONFIG_FILE)} is invalid:\n- ${messages.join('\n- ')}`; + log.error(message); + } + + public noBuildableLanguages(messages: string[]) { + const message = `No language in ${f.var('sdkRepos')} can be built yet:\n- ${messages.join('\n- ')}`; + log.error(message); + } + + public languagesDeferred(deferred: DeferredLanguage[]) { + const lines = deferred.map( + ({ language, reason }) => `Skipped ${f.var(language)} — ${DEFERRAL_DETAIL[reason] ?? 'is not supported yet'}.` + ); + log.info(lines.join('\n')); + } + + public nextStepsPublishSdks() { + const message = + `Publish an SDK for each language you want in the plugin:\n` + + `'${f.cmdAlt('apimatic', 'sdk', 'publish')} ${f.flag('language', '')}'\n` + + `Then run '${f.cmdAlt('apimatic', 'plugin', 'generate')}'.`; + noteWrapped(message, 'Next Steps'); + } + + public pluginGenerated(plugin: DirectoryPath) { + log.info(`Plugin artifacts can be found at ${f.path(plugin)}.`); + } +} diff --git a/src/types/plugin-context.ts b/src/types/plugin-context.ts new file mode 100644 index 00000000..2df756b7 --- /dev/null +++ b/src/types/plugin-context.ts @@ -0,0 +1,29 @@ +import { FileService } from '../infrastructure/file-service.js'; +import { ZipService } from '../infrastructure/zip-service.js'; +import { DirectoryPath } from './file/directoryPath.js'; +import { FileName } from './file/fileName.js'; +import { FilePath } from './file/filePath.js'; + +export class PluginContext { + private readonly fileService = new FileService(); + private readonly zipService = new ZipService(); + + constructor(private readonly pluginDirectory: DirectoryPath) {} + + private get zipPath(): FilePath { + return new FilePath(this.pluginDirectory, new FileName('plugin.zip')); + } + + public async exists(): Promise { + return !(await this.fileService.directoryEmpty(this.pluginDirectory)); + } + + public async save(tempPluginFilePath: FilePath, zipPlugin: boolean): Promise { + await this.fileService.cleanDirectory(this.pluginDirectory); + if (zipPlugin) { + await this.fileService.copy(tempPluginFilePath, this.zipPath); + } else { + await this.zipService.unArchive(tempPluginFilePath, this.pluginDirectory); + } + } +} diff --git a/src/types/plugin/generation-status.ts b/src/types/plugin/generation-status.ts new file mode 100644 index 00000000..c6434d5c --- /dev/null +++ b/src/types/plugin/generation-status.ts @@ -0,0 +1,51 @@ +/** + * Context plugin generation reports its own status vocabulary, which does not match the + * shared `Status` enum in `@apimatic/sdk`: there is no `SubscriptionError` (entitlement is + * a 403 on the generate call instead), completion is a status rather than a redirect, and + * the three in-flight values below have no equivalent. + */ +export enum PluginGenerationStatus { + Queued = 'Queued', + ExecutionStarted = 'ExecutionStarted', + GeneratingArtifacts = 'GeneratingArtifacts', + Completed = 'Completed', + Failed = 'Failed', + ValidationError = 'ValidationError', + Unknown = 'Unknown' +} + +export enum DeferralReason { + TargetsV3 = 'TargetsV3', + NoPluginGenerator = 'NoPluginGenerator' +} + +/** A language named in the config that this generator skipped; generation still succeeds. */ +export interface DeferredLanguage { + language: string; + reason: DeferralReason | string; +} + +export interface PluginGenerationStatusResponse { + status: PluginGenerationStatus; + errors?: Record; + deferred?: DeferredLanguage[]; +} + +export interface PluginGenerationInitiatedResponse { + id: string; +} + +export interface GeneratedPluginResult { + plugin: NodeJS.ReadableStream; + deferred: DeferredLanguage[]; +} + +const IN_FLIGHT: ReadonlySet = new Set([ + PluginGenerationStatus.Queued, + PluginGenerationStatus.ExecutionStarted, + PluginGenerationStatus.GeneratingArtifacts +]); + +export function isInFlight(status: PluginGenerationStatus): boolean { + return IN_FLIGHT.has(status); +} diff --git a/test/actions/plugin/generate.test.ts b/test/actions/plugin/generate.test.ts new file mode 100644 index 00000000..271c91ae --- /dev/null +++ b/test/actions/plugin/generate.test.ts @@ -0,0 +1,168 @@ +import * as path from 'path'; +import { Readable } from 'node:stream'; +import fsExtra from 'fs-extra'; +import sinon from 'sinon'; +import { expect } from 'chai'; +import { err, ok } from 'neverthrow'; +import { dir as tmpDir, DirectoryResult } from 'tmp-promise'; +import { PluginGenerateAction } from '../../../src/actions/plugin/generate.js'; +import { PluginGeneratePrompts } from '../../../src/prompts/plugin/generate.js'; +import { PluginService } from '../../../src/infrastructure/services/plugin-service.js'; +import { ServiceError } from '../../../src/infrastructure/service-error.js'; +import { DirectoryPath } from '../../../src/types/file/directoryPath.js'; +import { CommandMetadata } from '../../../src/types/common/command-metadata.js'; + +const COMMAND_METADATA: CommandMetadata = { commandName: 'plugin generate', shell: 'test' }; + +describe('PluginGenerateAction', () => { + let tmpDirResult: DirectoryResult; + let buildDirectory: string; + let pluginDirectory: string; + let action: PluginGenerateAction; + + const execute = (force = false, zipPlugin = true) => + action.execute(new DirectoryPath(buildDirectory), new DirectoryPath(pluginDirectory), force, zipPlugin); + + const generated = (deferred: { language: string; reason: string }[] = []) => + sinon + .stub(PluginService.prototype, 'generatePlugin') + .resolves(ok({ plugin: Readable.from(['PK context-plugin']), deferred })); + + beforeEach(async () => { + tmpDirResult = await tmpDir({ unsafeCleanup: true }); + const workingDirectory = path.join(tmpDirResult.path, 'acme-payments'); + buildDirectory = path.join(workingDirectory, 'src'); + pluginDirectory = path.join(workingDirectory, 'plugin'); + await fsExtra.ensureDir(buildDirectory); + await fsExtra.writeJson(path.join(buildDirectory, 'APIMATIC-BUILD.json'), {}); + await fsExtra.writeJson(path.join(buildDirectory, 'plugin-config.json'), { + schemaVersion: 1, + pluginId: 'acme-payments', + pluginName: 'Acme Payments', + sdkRepos: { csharp: { sourceCode: { srcCodeUrl: 'https://github.com/acme/acme-csharp' } } } + }); + + // The spinner would render to stdout; pass the underlying promise straight through. + sinon.stub(PluginGeneratePrompts.prototype, 'generatePlugin').callsFake((fn) => fn); + + action = new PluginGenerateAction(new DirectoryPath(tmpDirResult.path), COMMAND_METADATA, 'auth-key'); + }); + + afterEach(async () => { + sinon.restore(); + await tmpDirResult.cleanup(); + }); + + describe('input guards', () => { + it('fails when the build and plugin directories are the same', async () => { + const generatePlugin = sinon.stub(PluginService.prototype, 'generatePlugin'); + + const result = await action.execute( + new DirectoryPath(buildDirectory), + new DirectoryPath(buildDirectory), + false, + true + ); + + expect(result.isFailed()).to.be.true; + expect(generatePlugin.called).to.be.false; + }); + + it('fails when the build directory has no APIMATIC-BUILD.json', async () => { + const generatePlugin = sinon.stub(PluginService.prototype, 'generatePlugin'); + await fsExtra.remove(path.join(buildDirectory, 'APIMATIC-BUILD.json')); + + expect((await execute()).isFailed()).to.be.true; + expect(generatePlugin.called).to.be.false; + }); + }); + + describe('overwrite guard', () => { + beforeEach(() => fsExtra.outputFile(path.join(pluginDirectory, 'stale.md'), 'from a previous run')); + + it('cancels when the destination is not empty and the user declines', async () => { + const generatePlugin = sinon.stub(PluginService.prototype, 'generatePlugin'); + sinon.stub(PluginGeneratePrompts.prototype, 'overwritePlugin').resolves(false); + + expect((await execute()).isCancelled()).to.be.true; + expect(generatePlugin.called).to.be.false; + }); + + it('proceeds when the user accepts', async () => { + generated(); + sinon.stub(PluginGeneratePrompts.prototype, 'overwritePlugin').resolves(true); + + expect((await execute()).isSuccess()).to.be.true; + }); + + it('never asks when --force is set', async () => { + generated(); + const overwritePlugin = sinon.stub(PluginGeneratePrompts.prototype, 'overwritePlugin'); + + expect((await execute(true)).isSuccess()).to.be.true; + expect(overwritePlugin.called).to.be.false; + }); + }); + + describe('generation', () => { + it('saves the downloaded artifact into the plugin directory', async () => { + generated(); + + const result = await execute(); + + expect(result.isSuccess()).to.be.true; + expect(fsExtra.readFileSync(path.join(pluginDirectory, 'plugin.zip'), 'utf-8')).to.equal('PK context-plugin'); + }); + + it('reports deferred languages without failing the run', async () => { + generated([{ language: 'python', reason: 'NoPluginGenerator' }]); + const languagesDeferred = sinon.stub(PluginGeneratePrompts.prototype, 'languagesDeferred'); + + const result = await execute(); + + expect(result.isSuccess()).to.be.true; + expect(languagesDeferred.firstCall.args[0]).to.deep.equal([{ language: 'python', reason: 'NoPluginGenerator' }]); + }); + + it('stays quiet about deferrals when there are none', async () => { + generated(); + const languagesDeferred = sinon.stub(PluginGeneratePrompts.prototype, 'languagesDeferred'); + + await execute(); + + expect(languagesDeferred.called).to.be.false; + }); + }); + + describe('generation failures', () => { + it('routes plugin-config validation errors to their own message', async () => { + sinon + .stub(PluginService.prototype, 'generatePlugin') + .resolves(err(ServiceError.badRequest('invalid', { pluginConfig: ["'pluginId' must be kebab-case."] }))); + const pluginConfigInvalid = sinon.stub(PluginGeneratePrompts.prototype, 'pluginConfigInvalid'); + + expect((await execute()).isFailed()).to.be.true; + expect(pluginConfigInvalid.firstCall.args[0]).to.deep.equal(["'pluginId' must be kebab-case."]); + }); + + it('routes sdkRepos validation errors to the next-steps message', async () => { + sinon + .stub(PluginService.prototype, 'generatePlugin') + .resolves(err(ServiceError.badRequest('invalid', { sdkRepos: ['nothing buildable'] }))); + const noBuildableLanguages = sinon.stub(PluginGeneratePrompts.prototype, 'noBuildableLanguages'); + const nextSteps = sinon.stub(PluginGeneratePrompts.prototype, 'nextStepsPublishSdks'); + + expect((await execute()).isFailed()).to.be.true; + expect(noBuildableLanguages.firstCall.args[0]).to.deep.equal(['nothing buildable']); + expect(nextSteps.called).to.be.true; + }); + + it('falls back to the plain service message for any other failure', async () => { + sinon.stub(PluginService.prototype, 'generatePlugin').resolves(err(ServiceError.ServerError)); + const pluginGenerationError = sinon.stub(PluginGeneratePrompts.prototype, 'pluginGenerationError'); + + expect((await execute()).isFailed()).to.be.true; + expect(pluginGenerationError.called).to.be.true; + }); + }); +}); diff --git a/test/commands/examples-parse.test.ts b/test/commands/examples-parse.test.ts index 3a4bbf17..80d6299c 100644 --- a/test/commands/examples-parse.test.ts +++ b/test/commands/examples-parse.test.ts @@ -16,6 +16,7 @@ const COMMANDS: CommandMapping[] = [ { id: "auth login", fileParts: ["commands", "auth", "login.js"] }, { id: "auth logout", fileParts: ["commands", "auth", "logout.js"] }, { id: "auth status", fileParts: ["commands", "auth", "status.js"] }, + { id: "plugin generate", fileParts: ["commands", "plugin", "generate.js"] }, { id: "portal copilot", fileParts: ["commands", "portal", "copilot.js"] }, { id: "portal generate", fileParts: ["commands", "portal", "generate.js"], exportName: "PortalGenerate" }, { id: "portal recipe new", fileParts: ["commands", "portal", "recipe", "new.js"] }, diff --git a/test/infrastructure/services/plugin-service.test.ts b/test/infrastructure/services/plugin-service.test.ts new file mode 100644 index 00000000..6319503e --- /dev/null +++ b/test/infrastructure/services/plugin-service.test.ts @@ -0,0 +1,327 @@ +import { expect } from 'chai'; +import http from 'node:http'; +import { AddressInfo } from 'node:net'; +import fs from 'node:fs'; +import os from 'node:os'; +import path from 'node:path'; +import { Buffer } from 'node:buffer'; +import { PluginService } from '../../../src/infrastructure/services/plugin-service'; +import { DirectoryPath } from '../../../src/types/file/directoryPath'; +import { FilePath } from '../../../src/types/file/filePath'; +import { ServiceError, ServiceErrorCode } from '../../../src/infrastructure/service-error'; +import { envInfo } from '../../../src/infrastructure/env-info'; + +describe('PluginService', () => { + const GENERATION_ID = '019fdbef-91d5-7a1f-81c2-866af5d76fe9'; + const AUTH_KEY = 'test-auth-key'; + const metadata = { commandName: 'plugin generate', shell: 'bash' }; + + let server: http.Server; + let workDir: string; + let buildPath: FilePath; + let configDir: DirectoryPath; + let service: PluginService; + + let respondToGenerate: (res: http.ServerResponse) => void; + let respondToStatus: (res: http.ServerResponse, attempt: number) => void; + let respondToDownload: (res: http.ServerResponse) => void; + + const generateRequests: { url: string; headers: http.IncomingHttpHeaders; body: string }[] = []; + const statusRequests: { url: string; headers: http.IncomingHttpHeaders }[] = []; + + const json = (res: http.ServerResponse, statusCode: number, body: unknown) => { + res.writeHead(statusCode, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify(body)); + }; + + const problem = (res: http.ServerResponse, statusCode: number, body: unknown) => { + res.writeHead(statusCode, { 'Content-Type': 'application/problem+json' }); + res.end(JSON.stringify(body)); + }; + + const statusBody = (body: unknown) => (res: http.ServerResponse) => json(res, 200, body); + + const drain = async (stream: NodeJS.ReadableStream) => { + const chunks: Buffer[] = []; + for await (const chunk of stream) chunks.push(Buffer.from(chunk)); + return Buffer.concat(chunks).toString(); + }; + + const clearBaseUrlCache = () => { + (envInfo.constructor as unknown as { cachedBaseUrl?: string }).cachedBaseUrl = undefined; + }; + + before(async () => { + server = http.createServer((req, res) => { + const url = req.url ?? ''; + + if (req.method === 'POST') { + const chunks: Buffer[] = []; + req.on('data', (chunk) => chunks.push(Buffer.from(chunk))); + req.on('end', () => { + generateRequests.push({ url, headers: req.headers, body: Buffer.concat(chunks).toString() }); + respondToGenerate(res); + }); + return; + } + + if (url.endsWith('/status')) { + statusRequests.push({ url, headers: req.headers }); + respondToStatus(res, statusRequests.length); + return; + } + + if (url.endsWith('/download')) { + respondToDownload(res); + return; + } + + res.writeHead(404); + res.end(); + }); + + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)); + const { port } = server.address() as AddressInfo; + clearBaseUrlCache(); + process.env.APIMATIC_BASE_URL = `http://127.0.0.1:${port}`; + + workDir = fs.mkdtempSync(path.join(os.tmpdir(), 'plugin-service-')); + const buildFile = path.join(workDir, 'build.zip'); + fs.writeFileSync(buildFile, Buffer.from('PK build-input')); + buildPath = FilePath.create(buildFile)!; + // No config.json here, so the explicit authKey is used. + configDir = new DirectoryPath(workDir); + // Near-zero interval and budget so the suite is not paced by the production 3s / 5min. + service = new PluginService(1, 5000); + }); + + after(async () => { + delete process.env.APIMATIC_BASE_URL; + // Static cache — leaving it set would point every later suite at this closed server. + clearBaseUrlCache(); + fs.rmSync(workDir, { recursive: true, force: true }); + await new Promise((resolve, reject) => server.close((error) => (error ? reject(error) : resolve()))); + }); + + beforeEach(() => { + generateRequests.length = 0; + statusRequests.length = 0; + respondToGenerate = (res) => json(res, 202, { id: GENERATION_ID }); + respondToStatus = statusBody({ status: 'Completed' }); + respondToDownload = (res) => { + res.writeHead(200, { 'Content-Type': 'application/zip' }); + res.end(Buffer.from('PK context-plugin')); + }; + }); + + const generatePlugin = (authKey: string | null = AUTH_KEY) => + service.generatePlugin(buildPath, configDir, metadata, authKey); + const errorFrom = (result: { _unsafeUnwrapErr(): unknown }) => result._unsafeUnwrapErr() as ServiceError; + + describe('generatePlugin', () => { + it('uploads the build, polls the plugin status path and downloads once complete', async () => { + const result = await generatePlugin(); + + expect(result.isOk()).to.be.true; + expect(await drain(result._unsafeUnwrap().plugin)).to.equal('PK context-plugin'); + expect(generateRequests).to.have.length(1); + expect(statusRequests[0].url).to.equal(`/plugin/${GENERATION_ID}/status`); + }); + + it('sends the build as a multipart file part, authenticated, tagged with the command origin', async () => { + await generatePlugin(); + + const request = generateRequests[0]; + expect(request.url).to.equal('/plugin?origin=APIMATIC+CLI+plugin+generate'); + expect(request.headers.authorization).to.equal(`X-Auth-Key ${AUTH_KEY}`); + expect(request.headers['content-type']).to.match(/^multipart\/form-data; boundary=/); + expect(request.body).to.include('name="file"'); + expect(request.body).to.include('PK build-input'); + }); + + it('authenticates the status and download calls too', async () => { + await generatePlugin(); + + expect(statusRequests[0].headers.authorization).to.equal(`X-Auth-Key ${AUTH_KEY}`); + }); + + it('keeps polling through every in-flight status', async () => { + const inFlight = ['Queued', 'ExecutionStarted', 'GeneratingArtifacts']; + respondToStatus = (res, attempt) => json(res, 200, { status: inFlight[attempt - 1] ?? 'Completed' }); + + const result = await generatePlugin(); + + expect(result.isOk()).to.be.true; + expect(statusRequests).to.have.length(4); + }); + + it('reports the languages the backend deferred alongside the artifact', async () => { + respondToStatus = statusBody({ + status: 'Completed', + deferred: [ + { language: 'python', reason: 'NoPluginGenerator' }, + { language: 'java', reason: 'TargetsV3' } + ] + }); + + const result = await generatePlugin(); + + expect(result._unsafeUnwrap().deferred).to.deep.equal([ + { language: 'python', reason: 'NoPluginGenerator' }, + { language: 'java', reason: 'TargetsV3' } + ]); + }); + + it('reports no deferrals when the status omits them', async () => { + const result = await generatePlugin(); + + expect(result._unsafeUnwrap().deferred).to.deep.equal([]); + }); + + it('refuses to call the API at all without a key', async () => { + const result = await generatePlugin(null); + + expect(errorFrom(result).code).to.equal(ServiceErrorCode.UnAuthorized); + expect(generateRequests).to.have.length(0); + }); + }); + + describe('validation failures', () => { + it('keeps the plugin-config errors addressable by key and lists every message', async () => { + respondToStatus = statusBody({ + status: 'ValidationError', + errors: { pluginConfig: ["'pluginId' must be lower-case kebab-case.", "'author.email' must be an email."] } + }); + + const error = errorFrom(await generatePlugin()); + + expect(error.code).to.equal(ServiceErrorCode.BadRequest); + expect(error.getError('pluginConfig')).to.have.length(2); + expect(error.errorMessage).to.include("'pluginId' must be lower-case kebab-case."); + expect(error.errorMessage).to.include("'author.email' must be an email."); + }); + + it('keeps the sdkRepos errors addressable by key', async () => { + respondToStatus = statusBody({ + status: 'ValidationError', + errors: { sdkRepos: ['no language in sdkRepos can be built by this generator'] } + }); + + const error = errorFrom(await generatePlugin()); + + expect(error.getError('sdkRepos')).to.deep.equal(['no language in sdkRepos can be built by this generator']); + }); + + it('terminates on a validation error that carries no messages', async () => { + respondToStatus = statusBody({ status: 'ValidationError' }); + + const error = errorFrom(await generatePlugin()); + + expect(error.errorMessage).to.equal('One or more validation errors occurred.'); + }); + + it('maps a Failed status onto a server error', async () => { + respondToStatus = statusBody({ status: 'Failed' }); + + expect(errorFrom(await generatePlugin()).code).to.equal(ServiceErrorCode.ServerError); + }); + + it('stops on an Unknown status rather than polling to the timeout', async () => { + respondToStatus = statusBody({ status: 'Unknown' }); + + const error = errorFrom(await generatePlugin()); + + expect(error.errorMessage).to.equal('Unable to determine generation status. Please try again.'); + expect(statusRequests).to.have.length(1); + }); + + it('gives up once the generation budget is spent', async () => { + const impatient = new PluginService(1, 15); + respondToStatus = statusBody({ status: 'GeneratingArtifacts' }); + + const result = await impatient.generatePlugin(buildPath, configDir, metadata, AUTH_KEY); + + expect(errorFrom(result).code).to.equal(ServiceErrorCode.Timeout); + expect(errorFrom(result).errorMessage).to.equal('Plugin generation timed out after 5 minutes.'); + }); + }); + + describe('request errors', () => { + it('surfaces the invalid-zip 400 that io originates', async () => { + respondToGenerate = (res) => + problem(res, 400, { + title: 'One or more validation errors occurred.', + detail: 'The provided build file is not a valid zip archive.', + errors: { file: ['build file should be in valid zip format'] } + }); + + const error = errorFrom(await generatePlugin()); + + expect(error.code).to.equal(ServiceErrorCode.BadRequest); + expect(error.errorMessage).to.equal( + 'One or more validation errors occurred.\n- build file should be in valid zip format' + ); + }); + + it('surfaces the entitlement 403 that io originates', async () => { + respondToGenerate = (res) => + problem(res, 403, { + title: 'Access denied to resource.', + errors: { '': ['Context plugin generation is not allowed on your subscription'] } + }); + + const error = errorFrom(await generatePlugin()); + + expect(error.code).to.equal(ServiceErrorCode.Forbidden); + expect(error.errorMessage).to.equal( + 'Access denied to resource.\n- Context plugin generation is not allowed on your subscription' + ); + }); + + it('falls back to detail when a problem body carries no errors map', async () => { + respondToGenerate = (res) => + problem(res, 400, { title: 'One or more validation errors occurred.', detail: "Plugin id 'x' not found" }); + + expect(errorFrom(await generatePlugin()).errorMessage).to.equal( + "One or more validation errors occurred.\n- Plugin id 'x' not found" + ); + }); + + it('points an expired key at the login command', async () => { + respondToGenerate = (res) => { + res.writeHead(401); + res.end(); + }; + + const error = errorFrom(await generatePlugin()); + + expect(error.code).to.equal(ServiceErrorCode.UnAuthorized); + expect(error.errorMessage).to.include('auth'); + }); + + it('reports a generate response that carries no id', async () => { + respondToGenerate = (res) => json(res, 202, {}); + + expect(errorFrom(await generatePlugin()).code).to.equal(ServiceErrorCode.InvalidResponse); + }); + + it('stops polling when the generation id is unknown', async () => { + respondToStatus = (res) => { + res.writeHead(404); + res.end(); + }; + + expect(errorFrom(await generatePlugin()).code).to.equal(ServiceErrorCode.NotFound); + expect(statusRequests).to.have.length(1); + }); + + it('reports a failed download without hanging on its body', async () => { + respondToDownload = (res) => { + res.writeHead(500, { 'Content-Type': 'application/json' }); + res.end(JSON.stringify({ title: 'boom' })); + }; + + expect(errorFrom(await generatePlugin()).code).to.equal(ServiceErrorCode.ServerError); + }); + }); +}); diff --git a/test/types/plugin-context.test.ts b/test/types/plugin-context.test.ts new file mode 100644 index 00000000..5b8294f4 --- /dev/null +++ b/test/types/plugin-context.test.ts @@ -0,0 +1,63 @@ +import fs from 'fs'; +import path from 'path'; +import mockFs from 'mock-fs'; +import { expect } from 'chai'; +import { PluginContext } from '../../src/types/plugin-context'; +import { DirectoryPath } from '../../src/types/file/directoryPath'; +import { FileName } from '../../src/types/file/fileName'; +import { FilePath } from '../../src/types/file/filePath'; + +describe('PluginContext', () => { + const pluginDirectory = new DirectoryPath('plugin'); + const tempDirectory = new DirectoryPath('temp'); + const tempZip = new FilePath(tempDirectory, new FileName('downloaded')); + const context = new PluginContext(pluginDirectory); + + afterEach(() => mockFs.restore()); + + describe('exists', () => { + it('is false when the directory is absent', async () => { + mockFs({}); + + expect(await context.exists()).to.be.false; + }); + + it('is false when the directory holds nothing but dotfiles', async () => { + mockFs({ plugin: { '.gitkeep': '' } }); + + expect(await context.exists()).to.be.false; + }); + + it('is true when the directory holds artifacts', async () => { + mockFs({ plugin: { 'README.md': '# plugin' } }); + + expect(await context.exists()).to.be.true; + }); + }); + + describe('save', () => { + it('copies the archive as plugin.zip when asked to keep it zipped', async () => { + mockFs({ plugin: {}, temp: { downloaded: 'zip-bytes' } }); + + await context.save(tempZip, true); + + expect(fs.readFileSync(path.join(pluginDirectory.toString(), 'plugin.zip'), 'utf-8')).to.equal('zip-bytes'); + }); + + it('clears artifacts from a previous run before saving', async () => { + mockFs({ plugin: { 'stale.md': 'from a language that is gone' }, temp: { downloaded: 'zip-bytes' } }); + + await context.save(tempZip, true); + + expect(fs.existsSync(path.join(pluginDirectory.toString(), 'stale.md'))).to.be.false; + }); + + it('creates the destination directory when it does not exist yet', async () => { + mockFs({ temp: { downloaded: 'zip-bytes' } }); + + await context.save(tempZip, true); + + expect(fs.existsSync(path.join(pluginDirectory.toString(), 'plugin.zip'))).to.be.true; + }); + }); +}); From bdbef1ba54228b96904d9d89f2dc266bcb967341 Mon Sep 17 00:00:00 2001 From: MuHamza30 Date: Tue, 11 Aug 2026 11:01:51 +0500 Subject: [PATCH 02/12] fix: name the login remedy on every plugin generate 401 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `handleServiceError` maps an axios 401 onto the bare "Unauthorized access.", so a key that expires mid-run left the user with nothing to act on. Route every call through `mapTransportError` so upload, poll and download all offer the remedy the poll already did, and assert the rendered message instead of a substring — the previous test passed on "Unauthorized" containing "auth". Drop the deferred-language reporting. codegen-v2 no longer returns `deferred`; it proxies v3 languages to codegen v3 instead, so the enum, the prompt and the result wrapper were dead against the current backend. Trim the command description to what this PR ships — creating `plugin-config.json` and recording SDKs into it land later — and mark `flags` readonly like the other statics. Co-Authored-By: Claude Opus 5 (1M context) --- src/actions/plugin/generate.ts | 6 +-- src/commands/plugin/generate.ts | 4 +- src/infrastructure/services/plugin-service.ts | 32 ++++++++++----- src/prompts/plugin/generate.ts | 16 +------- src/types/plugin/generation-status.ts | 17 -------- test/actions/plugin/generate.test.ts | 25 +----------- .../services/plugin-service.test.ts | 40 +++++++------------ 7 files changed, 43 insertions(+), 97 deletions(-) diff --git a/src/actions/plugin/generate.ts b/src/actions/plugin/generate.ts index 4c5fa655..98225ca5 100644 --- a/src/actions/plugin/generate.ts +++ b/src/actions/plugin/generate.ts @@ -59,13 +59,9 @@ export class PluginGenerateAction { return ActionResult.failed(); } - const tempPluginZipPath = await tempContext.save(response.value.plugin); + const tempPluginZipPath = await tempContext.save(response.value); await pluginContext.save(tempPluginZipPath, zipPlugin); - if (response.value.deferred.length > 0) { - this.prompts.languagesDeferred(response.value.deferred); - } - if (displayMessages) { this.prompts.pluginGenerated(pluginDirectory); } diff --git a/src/commands/plugin/generate.ts b/src/commands/plugin/generate.ts index c5b1f067..4d551cd2 100644 --- a/src/commands/plugin/generate.ts +++ b/src/commands/plugin/generate.ts @@ -9,7 +9,7 @@ export default class PluginGenerate extends Command { static readonly summary = 'Generate a Claude Code context plugin for your published SDKs.'; static readonly description = - 'Generate a context plugin that teaches an AI coding assistant how to use your SDKs. Requires an input directory containing a `src` directory with a `plugin-config.json`. Running without one creates the file so that `apimatic sdk publish` can record each SDK it publishes.'; + 'Generate a context plugin that teaches an AI coding assistant how to use your SDKs. Requires an input directory containing a `src` directory with a `plugin-config.json`.'; static readonly cmdTxt = format.cmd('apimatic', 'plugin', 'generate'); @@ -18,7 +18,7 @@ export default class PluginGenerate extends Command { `${PluginGenerate.cmdTxt} ${format.flag('input', '"./"')} ${format.flag('destination', '"./plugin"')}` ]; - static flags = { + static readonly flags = { zip: Flags.boolean({ default: false, description: 'Download the generated plugin as a .zip archive' diff --git a/src/infrastructure/services/plugin-service.ts b/src/infrastructure/services/plugin-service.ts index 1e4c9364..7f53b86c 100644 --- a/src/infrastructure/services/plugin-service.ts +++ b/src/infrastructure/services/plugin-service.ts @@ -6,7 +6,6 @@ import { CommandMetadata } from '../../types/common/command-metadata.js'; import { DirectoryPath } from '../../types/file/directoryPath.js'; import { FilePath } from '../../types/file/filePath.js'; import { - GeneratedPluginResult, isInFlight, PluginGenerationInitiatedResponse, PluginGenerationStatus, @@ -45,7 +44,7 @@ export class PluginService { configDir: DirectoryPath, commandMetadata: CommandMetadata, authKey: string | null - ): Promise> { + ): Promise> { const authInfo: AuthInfo | null = await getAuthInfo(configDir.toString()); // `auth logout` blanks config.json rather than deleting it, so a logged-out user // still has a non-null AuthInfo with an empty key — check the key, not the object. @@ -67,12 +66,7 @@ export class PluginService { return err(completed.error); } - const download = await this.downloadPlugin(generationId, commandMetadata.shell, token); - if (download.isErr()) { - return err(download.error); - } - - return ok({ plugin: download.value, deferred: completed.value.deferred ?? [] }); + return await this.downloadPlugin(generationId, commandMetadata.shell, token); } private async initiateGeneration( @@ -95,7 +89,7 @@ export class PluginService { const id = (response.data as PluginGenerationInitiatedResponse | undefined)?.id; return id ? ok({ id }) : err(ServiceError.InvalidResponse); } catch (error) { - return err(mapProblemDetails(error) ?? handleServiceError(error)); + return err(mapRequestError(error)); } finally { buildFileStream.close(); } @@ -148,10 +142,11 @@ export class PluginService { return ok(response.data as NodeJS.ReadableStream); } catch (error) { // The body of a failed streamed response is itself a stream; leaving it open hangs the CLI. + // Discarding it also rules out reading ProblemDetails here, so only the status code maps. if (axios.isAxiosError(error)) { discardStreamBody(error.response?.data); } - return err(handleServiceError(error)); + return err(mapTransportError(error)); } } @@ -231,6 +226,23 @@ const formatValidationErrors = (errors: Record): string => { return 'One or more validation errors occurred.' + (messages.length ? '\n- ' + messages.join('\n- ') : ''); }; +/** + * `handleServiceError` maps a 401 onto the bare `Unauthorized access.`, which leaves the user + * with nothing to act on. An auth key can expire between any two calls here, so every one of + * them offers the same remedy the status poll already does. + */ +function mapTransportError(error: unknown): ServiceError { + if (axios.isAxiosError(error) && error.response?.status === 401) { + return ServiceError.unauthorizedWithHint(null); + } + return handleServiceError(error); +} + +/** For responses whose body is readable JSON; a streamed body can only be mapped by status. */ +function mapRequestError(error: unknown): ServiceError { + return mapProblemDetails(error) ?? mapTransportError(error); +} + /** * `handleServiceError` only reads ProblemDetails off the SDK's typed errors, so a raw axios * 400/403/404 would otherwise collapse into a generic server error and lose its message. diff --git a/src/prompts/plugin/generate.ts b/src/prompts/plugin/generate.ts index 7824e031..115bb33c 100644 --- a/src/prompts/plugin/generate.ts +++ b/src/prompts/plugin/generate.ts @@ -2,20 +2,13 @@ import { confirm, isCancel, log } from '@clack/prompts'; import { Result } from 'neverthrow'; import { ServiceError } from '../../infrastructure/service-error.js'; import { DirectoryPath } from '../../types/file/directoryPath.js'; -import { DeferralReason, DeferredLanguage, GeneratedPluginResult } from '../../types/plugin/generation-status.js'; import { format as f } from '../format.js'; import { noteWrapped, withSpinner } from '../prompt.js'; const PLUGIN_CONFIG_FILE = 'plugin-config.json'; -/** The wire carries the reason's name; the sentence that explains it lives here. */ -const DEFERRAL_DETAIL: Record = { - [DeferralReason.NoPluginGenerator]: 'has no context plugin generator', - [DeferralReason.TargetsV3]: 'targets v3, which this generator does not build' -}; - export class PluginGeneratePrompts { - public generatePlugin(fn: Promise>) { + public generatePlugin(fn: Promise>) { return withSpinner('Generating Context Plugin', 'Plugin generated successfully.', 'Plugin Generation failed.', fn); } @@ -62,13 +55,6 @@ export class PluginGeneratePrompts { log.error(message); } - public languagesDeferred(deferred: DeferredLanguage[]) { - const lines = deferred.map( - ({ language, reason }) => `Skipped ${f.var(language)} — ${DEFERRAL_DETAIL[reason] ?? 'is not supported yet'}.` - ); - log.info(lines.join('\n')); - } - public nextStepsPublishSdks() { const message = `Publish an SDK for each language you want in the plugin:\n` + diff --git a/src/types/plugin/generation-status.ts b/src/types/plugin/generation-status.ts index c6434d5c..ff12bff8 100644 --- a/src/types/plugin/generation-status.ts +++ b/src/types/plugin/generation-status.ts @@ -14,32 +14,15 @@ export enum PluginGenerationStatus { Unknown = 'Unknown' } -export enum DeferralReason { - TargetsV3 = 'TargetsV3', - NoPluginGenerator = 'NoPluginGenerator' -} - -/** A language named in the config that this generator skipped; generation still succeeds. */ -export interface DeferredLanguage { - language: string; - reason: DeferralReason | string; -} - export interface PluginGenerationStatusResponse { status: PluginGenerationStatus; errors?: Record; - deferred?: DeferredLanguage[]; } export interface PluginGenerationInitiatedResponse { id: string; } -export interface GeneratedPluginResult { - plugin: NodeJS.ReadableStream; - deferred: DeferredLanguage[]; -} - const IN_FLIGHT: ReadonlySet = new Set([ PluginGenerationStatus.Queued, PluginGenerationStatus.ExecutionStarted, diff --git a/test/actions/plugin/generate.test.ts b/test/actions/plugin/generate.test.ts index 271c91ae..7a6a814d 100644 --- a/test/actions/plugin/generate.test.ts +++ b/test/actions/plugin/generate.test.ts @@ -23,10 +23,8 @@ describe('PluginGenerateAction', () => { const execute = (force = false, zipPlugin = true) => action.execute(new DirectoryPath(buildDirectory), new DirectoryPath(pluginDirectory), force, zipPlugin); - const generated = (deferred: { language: string; reason: string }[] = []) => - sinon - .stub(PluginService.prototype, 'generatePlugin') - .resolves(ok({ plugin: Readable.from(['PK context-plugin']), deferred })); + const generated = () => + sinon.stub(PluginService.prototype, 'generatePlugin').resolves(ok(Readable.from(['PK context-plugin']))); beforeEach(async () => { tmpDirResult = await tmpDir({ unsafeCleanup: true }); @@ -113,25 +111,6 @@ describe('PluginGenerateAction', () => { expect(result.isSuccess()).to.be.true; expect(fsExtra.readFileSync(path.join(pluginDirectory, 'plugin.zip'), 'utf-8')).to.equal('PK context-plugin'); }); - - it('reports deferred languages without failing the run', async () => { - generated([{ language: 'python', reason: 'NoPluginGenerator' }]); - const languagesDeferred = sinon.stub(PluginGeneratePrompts.prototype, 'languagesDeferred'); - - const result = await execute(); - - expect(result.isSuccess()).to.be.true; - expect(languagesDeferred.firstCall.args[0]).to.deep.equal([{ language: 'python', reason: 'NoPluginGenerator' }]); - }); - - it('stays quiet about deferrals when there are none', async () => { - generated(); - const languagesDeferred = sinon.stub(PluginGeneratePrompts.prototype, 'languagesDeferred'); - - await execute(); - - expect(languagesDeferred.called).to.be.false; - }); }); describe('generation failures', () => { diff --git a/test/infrastructure/services/plugin-service.test.ts b/test/infrastructure/services/plugin-service.test.ts index 6319503e..a1222d5f 100644 --- a/test/infrastructure/services/plugin-service.test.ts +++ b/test/infrastructure/services/plugin-service.test.ts @@ -123,7 +123,7 @@ describe('PluginService', () => { const result = await generatePlugin(); expect(result.isOk()).to.be.true; - expect(await drain(result._unsafeUnwrap().plugin)).to.equal('PK context-plugin'); + expect(await drain(result._unsafeUnwrap())).to.equal('PK context-plugin'); expect(generateRequests).to.have.length(1); expect(statusRequests[0].url).to.equal(`/plugin/${GENERATION_ID}/status`); }); @@ -155,29 +155,6 @@ describe('PluginService', () => { expect(statusRequests).to.have.length(4); }); - it('reports the languages the backend deferred alongside the artifact', async () => { - respondToStatus = statusBody({ - status: 'Completed', - deferred: [ - { language: 'python', reason: 'NoPluginGenerator' }, - { language: 'java', reason: 'TargetsV3' } - ] - }); - - const result = await generatePlugin(); - - expect(result._unsafeUnwrap().deferred).to.deep.equal([ - { language: 'python', reason: 'NoPluginGenerator' }, - { language: 'java', reason: 'TargetsV3' } - ]); - }); - - it('reports no deferrals when the status omits them', async () => { - const result = await generatePlugin(); - - expect(result._unsafeUnwrap().deferred).to.deep.equal([]); - }); - it('refuses to call the API at all without a key', async () => { const result = await generatePlugin(null); @@ -296,7 +273,20 @@ describe('PluginService', () => { const error = errorFrom(await generatePlugin()); expect(error.code).to.equal(ServiceErrorCode.UnAuthorized); - expect(error.errorMessage).to.include('auth'); + // A bare 401 carries no body, so the remedy can only come from the CLI. + expect(error.errorMessage).to.equal(ServiceError.unauthorizedWithHint(null).errorMessage); + }); + + it('points an expired key at the login command when the download 401s', async () => { + respondToDownload = (res) => { + res.writeHead(401); + res.end(); + }; + + const error = errorFrom(await generatePlugin()); + + expect(error.code).to.equal(ServiceErrorCode.UnAuthorized); + expect(error.errorMessage).to.equal(ServiceError.unauthorizedWithHint(null).errorMessage); }); it('reports a generate response that carries no id', async () => { From fd818d979be0ddb05c9edfa4d9943bd4bc846fcd Mon Sep 17 00:00:00 2001 From: MuHamza30 Date: Tue, 11 Aug 2026 11:18:10 +0500 Subject: [PATCH 03/12] docs: document plugin generate and close conventions gaps Add the `apimatic plugin generate` entry to the README command reference, which shipped without it. Written by hand rather than via `pnpm readme`: `oclif readme` matches its markers with `\n`, so against this repo's CRLF README it appends a second copy of the reference instead of replacing the existing one, doubling the file. Use the `getConfigDir()` helper that `.ai/skills/command.md` lists on its review checklist, and lower-case the `--zip` description to match every other flag. Correct two comments that described behaviour the code does not have: `mapProblemDetails` handles 400 and 403, not 404, and the poll loop is only bounded between polls. The loop's comment now also records why it does not reuse `pollUntilCompleted` from `portal-service.ts` as `service.md` asks, and that deduping waits on the regenerated SDK. Co-Authored-By: Claude Opus 5 (1M context) --- README.md | 31 +++++++++++++++++++ src/commands/plugin/generate.ts | 8 +++-- src/infrastructure/services/plugin-service.ts | 14 ++++++--- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 08fef0d8..01a907a2 100644 --- a/README.md +++ b/README.md @@ -40,6 +40,7 @@ USAGE * [`apimatic auth status`](#apimatic-auth-status) * [`apimatic autocomplete [SHELL]`](#apimatic-autocomplete-shell) * [`apimatic help [COMMAND]`](#apimatic-help-command) +* [`apimatic plugin generate`](#apimatic-plugin-generate) * [`apimatic portal copilot`](#apimatic-portal-copilot) * [`apimatic portal generate`](#apimatic-portal-generate) * [`apimatic portal recipe new`](#apimatic-portal-recipe-new) @@ -222,6 +223,36 @@ DESCRIPTION _See code: [@oclif/plugin-help](https://github.com/oclif/plugin-help/blob/main/src/commands/help.ts)_ +## `apimatic plugin generate` + +Generate a Claude Code context plugin for your published SDKs. + +``` +USAGE + $ apimatic plugin generate [--zip] [-i ] [-d ] [-f] [-k ] + +FLAGS + -d, --destination= [default: /plugin] path where the plugin will be generated. + -f, --force overwrite changes without asking for user consent. + -i, --input= [default: ./] path to the parent directory containing the 'src' directory, which includes + API specifications and configuration files. + -k, --auth-key= override current authentication state with an authentication key. + --zip download the generated plugin as a .zip archive + +DESCRIPTION + Generate a Claude Code context plugin for your published SDKs. + + Generate a context plugin that teaches an AI coding assistant how to use your SDKs. Requires an input directory + containing a `src` directory with a `plugin-config.json`. + +EXAMPLES + apimatic plugin generate + + apimatic plugin generate --input="./" --destination="./plugin" +``` + +_See code: [src/commands/plugin/generate.ts](https://github.com/apimatic/apimatic-cli/blob/beta/src/commands/plugin/generate.ts)_ + ## `apimatic portal copilot` Configure API Copilot for your API Documentation portal diff --git a/src/commands/plugin/generate.ts b/src/commands/plugin/generate.ts index 4d551cd2..61a90665 100644 --- a/src/commands/plugin/generate.ts +++ b/src/commands/plugin/generate.ts @@ -21,7 +21,7 @@ export default class PluginGenerate extends Command { static readonly flags = { zip: Flags.boolean({ default: false, - description: 'Download the generated plugin as a .zip archive' + description: 'download the generated plugin as a .zip archive' }), ...FlagsProvider.input, ...FlagsProvider.destination('plugin', 'plugin'), @@ -43,8 +43,12 @@ export default class PluginGenerate extends Command { }; intro('Generate Context Plugin'); - const action = new PluginGenerateAction(new DirectoryPath(this.config.configDir), commandMetadata, authKey); + const action = new PluginGenerateAction(this.getConfigDir(), commandMetadata, authKey); const result = await action.execute(buildDirectory, pluginDirectory, force, zipPlugin); outro(result); } + + private readonly getConfigDir = () => { + return new DirectoryPath(this.config.configDir); + }; } diff --git a/src/infrastructure/services/plugin-service.ts b/src/infrastructure/services/plugin-service.ts index 7f53b86c..4b07296d 100644 --- a/src/infrastructure/services/plugin-service.ts +++ b/src/infrastructure/services/plugin-service.ts @@ -167,9 +167,13 @@ const UNKNOWN_STATUS_MESSAGE = 'Unable to determine generation status. Please tr type PollDecision = { kind: 'continue' } | { kind: 'done' } | { kind: 'error'; error: ServiceError }; /** - * Context plugin generation reports completion as a status body rather than a redirect, and - * unlike the portal and SDK endpoints it is bounded: a run that never reaches a terminal - * status gives up rather than polling forever. + * A second copy of the loop in `portal-service.ts`, which `.ai/skills/service.md` says to reuse. + * It can't be reused yet: this endpoint reports completion as a status body rather than a 302, + * and its status vocabulary is absent from the SDK's `Status` enum. Deduping the two is left to + * the follow-up that lands the regenerated SDK. + * + * Unlike the portal and SDK loops this one gives up rather than polling forever — though the + * deadline is only checked between polls, so a request that hangs still outlives it. */ async function pollUntilCompleted( pollIntervalMs: number, @@ -245,9 +249,9 @@ function mapRequestError(error: unknown): ServiceError { /** * `handleServiceError` only reads ProblemDetails off the SDK's typed errors, so a raw axios - * 400/403/404 would otherwise collapse into a generic server error and lose its message. + * 400 or 403 would otherwise collapse into a generic server error and lose its message. * Unlike the SDK path this also falls back to `detail`, which io uses when it sends no - * `errors` map. + * `errors` map. Every other status is left to `mapTransportError`. */ function mapProblemDetails(error: unknown): ServiceError | undefined { if (!axios.isAxiosError(error)) { From 7c793d835de0cd9b6d5d2590963c358540b74b89 Mon Sep 17 00:00:00 2001 From: MuHamza30 Date: Tue, 11 Aug 2026 13:29:17 +0500 Subject: [PATCH 04/12] fix: stop an empty error list hiding the real generation failure MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `getError` hands back the server's list verbatim and an empty array is truthy, so a `ValidationError` carrying `pluginConfig` with no messages rendered a headed but empty list. Being an if/else, it also skipped the branch that prints the full message the service had already assembled from every key — so the one line telling the user what to fix was discarded. Guard on `?.length`, as `formatSdkValidationError` already does for `sdkMergeFailed`. Inline the routing that picks between those messages. No other action carries a private method, and `portal/generate.ts` does the same job — inspect a `ServiceError` for a key, choose a prompt — inside `execute`. Co-Authored-By: Claude Opus 5 (1M context) --- src/actions/plugin/generate.ts | 32 +++++++++++----------------- test/actions/plugin/generate.test.ts | 16 ++++++++++++++ 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/src/actions/plugin/generate.ts b/src/actions/plugin/generate.ts index 98225ca5..41806584 100644 --- a/src/actions/plugin/generate.ts +++ b/src/actions/plugin/generate.ts @@ -1,6 +1,5 @@ import { withDirPath } from '../../infrastructure/tmp-extensions.js'; import { PluginService } from '../../infrastructure/services/plugin-service.js'; -import { ServiceError } from '../../infrastructure/service-error.js'; import { PluginGeneratePrompts } from '../../prompts/plugin/generate.js'; import { BuildContext } from '../../types/build-context.js'; import { CommandMetadata } from '../../types/common/command-metadata.js'; @@ -55,7 +54,19 @@ export class PluginGenerateAction { ); if (response.isErr()) { - this.reportGenerationError(response.error); + const error = response.error; + const pluginConfigErrors = error.getError('pluginConfig'); + const sdkRepoErrors = error.getError('sdkRepos'); + + if (pluginConfigErrors?.length) { + this.prompts.pluginConfigInvalid(pluginConfigErrors); + } else if (sdkRepoErrors?.length) { + this.prompts.noBuildableLanguages(sdkRepoErrors); + this.prompts.nextStepsPublishSdks(); + } else { + this.prompts.pluginGenerationError(error.errorMessage); + } + return ActionResult.failed(); } @@ -69,21 +80,4 @@ export class PluginGenerateAction { return ActionResult.success(); }); }; - - private reportGenerationError(error: ServiceError) { - const pluginConfigErrors = error.getError('pluginConfig'); - if (pluginConfigErrors) { - this.prompts.pluginConfigInvalid(pluginConfigErrors); - return; - } - - const sdkRepoErrors = error.getError('sdkRepos'); - if (sdkRepoErrors) { - this.prompts.noBuildableLanguages(sdkRepoErrors); - this.prompts.nextStepsPublishSdks(); - return; - } - - this.prompts.pluginGenerationError(error.errorMessage); - } } diff --git a/test/actions/plugin/generate.test.ts b/test/actions/plugin/generate.test.ts index 7a6a814d..f8190db8 100644 --- a/test/actions/plugin/generate.test.ts +++ b/test/actions/plugin/generate.test.ts @@ -143,5 +143,21 @@ describe('PluginGenerateAction', () => { expect((await execute()).isFailed()).to.be.true; expect(pluginGenerationError.called).to.be.true; }); + + it('ignores an error key that carries no messages', async () => { + // An empty array is truthy, so a bare `if (errors)` would print a headed but + // empty list and swallow the message the service already assembled. + sinon + .stub(PluginService.prototype, 'generatePlugin') + .resolves(err(ServiceError.badRequest('nothing buildable', { pluginConfig: [], sdkRepos: [] }))); + const pluginConfigInvalid = sinon.stub(PluginGeneratePrompts.prototype, 'pluginConfigInvalid'); + const noBuildableLanguages = sinon.stub(PluginGeneratePrompts.prototype, 'noBuildableLanguages'); + const pluginGenerationError = sinon.stub(PluginGeneratePrompts.prototype, 'pluginGenerationError'); + + expect((await execute()).isFailed()).to.be.true; + expect(pluginConfigInvalid.called).to.be.false; + expect(noBuildableLanguages.called).to.be.false; + expect(pluginGenerationError.firstCall.args[0]).to.equal('nothing buildable'); + }); }); }); From 12f390f7bbb6358fb785e294f9be78e3585bcd7d Mon Sep 17 00:00:00 2001 From: MuHamza30 Date: Tue, 11 Aug 2026 13:48:16 +0500 Subject: [PATCH 05/12] fix: report a failed plugin save instead of throwing to the command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ZipService.unArchive` throws on a payload that is not a zip, and on its own file-count and size guards — so the protection against a zip bomb surfaced as a raw stack trace rather than a message. Nothing between the context and oclif caught it, which `instructions.md:47` and `action.md:33` both forbid. Catch around the save and report it, the shape `portal/serve.ts:47` already uses for a throwing context. The reason is passed through so a user can tell an invalid archive from one that exceeded a limit. Co-Authored-By: Claude Opus 5 (1M context) --- src/actions/plugin/generate.ts | 9 +++++++-- src/prompts/plugin/generate.ts | 4 ++++ test/actions/plugin/generate.test.ts | 12 ++++++++++++ 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/src/actions/plugin/generate.ts b/src/actions/plugin/generate.ts index 41806584..79a0f564 100644 --- a/src/actions/plugin/generate.ts +++ b/src/actions/plugin/generate.ts @@ -70,8 +70,13 @@ export class PluginGenerateAction { return ActionResult.failed(); } - const tempPluginZipPath = await tempContext.save(response.value); - await pluginContext.save(tempPluginZipPath, zipPlugin); + try { + const tempPluginZipPath = await tempContext.save(response.value); + await pluginContext.save(tempPluginZipPath, zipPlugin); + } catch (error) { + this.prompts.pluginSaveFailed(error instanceof Error ? error.message : String(error)); + return ActionResult.failed(); + } if (displayMessages) { this.prompts.pluginGenerated(pluginDirectory); diff --git a/src/prompts/plugin/generate.ts b/src/prompts/plugin/generate.ts index 115bb33c..af72f5da 100644 --- a/src/prompts/plugin/generate.ts +++ b/src/prompts/plugin/generate.ts @@ -45,6 +45,10 @@ export class PluginGeneratePrompts { log.error(error); } + public pluginSaveFailed(reason: string) { + log.error(`The generated plugin could not be saved: ${reason}`); + } + public pluginConfigInvalid(messages: string[]) { const message = `Your ${f.var(PLUGIN_CONFIG_FILE)} is invalid:\n- ${messages.join('\n- ')}`; log.error(message); diff --git a/test/actions/plugin/generate.test.ts b/test/actions/plugin/generate.test.ts index f8190db8..1f066e22 100644 --- a/test/actions/plugin/generate.test.ts +++ b/test/actions/plugin/generate.test.ts @@ -10,6 +10,7 @@ import { PluginGeneratePrompts } from '../../../src/prompts/plugin/generate.js'; import { PluginService } from '../../../src/infrastructure/services/plugin-service.js'; import { ServiceError } from '../../../src/infrastructure/service-error.js'; import { DirectoryPath } from '../../../src/types/file/directoryPath.js'; +import { PluginContext } from '../../../src/types/plugin-context.js'; import { CommandMetadata } from '../../../src/types/common/command-metadata.js'; const COMMAND_METADATA: CommandMetadata = { commandName: 'plugin generate', shell: 'test' }; @@ -144,6 +145,17 @@ describe('PluginGenerateAction', () => { expect(pluginGenerationError.called).to.be.true; }); + it('reports a save failure instead of throwing to the command', async () => { + // `unArchive` throws on a payload that is not a zip, and on its own size and + // file-count guards. Actions never throw to the Command layer. + generated(); + sinon.stub(PluginContext.prototype, 'save').rejects(new Error('Invalid or unsupported zip format')); + const pluginSaveFailed = sinon.stub(PluginGeneratePrompts.prototype, 'pluginSaveFailed'); + + expect((await execute()).isFailed()).to.be.true; + expect(pluginSaveFailed.firstCall.args[0]).to.equal('Invalid or unsupported zip format'); + }); + it('ignores an error key that carries no messages', async () => { // An empty array is truthy, so a bare `if (errors)` would print a headed but // empty list and swallow the message the service already assembled. From a1830742b2734691782ad7b80f160a9a8f168589 Mon Sep 17 00:00:00 2001 From: MuHamza30 Date: Tue, 11 Aug 2026 15:29:48 +0500 Subject: [PATCH 06/12] fix: stop an Unknown status aborting a healthy generation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The orchestrator reports `Unknown` whenever it has not written a custom status yet, which includes the window right after a run starts, so treating it as terminal ended a generation that was about to succeed. Fall through to the next poll instead, as `portal-service.ts` does for any status it does not recognise, and let the deadline stop a genuinely stuck run. Reshape the loop to match that one while here: read the deadline after the status rather than before, so a run that finished during the last wait is not reported as a timeout, and derive the timeout message from the budget instead of hardcoding five minutes — the previous test asserted "after 5 minutes" against a 15ms budget. Move the axios ProblemDetails and 401 mappers into `service-error.ts`. `handleServiceError` reads bodies only off the SDK's typed errors, so every axios service loses 400 and 403 detail; the fix was reachable only from `plugin-service.ts` where it was written. Co-Authored-By: Claude Opus 5 (1M context) --- src/infrastructure/service-error.ts | 52 ++++++++ src/infrastructure/services/plugin-service.ts | 121 ++++-------------- src/types/plugin/generation-status.ts | 10 -- .../services/plugin-service.test.ts | 24 +++- 4 files changed, 93 insertions(+), 114 deletions(-) diff --git a/src/infrastructure/service-error.ts b/src/infrastructure/service-error.ts index 9ed5e1b0..b369a8d3 100644 --- a/src/infrastructure/service-error.ts +++ b/src/infrastructure/service-error.ts @@ -104,6 +104,58 @@ function mapApiError(error: ApiError): ServiceError { return ServiceError.ServerError; } +interface ProblemDetailsBody { + title?: string; + detail?: string; + errors?: Record; +} + +/** + * `handleServiceError` reads a ProblemDetails body only off the SDK's typed errors, so on a raw + * axios call a 400 or 403 would collapse into a generic server error and lose its message. + * Unlike the SDK path this also falls back to `detail`, which io uses when it sends no `errors` + * map. Every other status is left to `mapTransportError`. + */ +function mapAxiosProblemDetails(error: unknown): ServiceError | undefined { + if (!axios.isAxiosError(error)) { + return undefined; + } + + const body = error.response?.data as ProblemDetailsBody | undefined; + if (typeof body !== "object" || body === null) { + return undefined; + } + + const errors = body.errors ?? {}; + const firstMessage = Object.values(errors).flat()[0] ?? body.detail; + const title = body.title ?? "Request failed."; + const message = firstMessage ? `${title}\n- ${firstMessage}` : title; + + if (error.response?.status === 400) { + return ServiceError.badRequest(message, errors); + } + if (error.response?.status === 403) { + return ServiceError.forbidden(message); + } + return undefined; +} + +/** + * `handleServiceError` maps an axios 401 onto the bare `Unauthorized access.`, which leaves the + * user with nothing to act on — the SDK path names the remedy but the axios path does not. + */ +export function mapTransportError(error: unknown): ServiceError { + if (axios.isAxiosError(error) && error.response?.status === 401) { + return ServiceError.unauthorizedWithHint(null); + } + return handleServiceError(error); +} + +/** For responses whose body is readable JSON; a streamed body can only be mapped by status. */ +export function mapRequestError(error: unknown): ServiceError { + return mapAxiosProblemDetails(error) ?? mapTransportError(error); +} + export function handleServiceError(error: unknown): ServiceError { if (error instanceof ApiError) { // A `callAsStream` error body would otherwise hang the CLI. Order against diff --git a/src/infrastructure/services/plugin-service.ts b/src/infrastructure/services/plugin-service.ts index 4b07296d..f6aaa643 100644 --- a/src/infrastructure/services/plugin-service.ts +++ b/src/infrastructure/services/plugin-service.ts @@ -6,7 +6,6 @@ import { CommandMetadata } from '../../types/common/command-metadata.js'; import { DirectoryPath } from '../../types/file/directoryPath.js'; import { FilePath } from '../../types/file/filePath.js'; import { - isInFlight, PluginGenerationInitiatedResponse, PluginGenerationStatus, PluginGenerationStatusResponse @@ -14,17 +13,11 @@ import { import { discardStreamBody } from '../../utils/utils.js'; import { envInfo } from '../env-info.js'; import { FileService } from '../file-service.js'; -import { handleServiceError, ServiceError } from '../service-error.js'; +import { handleServiceError, mapRequestError, mapTransportError, ServiceError } from '../service-error.js'; const STATUS_POLL_INTERVAL_MS = 3000; const GENERATION_TIMEOUT_MS = 5 * 60 * 1000; -interface ProblemDetailsBody { - title?: string; - detail?: string; - errors?: Record; -} - export class PluginService { private readonly apiBaseUrl = 'https://api.apimatic.io' as const; private readonly fileService = new FileService(); @@ -161,118 +154,50 @@ export class PluginService { } } -const TIMED_OUT_MESSAGE = 'Plugin generation timed out after 5 minutes.'; -const UNKNOWN_STATUS_MESSAGE = 'Unable to determine generation status. Please try again.'; - -type PollDecision = { kind: 'continue' } | { kind: 'done' } | { kind: 'error'; error: ServiceError }; - -/** - * A second copy of the loop in `portal-service.ts`, which `.ai/skills/service.md` says to reuse. - * It can't be reused yet: this endpoint reports completion as a status body rather than a 302, - * and its status vocabulary is absent from the SDK's `Status` enum. Deduping the two is left to - * the follow-up that lands the regenerated SDK. - * - * Unlike the portal and SDK loops this one gives up rather than polling forever — though the - * deadline is only checked between polls, so a request that hangs still outlives it. - */ async function pollUntilCompleted( pollIntervalMs: number, timeoutMs: number, fetchStatus: () => Promise> ): Promise> { - const startedAt = Date.now(); + const deadline = Date.now() + timeoutMs; for (;;) { await new Promise((resolve) => setTimeout(resolve, pollIntervalMs)); - if (Date.now() - startedAt >= timeoutMs) { - return err(ServiceError.timeout(TIMED_OUT_MESSAGE)); - } - const statusResult = await fetchStatus(); if (statusResult.isErr()) { return err(statusResult.error); } - const decision = classifyPluginStatus(statusResult.value); - if (decision.kind === 'done') { + const { status, errors } = statusResult.value; + + if (status === PluginGenerationStatus.Completed) { return ok(statusResult.value); } - if (decision.kind === 'error') { - return err(decision.error); + if (status === PluginGenerationStatus.Failed) { + return err(ServiceError.ServerError); + } + if (status === PluginGenerationStatus.ValidationError) { + const validationErrors = errors ?? {}; + return err(ServiceError.badRequest(formatValidationErrors(validationErrors), validationErrors)); } - } -} -function classifyPluginStatus({ status, errors }: PluginGenerationStatusResponse): PollDecision { - if (isInFlight(status)) { - return { kind: 'continue' }; - } - if (status === PluginGenerationStatus.Completed) { - return { kind: 'done' }; - } - if (status === PluginGenerationStatus.Failed) { - return { kind: 'error', error: ServiceError.ServerError }; - } - if (status === PluginGenerationStatus.ValidationError) { - const validationErrors = errors ?? {}; - return { - kind: 'error', - error: ServiceError.badRequest(formatValidationErrors(validationErrors), validationErrors) - }; + // `Unknown` joins the in-flight values here rather than ending the run: the orchestrator + // reports it whenever it has not written a custom status yet, which includes the window + // right after a healthy run starts. The deadline is what stops a genuinely stuck one, and + // it is read after the status so a run that finished during the last wait still counts. + if (Date.now() >= deadline) { + return err(ServiceError.timeout(timedOutMessage(timeoutMs))); + } } - // `Unknown`, and anything a newer backend adds, ends the run rather than polling to the timeout. - return { kind: 'error', error: ServiceError.serverError(UNKNOWN_STATUS_MESSAGE) }; } +const timedOutMessage = (timeoutMs: number): string => { + const minutes = Math.round(timeoutMs / 60_000); + return minutes >= 1 ? `Plugin generation timed out after ${minutes} minutes.` : 'Plugin generation timed out.'; +}; + const formatValidationErrors = (errors: Record): string => { const messages = Object.values(errors).flat(); return 'One or more validation errors occurred.' + (messages.length ? '\n- ' + messages.join('\n- ') : ''); }; - -/** - * `handleServiceError` maps a 401 onto the bare `Unauthorized access.`, which leaves the user - * with nothing to act on. An auth key can expire between any two calls here, so every one of - * them offers the same remedy the status poll already does. - */ -function mapTransportError(error: unknown): ServiceError { - if (axios.isAxiosError(error) && error.response?.status === 401) { - return ServiceError.unauthorizedWithHint(null); - } - return handleServiceError(error); -} - -/** For responses whose body is readable JSON; a streamed body can only be mapped by status. */ -function mapRequestError(error: unknown): ServiceError { - return mapProblemDetails(error) ?? mapTransportError(error); -} - -/** - * `handleServiceError` only reads ProblemDetails off the SDK's typed errors, so a raw axios - * 400 or 403 would otherwise collapse into a generic server error and lose its message. - * Unlike the SDK path this also falls back to `detail`, which io uses when it sends no - * `errors` map. Every other status is left to `mapTransportError`. - */ -function mapProblemDetails(error: unknown): ServiceError | undefined { - if (!axios.isAxiosError(error)) { - return undefined; - } - - const body = error.response?.data as ProblemDetailsBody | undefined; - if (typeof body !== 'object' || body === null) { - return undefined; - } - - const errors = body.errors ?? {}; - const firstMessage = Object.values(errors).flat()[0] ?? body.detail; - const title = body.title ?? 'Request failed.'; - const message = firstMessage ? `${title}\n- ${firstMessage}` : title; - - if (error.response?.status === 400) { - return ServiceError.badRequest(message, errors); - } - if (error.response?.status === 403) { - return ServiceError.forbidden(message); - } - return undefined; -} diff --git a/src/types/plugin/generation-status.ts b/src/types/plugin/generation-status.ts index ff12bff8..b1895bae 100644 --- a/src/types/plugin/generation-status.ts +++ b/src/types/plugin/generation-status.ts @@ -22,13 +22,3 @@ export interface PluginGenerationStatusResponse { export interface PluginGenerationInitiatedResponse { id: string; } - -const IN_FLIGHT: ReadonlySet = new Set([ - PluginGenerationStatus.Queued, - PluginGenerationStatus.ExecutionStarted, - PluginGenerationStatus.GeneratingArtifacts -]); - -export function isInFlight(status: PluginGenerationStatus): boolean { - return IN_FLIGHT.has(status); -} diff --git a/test/infrastructure/services/plugin-service.test.ts b/test/infrastructure/services/plugin-service.test.ts index a1222d5f..8f7d9b72 100644 --- a/test/infrastructure/services/plugin-service.test.ts +++ b/test/infrastructure/services/plugin-service.test.ts @@ -203,13 +203,15 @@ describe('PluginService', () => { expect(errorFrom(await generatePlugin()).code).to.equal(ServiceErrorCode.ServerError); }); - it('stops on an Unknown status rather than polling to the timeout', async () => { - respondToStatus = statusBody({ status: 'Unknown' }); + it('keeps polling through an Unknown status', async () => { + // The orchestrator reports Unknown until it writes its first custom status, so a run + // polled in that window is healthy rather than broken. + respondToStatus = (res, attempt) => json(res, 200, { status: attempt === 1 ? 'Unknown' : 'Completed' }); - const error = errorFrom(await generatePlugin()); + const result = await generatePlugin(); - expect(error.errorMessage).to.equal('Unable to determine generation status. Please try again.'); - expect(statusRequests).to.have.length(1); + expect(result.isOk()).to.be.true; + expect(statusRequests).to.have.length(2); }); it('gives up once the generation budget is spent', async () => { @@ -219,7 +221,17 @@ describe('PluginService', () => { const result = await impatient.generatePlugin(buildPath, configDir, metadata, AUTH_KEY); expect(errorFrom(result).code).to.equal(ServiceErrorCode.Timeout); - expect(errorFrom(result).errorMessage).to.equal('Plugin generation timed out after 5 minutes.'); + expect(errorFrom(result).errorMessage).to.equal('Plugin generation timed out.'); + }); + + it('reads one last status before declaring a timeout', async () => { + // The budget is spent during the wait, but the run finished in that window. + const impatient = new PluginService(20, 10); + respondToStatus = statusBody({ status: 'Completed' }); + + const result = await impatient.generatePlugin(buildPath, configDir, metadata, AUTH_KEY); + + expect(result.isOk()).to.be.true; }); }); From c1817f6017484c5fd91b9877a4add9a0a55d55c3 Mon Sep 17 00:00:00 2001 From: MuHamza30 Date: Tue, 11 Aug 2026 15:50:26 +0500 Subject: [PATCH 07/12] fix: bound the requests the generation budget is measured against MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `axios.create` set no timeout, so a request that connected and then never answered never settled. The budget is only read once a status call returns, so it was never reached: the CLI sat on a spinner indefinitely. Removing the line added here does not merely fail the new test — the test process itself refuses to exit, which is the same event loop that keeps the command alive. `src/config/axios-config.ts` already held the value and was imported nowhere; it now exports it. The size caps stay unused: there is no client-side upload limit today and adding one is a separate decision. Co-Authored-By: Claude Opus 5 (1M context) --- src/config/axios-config.ts | 3 +++ src/infrastructure/services/plugin-service.ts | 9 ++++++++- test/infrastructure/services/plugin-service.test.ts | 11 +++++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/config/axios-config.ts b/src/config/axios-config.ts index 97f258c6..b0cd399f 100644 --- a/src/config/axios-config.ts +++ b/src/config/axios-config.ts @@ -3,6 +3,9 @@ import axios from "axios"; const fiftyMBsInBytes = 50 * 1024 * 1024; const fiveMinutesInMilliseconds = 5 * 60 * 1000; +/** For services that build their own instance and so cannot use the one below. */ +export const REQUEST_TIMEOUT_MS = fiveMinutesInMilliseconds; + const axiosInstance = axios.create({ maxContentLength: fiftyMBsInBytes, maxBodyLength: fiftyMBsInBytes, diff --git a/src/infrastructure/services/plugin-service.ts b/src/infrastructure/services/plugin-service.ts index f6aaa643..be838d59 100644 --- a/src/infrastructure/services/plugin-service.ts +++ b/src/infrastructure/services/plugin-service.ts @@ -2,6 +2,7 @@ import axios from 'axios'; import FormData from 'form-data'; import { err, ok, Result } from 'neverthrow'; import { AuthInfo, getAuthInfo } from '../../client-utils/auth-manager.js'; +import { REQUEST_TIMEOUT_MS } from '../../config/axios-config.js'; import { CommandMetadata } from '../../types/common/command-metadata.js'; import { DirectoryPath } from '../../types/file/directoryPath.js'; import { FilePath } from '../../types/file/filePath.js'; @@ -23,13 +24,16 @@ export class PluginService { private readonly fileService = new FileService(); private readonly statusPollIntervalMs: number; private readonly generationTimeoutMs: number; + private readonly requestTimeoutMs: number; constructor( statusPollIntervalMs: number = STATUS_POLL_INTERVAL_MS, - generationTimeoutMs: number = GENERATION_TIMEOUT_MS + generationTimeoutMs: number = GENERATION_TIMEOUT_MS, + requestTimeoutMs: number = REQUEST_TIMEOUT_MS ) { this.statusPollIntervalMs = statusPollIntervalMs; this.generationTimeoutMs = generationTimeoutMs; + this.requestTimeoutMs = requestTimeoutMs; } public async generatePlugin( @@ -146,6 +150,9 @@ export class PluginService { private axiosInstance(shell: string, apiKey: string) { return axios.create({ baseURL: envInfo.getBaseUrl() ?? this.apiBaseUrl, + // The generation budget is only read between polls, so a request that connects and + // then never answers would outlive it. Bounding the request is what makes it a limit. + timeout: this.requestTimeoutMs, headers: { 'User-Agent': envInfo.getUserAgent(shell), Authorization: `X-Auth-Key ${apiKey}` diff --git a/test/infrastructure/services/plugin-service.test.ts b/test/infrastructure/services/plugin-service.test.ts index 8f7d9b72..c8605326 100644 --- a/test/infrastructure/services/plugin-service.test.ts +++ b/test/infrastructure/services/plugin-service.test.ts @@ -224,6 +224,17 @@ describe('PluginService', () => { expect(errorFrom(result).errorMessage).to.equal('Plugin generation timed out.'); }); + it('gives up on a status request that never answers', async () => { + // Without a request timeout this hangs rather than fails: the generation budget is + // only read once a status call returns, and this one never does. + const impatient = new PluginService(1, 5_000, 30); + respondToStatus = () => {}; + + const result = await impatient.generatePlugin(buildPath, configDir, metadata, AUTH_KEY); + + expect(errorFrom(result).code).to.equal(ServiceErrorCode.NetworkError); + }); + it('reads one last status before declaring a timeout', async () => { // The budget is spent during the wait, but the run finished in that window. const impatient = new PluginService(20, 10); From 73e65ac3557217280c1251ecce3fa0c2f2fea282 Mon Sep 17 00:00:00 2001 From: MuHamza30 Date: Tue, 11 Aug 2026 17:41:24 +0500 Subject: [PATCH 08/12] fix: report every validation error key, not just the first MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `ValidationError` can carry `pluginConfig` and `sdkRepos` together, but the routing treated them as alternatives, so the second was dropped along with its next-steps note — and `errorMessage`, which already held every message, was never printed either. The user paid a second upload and generation to learn what the first response had told us. Delete `ServiceError.serverError`: it was added for the `Unknown` branch and has had no caller since that branch went. Note this leaves a `Failed` status still discarding the reasons the backend attaches to it. Cover the default `--zip=false` path, which every ordinary run takes and no test exercised, and record download requests in the fake server so the test claiming to authenticate them can actually check. Rename the download-failure test to what it asserts — the error code, not the stream cleanup its name promised. Co-Authored-By: Claude Opus 5 (1M context) --- src/actions/plugin/generate.ts | 8 +++- src/infrastructure/service-error.ts | 3 -- src/infrastructure/services/plugin-service.ts | 7 ++- test/actions/plugin/generate.test.ts | 21 +++++++++ .../services/plugin-service.test.ts | 6 ++- test/types/plugin-context.test.ts | 43 +++++++++++++++++++ 6 files changed, 80 insertions(+), 8 deletions(-) diff --git a/src/actions/plugin/generate.ts b/src/actions/plugin/generate.ts index 79a0f564..d865b570 100644 --- a/src/actions/plugin/generate.ts +++ b/src/actions/plugin/generate.ts @@ -58,12 +58,16 @@ export class PluginGenerateAction { const pluginConfigErrors = error.getError('pluginConfig'); const sdkRepoErrors = error.getError('sdkRepos'); + // One response can carry both keys, so these are not alternatives: reporting only + // the first costs the user a second upload and generation to learn the rest. if (pluginConfigErrors?.length) { this.prompts.pluginConfigInvalid(pluginConfigErrors); - } else if (sdkRepoErrors?.length) { + } + if (sdkRepoErrors?.length) { this.prompts.noBuildableLanguages(sdkRepoErrors); this.prompts.nextStepsPublishSdks(); - } else { + } + if (!pluginConfigErrors?.length && !sdkRepoErrors?.length) { this.prompts.pluginGenerationError(error.errorMessage); } diff --git a/src/infrastructure/service-error.ts b/src/infrastructure/service-error.ts index b369a8d3..2ee6789a 100644 --- a/src/infrastructure/service-error.ts +++ b/src/infrastructure/service-error.ts @@ -36,9 +36,6 @@ export class ServiceError { static timeout(customMessage: string): ServiceError { return new ServiceError(ServiceErrorCode.Timeout, customMessage, {}); } - static serverError(customMessage: string): ServiceError { - return new ServiceError(ServiceErrorCode.ServerError, customMessage, {}); - } static unauthorizedWithHint(apiMessage: string | null): ServiceError { // Both remedies name the full `auth login` command: the key is supplied to // that command, not to whichever one hit the 401 — most of them don't accept diff --git a/src/infrastructure/services/plugin-service.ts b/src/infrastructure/services/plugin-service.ts index be838d59..126542bc 100644 --- a/src/infrastructure/services/plugin-service.ts +++ b/src/infrastructure/services/plugin-service.ts @@ -200,8 +200,11 @@ async function pollUntilCompleted( } const timedOutMessage = (timeoutMs: number): string => { - const minutes = Math.round(timeoutMs / 60_000); - return minutes >= 1 ? `Plugin generation timed out after ${minutes} minutes.` : 'Plugin generation timed out.'; + const minutes = Math.floor(timeoutMs / 60_000); + if (minutes < 1) { + return 'Plugin generation timed out.'; + } + return `Plugin generation timed out after ${minutes} ${minutes === 1 ? 'minute' : 'minutes'}.`; }; const formatValidationErrors = (errors: Record): string => { diff --git a/test/actions/plugin/generate.test.ts b/test/actions/plugin/generate.test.ts index 1f066e22..0e8f8703 100644 --- a/test/actions/plugin/generate.test.ts +++ b/test/actions/plugin/generate.test.ts @@ -156,6 +156,27 @@ describe('PluginGenerateAction', () => { expect(pluginSaveFailed.firstCall.args[0]).to.equal('Invalid or unsupported zip format'); }); + it('reports every error key the response carries, not just the first', async () => { + // Both arrive in one response; showing only one costs the user a second upload + // and generation to discover the other. + sinon.stub(PluginService.prototype, 'generatePlugin').resolves( + err( + ServiceError.badRequest('invalid', { + pluginConfig: ["'pluginId' must be kebab-case."], + sdkRepos: ['nothing buildable'] + }) + ) + ); + const pluginConfigInvalid = sinon.stub(PluginGeneratePrompts.prototype, 'pluginConfigInvalid'); + const noBuildableLanguages = sinon.stub(PluginGeneratePrompts.prototype, 'noBuildableLanguages'); + const nextSteps = sinon.stub(PluginGeneratePrompts.prototype, 'nextStepsPublishSdks'); + + expect((await execute()).isFailed()).to.be.true; + expect(pluginConfigInvalid.firstCall.args[0]).to.deep.equal(["'pluginId' must be kebab-case."]); + expect(noBuildableLanguages.firstCall.args[0]).to.deep.equal(['nothing buildable']); + expect(nextSteps.called).to.be.true; + }); + it('ignores an error key that carries no messages', async () => { // An empty array is truthy, so a bare `if (errors)` would print a headed but // empty list and swallow the message the service already assembled. diff --git a/test/infrastructure/services/plugin-service.test.ts b/test/infrastructure/services/plugin-service.test.ts index c8605326..5d3c20f6 100644 --- a/test/infrastructure/services/plugin-service.test.ts +++ b/test/infrastructure/services/plugin-service.test.ts @@ -28,6 +28,7 @@ describe('PluginService', () => { const generateRequests: { url: string; headers: http.IncomingHttpHeaders; body: string }[] = []; const statusRequests: { url: string; headers: http.IncomingHttpHeaders }[] = []; + const downloadRequests: { url: string; headers: http.IncomingHttpHeaders }[] = []; const json = (res: http.ServerResponse, statusCode: number, body: unknown) => { res.writeHead(statusCode, { 'Content-Type': 'application/json' }); @@ -72,6 +73,7 @@ describe('PluginService', () => { } if (url.endsWith('/download')) { + downloadRequests.push({ url, headers: req.headers }); respondToDownload(res); return; } @@ -106,6 +108,7 @@ describe('PluginService', () => { beforeEach(() => { generateRequests.length = 0; statusRequests.length = 0; + downloadRequests.length = 0; respondToGenerate = (res) => json(res, 202, { id: GENERATION_ID }); respondToStatus = statusBody({ status: 'Completed' }); respondToDownload = (res) => { @@ -143,6 +146,7 @@ describe('PluginService', () => { await generatePlugin(); expect(statusRequests[0].headers.authorization).to.equal(`X-Auth-Key ${AUTH_KEY}`); + expect(downloadRequests[0].headers.authorization).to.equal(`X-Auth-Key ${AUTH_KEY}`); }); it('keeps polling through every in-flight status', async () => { @@ -328,7 +332,7 @@ describe('PluginService', () => { expect(statusRequests).to.have.length(1); }); - it('reports a failed download without hanging on its body', async () => { + it('maps a failed download onto a server error', async () => { respondToDownload = (res) => { res.writeHead(500, { 'Content-Type': 'application/json' }); res.end(JSON.stringify({ title: 'boom' })); diff --git a/test/types/plugin-context.test.ts b/test/types/plugin-context.test.ts index 5b8294f4..6874a6c6 100644 --- a/test/types/plugin-context.test.ts +++ b/test/types/plugin-context.test.ts @@ -1,7 +1,9 @@ import fs from 'fs'; +import os from 'os'; import path from 'path'; import mockFs from 'mock-fs'; import { expect } from 'chai'; +import { ZipService } from '../../src/infrastructure/zip-service'; import { PluginContext } from '../../src/types/plugin-context'; import { DirectoryPath } from '../../src/types/file/directoryPath'; import { FileName } from '../../src/types/file/fileName'; @@ -60,4 +62,45 @@ describe('PluginContext', () => { expect(fs.existsSync(path.join(pluginDirectory.toString(), 'plugin.zip'))).to.be.true; }); }); + + // The default `--zip=false` path, which every ordinary run takes. Real files rather than + // mock-fs: adm-zip reads the archive itself, so the bytes have to be a genuine zip. + describe('save, expanding the archive', () => { + let workDir: string; + let archive: FilePath; + let destination: DirectoryPath; + + beforeEach(async () => { + workDir = fs.mkdtempSync(path.join(os.tmpdir(), 'plugin-context-')); + const source = new DirectoryPath(path.join(workDir, 'source')); + fs.mkdirSync(path.join(source.toString(), 'skills'), { recursive: true }); + fs.writeFileSync(path.join(source.toString(), 'README.md'), '# plugin'); + fs.writeFileSync(path.join(source.toString(), 'skills', 'SKILL.md'), '# skill'); + + archive = new FilePath(new DirectoryPath(workDir), new FileName('plugin.zip')); + await new ZipService().archive(source, archive); + + destination = new DirectoryPath(path.join(workDir, 'destination')); + }); + + afterEach(() => fs.rmSync(workDir, { recursive: true, force: true })); + + it('expands the archive into the destination', async () => { + await new PluginContext(destination).save(archive, false); + + expect(fs.readFileSync(path.join(destination.toString(), 'README.md'), 'utf-8')).to.equal('# plugin'); + expect(fs.readFileSync(path.join(destination.toString(), 'skills', 'SKILL.md'), 'utf-8')).to.equal('# skill'); + expect(fs.existsSync(path.join(destination.toString(), 'plugin.zip'))).to.be.false; + }); + + it('clears a previous run before expanding', async () => { + fs.mkdirSync(destination.toString(), { recursive: true }); + fs.writeFileSync(path.join(destination.toString(), 'stale.md'), 'from a language that is gone'); + + await new PluginContext(destination).save(archive, false); + + expect(fs.existsSync(path.join(destination.toString(), 'stale.md'))).to.be.false; + expect(fs.existsSync(path.join(destination.toString(), 'README.md'))).to.be.true; + }); + }); }); From b6aae2f62eb4d386403f994708d4dcdd9d7d96f9 Mon Sep 17 00:00:00 2001 From: MuHamza30 Date: Thu, 13 Aug 2026 09:20:34 +0500 Subject: [PATCH 09/12] refactor: let the prompts layer choose the generation failure message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The action was reading `getError('pluginConfig')` and `getError('sdkRepos')` to decide which message to show — knowing the shape of an error payload in order to pick wording. Hand the whole `ServiceError` to one prompt, as `sdk generate` does, and let the prompts class hold the variations. Portal branches in its action only because its error is a union and it needs `instanceof` to tell a `ServiceError` from a stream; ours is not. Drop the guard around the save so it reads like portal's two lines, and with it the `pluginSaveFailed` prompt that then had no caller. Note this restores the raw stack trace a payload that is not a zip produces. Co-Authored-By: Claude Opus 5 (1M context) --- src/actions/plugin/generate.ts | 27 +++------------------------ src/prompts/plugin/generate.ts | 22 ++++++++++++++++++++-- test/actions/plugin/generate.test.ts | 12 ------------ 3 files changed, 23 insertions(+), 38 deletions(-) diff --git a/src/actions/plugin/generate.ts b/src/actions/plugin/generate.ts index d865b570..52df8764 100644 --- a/src/actions/plugin/generate.ts +++ b/src/actions/plugin/generate.ts @@ -54,33 +54,12 @@ export class PluginGenerateAction { ); if (response.isErr()) { - const error = response.error; - const pluginConfigErrors = error.getError('pluginConfig'); - const sdkRepoErrors = error.getError('sdkRepos'); - - // One response can carry both keys, so these are not alternatives: reporting only - // the first costs the user a second upload and generation to learn the rest. - if (pluginConfigErrors?.length) { - this.prompts.pluginConfigInvalid(pluginConfigErrors); - } - if (sdkRepoErrors?.length) { - this.prompts.noBuildableLanguages(sdkRepoErrors); - this.prompts.nextStepsPublishSdks(); - } - if (!pluginConfigErrors?.length && !sdkRepoErrors?.length) { - this.prompts.pluginGenerationError(error.errorMessage); - } - + this.prompts.pluginGenerationServiceError(response.error); return ActionResult.failed(); } - try { - const tempPluginZipPath = await tempContext.save(response.value); - await pluginContext.save(tempPluginZipPath, zipPlugin); - } catch (error) { - this.prompts.pluginSaveFailed(error instanceof Error ? error.message : String(error)); - return ActionResult.failed(); - } + const tempPluginZipPath = await tempContext.save(response.value); + await pluginContext.save(tempPluginZipPath, zipPlugin); if (displayMessages) { this.prompts.pluginGenerated(pluginDirectory); diff --git a/src/prompts/plugin/generate.ts b/src/prompts/plugin/generate.ts index af72f5da..5d5cd8ed 100644 --- a/src/prompts/plugin/generate.ts +++ b/src/prompts/plugin/generate.ts @@ -45,8 +45,26 @@ export class PluginGeneratePrompts { log.error(error); } - public pluginSaveFailed(reason: string) { - log.error(`The generated plugin could not be saved: ${reason}`); + /** + * One response can carry several error keys, so these are reported together rather than + * as alternatives: showing only the first costs a second upload and generation to learn + * the rest. Keys the backend adds later fall through to the assembled message, which + * already lists every one of them. + */ + public pluginGenerationServiceError(error: ServiceError) { + const pluginConfigErrors = error.getError('pluginConfig'); + const sdkRepoErrors = error.getError('sdkRepos'); + + if (pluginConfigErrors?.length) { + this.pluginConfigInvalid(pluginConfigErrors); + } + if (sdkRepoErrors?.length) { + this.noBuildableLanguages(sdkRepoErrors); + this.nextStepsPublishSdks(); + } + if (!pluginConfigErrors?.length && !sdkRepoErrors?.length) { + this.pluginGenerationError(error.errorMessage); + } } public pluginConfigInvalid(messages: string[]) { diff --git a/test/actions/plugin/generate.test.ts b/test/actions/plugin/generate.test.ts index 0e8f8703..2cf70082 100644 --- a/test/actions/plugin/generate.test.ts +++ b/test/actions/plugin/generate.test.ts @@ -10,7 +10,6 @@ import { PluginGeneratePrompts } from '../../../src/prompts/plugin/generate.js'; import { PluginService } from '../../../src/infrastructure/services/plugin-service.js'; import { ServiceError } from '../../../src/infrastructure/service-error.js'; import { DirectoryPath } from '../../../src/types/file/directoryPath.js'; -import { PluginContext } from '../../../src/types/plugin-context.js'; import { CommandMetadata } from '../../../src/types/common/command-metadata.js'; const COMMAND_METADATA: CommandMetadata = { commandName: 'plugin generate', shell: 'test' }; @@ -145,17 +144,6 @@ describe('PluginGenerateAction', () => { expect(pluginGenerationError.called).to.be.true; }); - it('reports a save failure instead of throwing to the command', async () => { - // `unArchive` throws on a payload that is not a zip, and on its own size and - // file-count guards. Actions never throw to the Command layer. - generated(); - sinon.stub(PluginContext.prototype, 'save').rejects(new Error('Invalid or unsupported zip format')); - const pluginSaveFailed = sinon.stub(PluginGeneratePrompts.prototype, 'pluginSaveFailed'); - - expect((await execute()).isFailed()).to.be.true; - expect(pluginSaveFailed.firstCall.args[0]).to.equal('Invalid or unsupported zip format'); - }); - it('reports every error key the response carries, not just the first', async () => { // Both arrive in one response; showing only one costs the user a second upload // and generation to discover the other. From e486c4783d87a49e714dbd41e6146c4a0bd68dcc Mon Sep 17 00:00:00 2001 From: saeedjamshaid Date: Thu, 13 Aug 2026 15:18:13 +0500 Subject: [PATCH 10/12] refactor: report plugin generation failures from the assembled message MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `pluginGenerationServiceError` inspected the error for `pluginConfig` and `sdkRepos` and picked wording per key. Any other key was reported by neither the key branches nor the fallback, which only ran when both were absent — so a response carrying a key the CLI did not know about lost those messages entirely, and the comment above the method claimed the opposite. The service already assembles one message from every key, so logging `errorMessage` reports all of them and no branch can suppress any. The per-key prompts and their tests go with it, and the surviving test asserts that a key the CLI does not recognise still reaches the user. `nextStepsPublishSdks` is kept but has no caller: it names the `apimatic sdk publish` remedy, which is the one thing the server's message cannot supply, and wants a trigger that does not reintroduce the branching. Co-Authored-By: Claude Opus 5 (1M context) --- src/actions/plugin/generate.ts | 2 +- src/prompts/plugin/generate.ts | 44 +------------------ test/actions/plugin/generate.test.ts | 63 ++++------------------------ 3 files changed, 11 insertions(+), 98 deletions(-) diff --git a/src/actions/plugin/generate.ts b/src/actions/plugin/generate.ts index 52df8764..bae9c876 100644 --- a/src/actions/plugin/generate.ts +++ b/src/actions/plugin/generate.ts @@ -54,7 +54,7 @@ export class PluginGenerateAction { ); if (response.isErr()) { - this.prompts.pluginGenerationServiceError(response.error); + this.prompts.pluginGenerationError(response.error.errorMessage); return ActionResult.failed(); } diff --git a/src/prompts/plugin/generate.ts b/src/prompts/plugin/generate.ts index 5d5cd8ed..a1572fc7 100644 --- a/src/prompts/plugin/generate.ts +++ b/src/prompts/plugin/generate.ts @@ -3,9 +3,7 @@ import { Result } from 'neverthrow'; import { ServiceError } from '../../infrastructure/service-error.js'; import { DirectoryPath } from '../../types/file/directoryPath.js'; import { format as f } from '../format.js'; -import { noteWrapped, withSpinner } from '../prompt.js'; - -const PLUGIN_CONFIG_FILE = 'plugin-config.json'; +import { withSpinner } from '../prompt.js'; export class PluginGeneratePrompts { public generatePlugin(fn: Promise>) { @@ -45,46 +43,6 @@ export class PluginGeneratePrompts { log.error(error); } - /** - * One response can carry several error keys, so these are reported together rather than - * as alternatives: showing only the first costs a second upload and generation to learn - * the rest. Keys the backend adds later fall through to the assembled message, which - * already lists every one of them. - */ - public pluginGenerationServiceError(error: ServiceError) { - const pluginConfigErrors = error.getError('pluginConfig'); - const sdkRepoErrors = error.getError('sdkRepos'); - - if (pluginConfigErrors?.length) { - this.pluginConfigInvalid(pluginConfigErrors); - } - if (sdkRepoErrors?.length) { - this.noBuildableLanguages(sdkRepoErrors); - this.nextStepsPublishSdks(); - } - if (!pluginConfigErrors?.length && !sdkRepoErrors?.length) { - this.pluginGenerationError(error.errorMessage); - } - } - - public pluginConfigInvalid(messages: string[]) { - const message = `Your ${f.var(PLUGIN_CONFIG_FILE)} is invalid:\n- ${messages.join('\n- ')}`; - log.error(message); - } - - public noBuildableLanguages(messages: string[]) { - const message = `No language in ${f.var('sdkRepos')} can be built yet:\n- ${messages.join('\n- ')}`; - log.error(message); - } - - public nextStepsPublishSdks() { - const message = - `Publish an SDK for each language you want in the plugin:\n` + - `'${f.cmdAlt('apimatic', 'sdk', 'publish')} ${f.flag('language', '')}'\n` + - `Then run '${f.cmdAlt('apimatic', 'plugin', 'generate')}'.`; - noteWrapped(message, 'Next Steps'); - } - public pluginGenerated(plugin: DirectoryPath) { log.info(`Plugin artifacts can be found at ${f.path(plugin)}.`); } diff --git a/test/actions/plugin/generate.test.ts b/test/actions/plugin/generate.test.ts index 2cf70082..592424b1 100644 --- a/test/actions/plugin/generate.test.ts +++ b/test/actions/plugin/generate.test.ts @@ -114,28 +114,6 @@ describe('PluginGenerateAction', () => { }); describe('generation failures', () => { - it('routes plugin-config validation errors to their own message', async () => { - sinon - .stub(PluginService.prototype, 'generatePlugin') - .resolves(err(ServiceError.badRequest('invalid', { pluginConfig: ["'pluginId' must be kebab-case."] }))); - const pluginConfigInvalid = sinon.stub(PluginGeneratePrompts.prototype, 'pluginConfigInvalid'); - - expect((await execute()).isFailed()).to.be.true; - expect(pluginConfigInvalid.firstCall.args[0]).to.deep.equal(["'pluginId' must be kebab-case."]); - }); - - it('routes sdkRepos validation errors to the next-steps message', async () => { - sinon - .stub(PluginService.prototype, 'generatePlugin') - .resolves(err(ServiceError.badRequest('invalid', { sdkRepos: ['nothing buildable'] }))); - const noBuildableLanguages = sinon.stub(PluginGeneratePrompts.prototype, 'noBuildableLanguages'); - const nextSteps = sinon.stub(PluginGeneratePrompts.prototype, 'nextStepsPublishSdks'); - - expect((await execute()).isFailed()).to.be.true; - expect(noBuildableLanguages.firstCall.args[0]).to.deep.equal(['nothing buildable']); - expect(nextSteps.called).to.be.true; - }); - it('falls back to the plain service message for any other failure', async () => { sinon.stub(PluginService.prototype, 'generatePlugin').resolves(err(ServiceError.ServerError)); const pluginGenerationError = sinon.stub(PluginGeneratePrompts.prototype, 'pluginGenerationError'); @@ -144,41 +122,18 @@ describe('PluginGenerateAction', () => { expect(pluginGenerationError.called).to.be.true; }); - it('reports every error key the response carries, not just the first', async () => { - // Both arrive in one response; showing only one costs the user a second upload - // and generation to discover the other. - sinon.stub(PluginService.prototype, 'generatePlugin').resolves( - err( - ServiceError.badRequest('invalid', { - pluginConfig: ["'pluginId' must be kebab-case."], - sdkRepos: ['nothing buildable'] - }) - ) - ); - const pluginConfigInvalid = sinon.stub(PluginGeneratePrompts.prototype, 'pluginConfigInvalid'); - const noBuildableLanguages = sinon.stub(PluginGeneratePrompts.prototype, 'noBuildableLanguages'); - const nextSteps = sinon.stub(PluginGeneratePrompts.prototype, 'nextStepsPublishSdks'); - - expect((await execute()).isFailed()).to.be.true; - expect(pluginConfigInvalid.firstCall.args[0]).to.deep.equal(["'pluginId' must be kebab-case."]); - expect(noBuildableLanguages.firstCall.args[0]).to.deep.equal(['nothing buildable']); - expect(nextSteps.called).to.be.true; - }); - - it('ignores an error key that carries no messages', async () => { - // An empty array is truthy, so a bare `if (errors)` would print a headed but - // empty list and swallow the message the service already assembled. - sinon - .stub(PluginService.prototype, 'generatePlugin') - .resolves(err(ServiceError.badRequest('nothing buildable', { pluginConfig: [], sdkRepos: [] }))); - const pluginConfigInvalid = sinon.stub(PluginGeneratePrompts.prototype, 'pluginConfigInvalid'); - const noBuildableLanguages = sinon.stub(PluginGeneratePrompts.prototype, 'noBuildableLanguages'); + it('reports every message the response carries, whatever key it arrived under', async () => { + // The service assembles one message from every key, so no key can be dropped for + // being one the CLI does not recognise. + const error = ServiceError.badRequest('One or more validation errors occurred.\n- a\n- b', { + pluginConfig: ['a'], + someKeyTheCliDoesNotKnow: ['b'] + }); + sinon.stub(PluginService.prototype, 'generatePlugin').resolves(err(error)); const pluginGenerationError = sinon.stub(PluginGeneratePrompts.prototype, 'pluginGenerationError'); expect((await execute()).isFailed()).to.be.true; - expect(pluginConfigInvalid.called).to.be.false; - expect(noBuildableLanguages.called).to.be.false; - expect(pluginGenerationError.firstCall.args[0]).to.equal('nothing buildable'); + expect(pluginGenerationError.firstCall.args[0]).to.equal('One or more validation errors occurred.\n- a\n- b'); }); }); }); From c6ce25209f681d881c0cc3324d004020a97ba16d Mon Sep 17 00:00:00 2001 From: saeedjamshaid Date: Thu, 13 Aug 2026 15:18:41 +0500 Subject: [PATCH 11/12] fix: complete a generation on the status redirect and bound every poll MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The plugin status endpoint signals a finished run with a `302` to the download location, exactly as `/portal/v2` does — it never sends a `Completed` status body. `PluginService` waited for that body and mapped the redirect to `InvalidResponse`, so every successful generation ended on "An unexpected error occurred". The spec added in apimatic/apimatic-docs#829 documents the redirect for both endpoints, and `ApiService` already handled it for portal. `ApiService.getGenerationStatus()` took the endpoint as a parameter, so the four flows shared one status read and would have accumulated per-endpoint branches as they diverge. Each flow now owns its own: portal, SDK and V4 SDK move onto `PortalService`, which used `ApiService` for nothing else and carries the axios plumbing that comes with them. `ApiService` is back to account and telemetry, and `GenerationStatusEndpoint` is gone. The poll loop existed twice — portal's polled a stuck generation forever, plugin's carried a deadline that never reached the other three. One copy now lives in `src/infrastructure/generation-status-poller.ts`, takes a budget from every caller, and keeps the invariant the plugin copy established: the deadline is read after the status, so a run that finished during the last wait is not reported as a timeout, and an unrecognised status keeps polling. Timeouts are now one value each: a 3s poll interval and a 30 min generation budget beside the loop, and a single 4 min request timeout in the axios config. `PortalService` had no request timeout at all, so its budget could not fire on a poll that connected and never answered. Note the budget is still only read between polls, which leaves the effective ceiling at 30 min plus one request timeout rather than 30 min flat. Co-Authored-By: Claude Opus 5 (1M context) --- .ai/skills/service.md | 7 +- src/config/axios-config.ts | 12 +- .../generation-status-poller.ts | 92 ++++++ src/infrastructure/services/api-service.ts | 55 ---- src/infrastructure/services/plugin-service.ts | 79 ++--- src/infrastructure/services/portal-service.ts | 270 ++++++++++++------ src/types/api/generation-status-endpoint.ts | 15 - src/types/plugin/generation-status.ts | 5 +- .../generation-status-poller.test.ts | 53 ++++ .../services/plugin-service.test.ts | 31 +- .../services/portal-service.test.ts | 48 ++++ 11 files changed, 441 insertions(+), 226 deletions(-) create mode 100644 src/infrastructure/generation-status-poller.ts delete mode 100644 src/types/api/generation-status-endpoint.ts create mode 100644 test/infrastructure/generation-status-poller.test.ts diff --git a/.ai/skills/service.md b/.ai/skills/service.md index 91359365..33058793 100644 --- a/.ai/skills/service.md +++ b/.ai/skills/service.md @@ -34,7 +34,9 @@ Services live at `src/infrastructure/services/` and are the only layer that make - Instantiate controller per method call: `new {ControllerName}(client)` inside the method. - `apiClientFactory.createApiClient(authHeader, shell)` provides the configured client. -- For async/polling SDK methods, don't hand-roll the poll loop — reuse `pollUntilCompleted()` in `portal-service.ts`, passing the poll interval as its first argument. Add a `static readonly` instance to `GenerationStatusEndpoint` for the new `{basePath}/{requestId}/status` endpoint and pass it to `ApiService.getGenerationStatus()`. Only supply a `ValidationErrorFormatter` when the endpoint needs custom validation wording (see `formatSdkValidationError`). +- For async/polling SDK methods, don't hand-roll the poll loop — reuse `pollUntilCompleted()` from `src/infrastructure/generation-status-poller.ts`, passing `{ pollIntervalMs, fetchStatus, timeout }`. Every pollable flow must pass a `timeout: { budgetMs, label }`; without one a stuck generation sits on the spinner forever. The budget alone is not a limit — it is only read between polls, so the `axiosInstance` behind `fetchStatus` also needs a `timeout`, or a poll that connects and never answers outlives the budget and hangs the CLI. The `label` opens the timeout message, so name the flow as the user knows it (`"Portal generation"`, `"SDK generation"`). Take the budget from a `generationTimeoutMs` constructor parameter so tests can shrink it. Only supply a `formatValidationError` when the endpoint needs custom validation wording (see `formatSdkValidationError`). +- Give each pollable endpoint its own private `get{Flow}GenerationStatus()` on the service that owns the flow, with the `{basePath}/{requestId}/status` path written out in it. Generation flows are independent and free to diverge, so the status fetch is deliberately not shared and takes no endpoint parameter, even where two flows currently read identically — resist factoring the bodies back together. A finished generation arrives as a `302` to the download location, not a `Completed` status body, so each method needs `maxRedirects: 0` and its own `302` mapping. +- An SDK-controller service that polls therefore also carries the axios-auth plumbing (`axiosInstance`, `apiBaseUrl`): the status endpoints are read over raw axios because a generated controller cannot surface the `302`. `portal-service.ts` is both variants at once for that reason. ### Axios-auth variant rules @@ -72,7 +74,8 @@ Services live at `src/infrastructure/services/` and are the only layer that make | Pattern | File | |---|---| | SDK controller + async polling | `src/infrastructure/services/portal-service.ts` | -| Pollable generation endpoints | `src/types/api/generation-status-endpoint.ts` | +| Per-endpoint generation status fetch | `src/infrastructure/services/portal-service.ts`, `src/infrastructure/services/plugin-service.ts` | +| Shared generation poll loop | `src/infrastructure/generation-status-poller.ts` | | SDK controller + FormData | `src/infrastructure/services/validation-service.ts` | | Raw axios with auth + axiosInstance | `src/infrastructure/services/api-service.ts` | | Raw axios with different base URL | `src/infrastructure/services/auth-service.ts` | diff --git a/src/config/axios-config.ts b/src/config/axios-config.ts index b0cd399f..9728f9ff 100644 --- a/src/config/axios-config.ts +++ b/src/config/axios-config.ts @@ -1,15 +1,19 @@ import axios from "axios"; const fiftyMBsInBytes = 50 * 1024 * 1024; -const fiveMinutesInMilliseconds = 5 * 60 * 1000; +const fourMinutesInMilliseconds = 4 * 60 * 1000; -/** For services that build their own instance and so cannot use the one below. */ -export const REQUEST_TIMEOUT_MS = fiveMinutesInMilliseconds; +/** + * The single bound on any one API request, shared by the instance below and by services that + * build their own. A generation budget is only read between polls, so without this a request + * that connects and never answers would outlive it and hang the CLI. + */ +export const REQUEST_TIMEOUT_MS = fourMinutesInMilliseconds; const axiosInstance = axios.create({ maxContentLength: fiftyMBsInBytes, maxBodyLength: fiftyMBsInBytes, - timeout: fiveMinutesInMilliseconds + timeout: REQUEST_TIMEOUT_MS }); export default axiosInstance; diff --git a/src/infrastructure/generation-status-poller.ts b/src/infrastructure/generation-status-poller.ts new file mode 100644 index 00000000..5199dcb3 --- /dev/null +++ b/src/infrastructure/generation-status-poller.ts @@ -0,0 +1,92 @@ +import { Status } from '@apimatic/sdk'; +import { err, ok, Result } from 'neverthrow'; +import { ServiceError } from './service-error.js'; + +export const STATUS_POLL_INTERVAL_MS = 3000; + +/** + * Set well clear of any plausible run: it exists to end a generation that is stuck, not to + * cap a slow one. No duration data exists for real runs, so this is the number to revisit + * if a legitimate generation ever reports a timeout. + */ +export const GENERATION_TIMEOUT_MS = 30 * 60 * 1000; + +export interface GenerationStatus { + status: string; + errors?: Record; +} + +export type ValidationErrorFormatter = (errors: Record) => string; + +export interface GenerationPoll { + pollIntervalMs: number; + fetchStatus: () => Promise>; + /** + * Bounds a run whose status never reaches a terminal value. `label` opens the message. + * The budget is only read between polls, so `fetchStatus` must bound its own request — + * one that connects and never answers outlives any budget set here. + */ + timeout: { budgetMs: number; label: string }; + /** Only when the endpoint needs custom validation wording. */ + formatValidationError?: ValidationErrorFormatter; +} + +export async function pollUntilCompleted({ + pollIntervalMs, + fetchStatus, + timeout, + formatValidationError = formatValidationErrors +}: GenerationPoll): Promise> { + const deadline = Date.now() + timeout.budgetMs; + + for (;;) { + await new Promise((resolve) => setTimeout(resolve, pollIntervalMs)); + + const statusResult = await fetchStatus(); + if (statusResult.isErr()) { + return err(statusResult.error); + } + + const { status, errors } = statusResult.value; + + if (status === Status.Completed) { + return ok(statusResult.value); + } + if (status === Status.Failed) { + return err(ServiceError.ServerError); + } + if (status === Status.ValidationError) { + const validationErrors = asMessages(errors); + return err(ServiceError.badRequest(formatValidationError(validationErrors), validationErrors)); + } + if (status === Status.SubscriptionError) { + const message = Object.values(asMessages(errors)).flat()[0]; + return err(ServiceError.forbidden('Access denied to resource.' + (message ? '\n- ' + message : ''))); + } + + // Every other status keeps the run alive rather than ending it: an endpoint reporting + // its own in-flight vocabulary is healthy, and the plugin orchestrator reports + // `Unknown` until it writes a custom status. The deadline is what stops a genuinely + // stuck run, and it is read after the status so a run that finished during the last + // wait still counts. + if (Date.now() >= deadline) { + return err(ServiceError.timeout(timedOutMessage(timeout.label, timeout.budgetMs))); + } + } +} + +export const formatValidationErrors: ValidationErrorFormatter = (errors) => { + const messages = Object.values(errors).flat(); + return 'One or more validation errors occurred.' + (messages.length ? '\n- ' + messages.join('\n- ') : ''); +}; + +const timedOutMessage = (label: string, budgetMs: number): string => { + const minutes = Math.floor(budgetMs / 60_000); + if (minutes < 1) { + return `${label} timed out.`; + } + return `${label} timed out after ${minutes} ${minutes === 1 ? 'minute' : 'minutes'}.`; +}; + +const asMessages = (errors: Record | undefined): Record => + (errors ?? {}) as Record; diff --git a/src/infrastructure/services/api-service.ts b/src/infrastructure/services/api-service.ts index a3301c56..c2640658 100644 --- a/src/infrastructure/services/api-service.ts +++ b/src/infrastructure/services/api-service.ts @@ -5,9 +5,6 @@ import { SubscriptionInfo } from "../../types/api/account.js"; import { envInfo } from "../env-info.js"; import { err, ok, Result } from "neverthrow"; import { handleServiceError, ServiceError } from "../service-error.js"; -import { Status } from "@apimatic/sdk"; -import { GenerationStatusEndpoint } from "../../types/api/generation-status-endpoint.js"; -import { GenerationStatusResponse } from "../../types/api/generation-status.js"; export class ApiService { private readonly apiBaseUrl = "https://api.apimatic.io" as const; @@ -35,58 +32,6 @@ export class ApiService { } } - /** - * Reads the status of one in-flight generation request. Shared by every - * async generation endpoint — see `GenerationStatusEndpoint`. - */ - public async getGenerationStatus( - endpoint: GenerationStatusEndpoint, - requestId: string, - configDir: DirectoryPath, - shell: string, - authKey: string | null - ): Promise> { - const authInfo: AuthInfo | null = await getAuthInfo(configDir.toString()); - if (authInfo === null && !authKey) { - return err(ServiceError.UnAuthorized); - } - - try { - const token = authKey || authInfo?.authKey; - const response = await this.axiosInstance(shell, token).get(`${endpoint}/${requestId}/status`, { - headers: { Accept: "application/json" }, - maxRedirects: 0, - validateStatus: () => true - }); - - if (response.status === 200) { - return ok(response.data as GenerationStatusResponse); - } - - // Once generation finishes, the API redirects to the download location. - if (response.status === 302) { - return ok({ status: Status.Completed }); - } - - // `validateStatus` above stops axios throwing, so nothing reaches the - // catch block — classify the status here, or a mistyped endpoint path and - // an expired auth key both surface as a generic "unexpected error". - if (response.status === 401) { - return err(ServiceError.UnAuthorized); - } - if (response.status === 404) { - return err(ServiceError.NotFound); - } - if (response.status === 500) { - return err(ServiceError.ServerError); - } - - return err(ServiceError.InvalidResponse); - } catch (error: unknown) { - return err(handleServiceError(error)); - } - } - public async sendTelemetry(payload: string, authKey: string, shell: string): Promise> { try { const response = await this.axiosInstance(shell, authKey).post("/telemetry/track", payload, { diff --git a/src/infrastructure/services/plugin-service.ts b/src/infrastructure/services/plugin-service.ts index 126542bc..55829653 100644 --- a/src/infrastructure/services/plugin-service.ts +++ b/src/infrastructure/services/plugin-service.ts @@ -13,11 +13,13 @@ import { } from '../../types/plugin/generation-status.js'; import { discardStreamBody } from '../../utils/utils.js'; import { envInfo } from '../env-info.js'; +import { + GENERATION_TIMEOUT_MS, + pollUntilCompleted, + STATUS_POLL_INTERVAL_MS +} from '../generation-status-poller.js'; import { FileService } from '../file-service.js'; -import { handleServiceError, mapRequestError, mapTransportError, ServiceError } from '../service-error.js'; - -const STATUS_POLL_INTERVAL_MS = 3000; -const GENERATION_TIMEOUT_MS = 5 * 60 * 1000; +import { mapRequestError, mapTransportError, ServiceError } from '../service-error.js'; export class PluginService { private readonly apiBaseUrl = 'https://api.apimatic.io' as const; @@ -56,9 +58,11 @@ export class PluginService { } const generationId = initiated.value.id; - const completed = await pollUntilCompleted(this.statusPollIntervalMs, this.generationTimeoutMs, () => - this.getGenerationStatus(generationId, commandMetadata.shell, token) - ); + const completed = await pollUntilCompleted({ + pollIntervalMs: this.statusPollIntervalMs, + fetchStatus: () => this.getGenerationStatus(generationId, commandMetadata.shell, token), + timeout: { budgetMs: this.generationTimeoutMs, label: 'Plugin generation' } + }); if (completed.isErr()) { return err(completed.error); } @@ -100,8 +104,6 @@ export class PluginService { try { const response = await this.axiosInstance(shell, token).get(`/plugin/${generationId}/status`, { headers: { Accept: 'application/json' }, - // This endpoint reports completion as a status body, never a redirect. Refusing to - // follow one surfaces a contract change immediately instead of polling to the timeout. maxRedirects: 0, validateStatus: () => true }); @@ -110,6 +112,11 @@ export class PluginService { return ok(response.data as PluginGenerationStatusResponse); } + // Once generation finishes, the API redirects to the download location. + if (response.status === 302) { + return ok({ status: PluginGenerationStatus.Completed }); + } + // `validateStatus` above stops axios throwing, so nothing reaches the catch block. if (response.status === 401) { return err(ServiceError.unauthorizedWithHint(null)); @@ -123,7 +130,7 @@ export class PluginService { return err(ServiceError.InvalidResponse); } catch (error) { - return err(handleServiceError(error)); + return err(mapRequestError(error)); } } @@ -150,8 +157,6 @@ export class PluginService { private axiosInstance(shell: string, apiKey: string) { return axios.create({ baseURL: envInfo.getBaseUrl() ?? this.apiBaseUrl, - // The generation budget is only read between polls, so a request that connects and - // then never answers would outlive it. Bounding the request is what makes it a limit. timeout: this.requestTimeoutMs, headers: { 'User-Agent': envInfo.getUserAgent(shell), @@ -161,53 +166,3 @@ export class PluginService { } } -async function pollUntilCompleted( - pollIntervalMs: number, - timeoutMs: number, - fetchStatus: () => Promise> -): Promise> { - const deadline = Date.now() + timeoutMs; - - for (;;) { - await new Promise((resolve) => setTimeout(resolve, pollIntervalMs)); - - const statusResult = await fetchStatus(); - if (statusResult.isErr()) { - return err(statusResult.error); - } - - const { status, errors } = statusResult.value; - - if (status === PluginGenerationStatus.Completed) { - return ok(statusResult.value); - } - if (status === PluginGenerationStatus.Failed) { - return err(ServiceError.ServerError); - } - if (status === PluginGenerationStatus.ValidationError) { - const validationErrors = errors ?? {}; - return err(ServiceError.badRequest(formatValidationErrors(validationErrors), validationErrors)); - } - - // `Unknown` joins the in-flight values here rather than ending the run: the orchestrator - // reports it whenever it has not written a custom status yet, which includes the window - // right after a healthy run starts. The deadline is what stops a genuinely stuck one, and - // it is read after the status so a run that finished during the last wait still counts. - if (Date.now() >= deadline) { - return err(ServiceError.timeout(timedOutMessage(timeoutMs))); - } - } -} - -const timedOutMessage = (timeoutMs: number): string => { - const minutes = Math.floor(timeoutMs / 60_000); - if (minutes < 1) { - return 'Plugin generation timed out.'; - } - return `Plugin generation timed out after ${minutes} ${minutes === 1 ? 'minute' : 'minutes'}.`; -}; - -const formatValidationErrors = (errors: Record): string => { - const messages = Object.values(errors).flat(); - return 'One or more validation errors occurred.' + (messages.length ? '\n- ' + messages.join('\n- ') : ''); -}; diff --git a/src/infrastructure/services/portal-service.ts b/src/infrastructure/services/portal-service.ts index cb2ddbb7..9556b6ef 100644 --- a/src/infrastructure/services/portal-service.ts +++ b/src/infrastructure/services/portal-service.ts @@ -1,4 +1,5 @@ import { ReadStream } from "fs"; +import axios from "axios"; import { ApiError, ApiResponse, @@ -28,10 +29,17 @@ import { CommandMetadata } from "../../types/common/command-metadata.js"; import { err, ok, Result } from "neverthrow"; import { Language, Stability } from "../../types/sdk/generate.js"; import { handleServiceError, ServiceError } from "../service-error.js"; -import { ApiService } from "./api-service.js"; +import { + formatValidationErrors, + GENERATION_TIMEOUT_MS, + pollUntilCompleted, + STATUS_POLL_INTERVAL_MS, + ValidationErrorFormatter +} from "../generation-status-poller.js"; +import { envInfo } from "../env-info.js"; +import { REQUEST_TIMEOUT_MS } from "../../config/axios-config.js"; import { SemVersion } from "../../types/publish/version.js"; import { TocData } from "../../types/toc/toc-components.js"; -import { GenerationStatusEndpoint } from "../../types/api/generation-status-endpoint.js"; import { GenerationStatusResponse } from "../../types/api/generation-status.js"; export interface GeneratedSdkResult { @@ -39,19 +47,22 @@ export interface GeneratedSdkResult { sdkSourceTree: NodeJS.ReadableStream; } -const STATUS_POLL_INTERVAL_MS = 3000; - -type FetchGenerationStatus = () => Promise>; -type ValidationErrorFormatter = (errors: Record) => string; - export class PortalService { private readonly CONTENT_TYPE = ContentType.EnumMultipartformdata; + private readonly apiBaseUrl = "https://api.apimatic.io" as const; private readonly fileService = new FileService(); - private readonly apiService = new ApiService(); private readonly statusPollIntervalMs: number; - - constructor(statusPollIntervalMs: number = STATUS_POLL_INTERVAL_MS) { + private readonly generationTimeoutMs: number; + private readonly requestTimeoutMs: number; + + constructor( + statusPollIntervalMs: number = STATUS_POLL_INTERVAL_MS, + generationTimeoutMs: number = GENERATION_TIMEOUT_MS, + requestTimeoutMs: number = REQUEST_TIMEOUT_MS + ) { this.statusPollIntervalMs = statusPollIntervalMs; + this.generationTimeoutMs = generationTimeoutMs; + this.requestTimeoutMs = requestTimeoutMs; } // TODO: Pass stream as parameter instead of file path. @@ -83,15 +94,12 @@ export class PortalService { buildFileStream.close(); } - const statusResult = await pollUntilCompleted(this.statusPollIntervalMs, () => - this.apiService.getGenerationStatus( - GenerationStatusEndpoint.Portal, - generationId, - configDir, - commandMetadata.shell, - authKey - ) - ); + const statusResult = await pollUntilCompleted({ + pollIntervalMs: this.statusPollIntervalMs, + fetchStatus: () => + this.getPortalGenerationStatus(generationId, commandMetadata.shell, this.resolveToken(authInfo, authKey)), + timeout: { budgetMs: this.generationTimeoutMs, label: "Portal generation" } + }); if (statusResult.isErr()) { return err(statusResult.error); } @@ -140,18 +148,13 @@ export class PortalService { buildFileStream.close(); } - const statusResult = await pollUntilCompleted( - this.statusPollIntervalMs, - () => - this.apiService.getGenerationStatus( - GenerationStatusEndpoint.Sdk, - generationId, - configDir, - commandMetadata.shell, - authKey - ), - formatSdkValidationError - ); + const statusResult = await pollUntilCompleted({ + pollIntervalMs: this.statusPollIntervalMs, + fetchStatus: () => + this.getSdkGenerationStatus(generationId, commandMetadata.shell, this.resolveToken(authInfo, authKey)), + timeout: { budgetMs: this.generationTimeoutMs, label: "SDK generation" }, + formatValidationError: formatSdkValidationError + }); if (statusResult.isErr()) { return err(statusResult.error); } @@ -200,15 +203,12 @@ export class PortalService { buildFileStream.close(); } - const statusResult = await pollUntilCompleted(this.statusPollIntervalMs, () => - this.apiService.getGenerationStatus( - GenerationStatusEndpoint.V4Sdk, - generationId, - configDir, - commandMetadata.shell, - authKey - ) - ); + const statusResult = await pollUntilCompleted({ + pollIntervalMs: this.statusPollIntervalMs, + fetchStatus: () => + this.getV4SdkGenerationStatus(generationId, commandMetadata.shell, this.resolveToken(authInfo, authKey)), + timeout: { budgetMs: this.generationTimeoutMs, label: "SDK generation" } + }); if (statusResult.isErr()) { return err(statusResult.error); } @@ -290,10 +290,156 @@ export class PortalService { * back to the shared format. */ private createAuthorizationHeader =(authInfo: AuthInfo | null, overrideAuthKey: string | null): string => { - const key = overrideAuthKey || authInfo?.authKey; - return `X-Auth-Key ${key ?? ""}`; + return `X-Auth-Key ${this.resolveToken(authInfo, overrideAuthKey) ?? ""}`; }; + private resolveToken = (authInfo: AuthInfo | null, overrideAuthKey: string | null): string | undefined => { + return overrideAuthKey || authInfo?.authKey; + }; + + private async getPortalGenerationStatus( + requestId: string, + shell: string, + token: string | undefined + ): Promise> { + if (!token) { + return err(ServiceError.UnAuthorized); + } + + try { + const response = await this.axiosInstance(shell, token).get(`/portal/v2/${requestId}/status`, { + headers: { Accept: "application/json" }, + maxRedirects: 0, + validateStatus: () => true + }); + + if (response.status === 200) { + return ok(response.data as GenerationStatusResponse); + } + + // Once generation finishes, the API redirects to the download location. + if (response.status === 302) { + return ok({ status: Status.Completed }); + } + + // `validateStatus` above stops axios throwing, so nothing reaches the + // catch block — classify the status here, or a mistyped endpoint path and + // an expired auth key both surface as a generic "unexpected error". + if (response.status === 401) { + return err(ServiceError.UnAuthorized); + } + if (response.status === 404) { + return err(ServiceError.NotFound); + } + if (response.status === 500) { + return err(ServiceError.ServerError); + } + + return err(ServiceError.InvalidResponse); + } catch (error: unknown) { + return err(handleServiceError(error)); + } + } + + private async getSdkGenerationStatus( + requestId: string, + shell: string, + token: string | undefined + ): Promise> { + if (!token) { + return err(ServiceError.UnAuthorized); + } + + try { + const response = await this.axiosInstance(shell, token).get(`/sdk/${requestId}/status`, { + headers: { Accept: "application/json" }, + maxRedirects: 0, + validateStatus: () => true + }); + + if (response.status === 200) { + return ok(response.data as GenerationStatusResponse); + } + + // Once generation finishes, the API redirects to the download location. + if (response.status === 302) { + return ok({ status: Status.Completed }); + } + + // `validateStatus` above stops axios throwing, so nothing reaches the + // catch block — classify the status here, or a mistyped endpoint path and + // an expired auth key both surface as a generic "unexpected error". + if (response.status === 401) { + return err(ServiceError.UnAuthorized); + } + if (response.status === 404) { + return err(ServiceError.NotFound); + } + if (response.status === 500) { + return err(ServiceError.ServerError); + } + + return err(ServiceError.InvalidResponse); + } catch (error: unknown) { + return err(handleServiceError(error)); + } + } + + private async getV4SdkGenerationStatus( + requestId: string, + shell: string, + token: string | undefined + ): Promise> { + if (!token) { + return err(ServiceError.UnAuthorized); + } + + try { + const response = await this.axiosInstance(shell, token).get(`/sdk/v2/${requestId}/status`, { + headers: { Accept: "application/json" }, + maxRedirects: 0, + validateStatus: () => true + }); + + if (response.status === 200) { + return ok(response.data as GenerationStatusResponse); + } + + // Once generation finishes, the API redirects to the download location. + if (response.status === 302) { + return ok({ status: Status.Completed }); + } + + // `validateStatus` above stops axios throwing, so nothing reaches the + // catch block — classify the status here, or a mistyped endpoint path and + // an expired auth key both surface as a generic "unexpected error". + if (response.status === 401) { + return err(ServiceError.UnAuthorized); + } + if (response.status === 404) { + return err(ServiceError.NotFound); + } + if (response.status === 500) { + return err(ServiceError.ServerError); + } + + return err(ServiceError.InvalidResponse); + } catch (error: unknown) { + return err(handleServiceError(error)); + } + } + + private axiosInstance(shell: string, apiKey: string) { + return axios.create({ + baseURL: envInfo.getBaseUrl() ?? this.apiBaseUrl, + timeout: this.requestTimeoutMs, + headers: { + "User-Agent": envInfo.getUserAgent(shell), + Authorization: `X-Auth-Key ${apiKey}` + } + }); + } + private createOriginQueryParameter = (commandName: string): Record => { return { origin: `APIMATIC CLI ${commandName}` @@ -316,38 +462,6 @@ export class PortalService { }; } -async function pollUntilCompleted( - pollIntervalMs: number, - fetchStatus: FetchGenerationStatus, - formatValidationError: ValidationErrorFormatter = formatValidationErrors -): Promise> { - for (;;) { - await new Promise((resolve) => setTimeout(resolve, pollIntervalMs)); - - const statusResult = await fetchStatus(); - if (statusResult.isErr()) { - return err(statusResult.error); - } - - const { status, errors } = statusResult.value; - - if (status === Status.Completed) { - return ok(statusResult.value); - } - if (status === Status.Failed) { - return err(ServiceError.ServerError); - } - if (status === Status.ValidationError) { - const validationErrors = asMessages(errors); - return err(ServiceError.badRequest(formatValidationError(validationErrors), validationErrors)); - } - if (status === Status.SubscriptionError) { - const message = Object.values(asMessages(errors)).flat()[0]; - return err(ServiceError.forbidden("Access denied to resource." + (message ? "\n- " + message : ""))); - } - } -} - const formatSdkValidationError: ValidationErrorFormatter = (errors) => { const sdkMergeFailedLanguages = errors.sdkMergeFailed; if (sdkMergeFailedLanguages?.length) { @@ -360,11 +474,3 @@ const formatSdkValidationError: ValidationErrorFormatter = (errors) => { return formatValidationErrors(errors); }; -const formatValidationErrors: ValidationErrorFormatter = (errors) => { - const messages = Object.values(errors).flat(); - return "One or more validation errors occurred." + (messages.length ? "\n- " + messages.join("\n- ") : ""); -}; - -const asMessages = (errors: Record | undefined): Record => - (errors ?? {}) as Record; - diff --git a/src/types/api/generation-status-endpoint.ts b/src/types/api/generation-status-endpoint.ts deleted file mode 100644 index 2ab672ae..00000000 --- a/src/types/api/generation-status-endpoint.ts +++ /dev/null @@ -1,15 +0,0 @@ -export class GenerationStatusEndpoint { - public static readonly Portal = new GenerationStatusEndpoint("/portal/v2"); - public static readonly Sdk = new GenerationStatusEndpoint("/sdk"); - public static readonly V4Sdk = new GenerationStatusEndpoint("/sdk/v2"); - - private readonly basePath: string; - - private constructor(basePath: string) { - this.basePath = basePath; - } - - public toString(): string { - return this.basePath; - } -} diff --git a/src/types/plugin/generation-status.ts b/src/types/plugin/generation-status.ts index b1895bae..bede1c20 100644 --- a/src/types/plugin/generation-status.ts +++ b/src/types/plugin/generation-status.ts @@ -1,8 +1,9 @@ /** * Context plugin generation reports its own status vocabulary, which does not match the * shared `Status` enum in `@apimatic/sdk`: there is no `SubscriptionError` (entitlement is - * a 403 on the generate call instead), completion is a status rather than a redirect, and - * the three in-flight values below have no equivalent. + * a 403 on the generate call instead), and the in-flight values below have no equivalent. + * `Completed` is not sent on the wire — as with portal generation, a finished run is a 302 + * to the download endpoint, which `getGenerationStatus` maps onto it. */ export enum PluginGenerationStatus { Queued = 'Queued', diff --git a/test/infrastructure/generation-status-poller.test.ts b/test/infrastructure/generation-status-poller.test.ts new file mode 100644 index 00000000..b2e881dd --- /dev/null +++ b/test/infrastructure/generation-status-poller.test.ts @@ -0,0 +1,53 @@ +import { expect } from 'chai'; +import sinon from 'sinon'; +import { ok, Result } from 'neverthrow'; +import { pollUntilCompleted } from '../../src/infrastructure/generation-status-poller'; +import { ServiceError, ServiceErrorCode } from '../../src/infrastructure/service-error'; + +// The service suites reach the timeout with millisecond budgets, which only ever renders the +// sub-minute wording. The message a user actually sees is built from a production budget, so +// the minute branches are exercised here over a fake clock rather than in real time. +describe('pollUntilCompleted', () => { + let clock: sinon.SinonFakeTimers; + + beforeEach(() => { + clock = sinon.useFakeTimers(); + }); + + afterEach(() => { + clock.restore(); + }); + + const neverFinishes = async (): Promise> => ok({ status: 'InProgress' }); + + const timeoutAfter = async (budgetMs: number, label: string) => { + const polling = pollUntilCompleted({ + pollIntervalMs: 1000, + fetchStatus: neverFinishes, + timeout: { budgetMs, label } + }); + await clock.tickAsync(budgetMs + 1000); + const result = await polling; + + expect(result.isErr(), 'a run that never finishes should time out').to.be.true; + const error = result._unsafeUnwrapErr(); + expect(error.code).to.equal(ServiceErrorCode.Timeout); + return error.errorMessage; + }; + + it('names the budget in minutes', async () => { + expect(await timeoutAfter(5 * 60_000, 'Plugin generation')).to.equal('Plugin generation timed out after 5 minutes.'); + }); + + it('says minute rather than minutes for a one-minute budget', async () => { + expect(await timeoutAfter(60_000, 'Portal generation')).to.equal('Portal generation timed out after 1 minute.'); + }); + + it('omits the duration when the budget is under a minute', async () => { + expect(await timeoutAfter(15, 'SDK generation')).to.equal('SDK generation timed out.'); + }); + + it('rounds down to whole minutes', async () => { + expect(await timeoutAfter(90_000, 'Portal generation')).to.equal('Portal generation timed out after 1 minute.'); + }); +}); diff --git a/test/infrastructure/services/plugin-service.test.ts b/test/infrastructure/services/plugin-service.test.ts index 5d3c20f6..49676417 100644 --- a/test/infrastructure/services/plugin-service.test.ts +++ b/test/infrastructure/services/plugin-service.test.ts @@ -42,6 +42,12 @@ describe('PluginService', () => { const statusBody = (body: unknown) => (res: http.ServerResponse) => json(res, 200, body); + // A finished run is signalled by a redirect to the download endpoint, not by a status body. + const redirect = (res: http.ServerResponse) => { + res.writeHead(302, { Location: `/plugin/${GENERATION_ID}/download` }); + res.end(); + }; + const drain = async (stream: NodeJS.ReadableStream) => { const chunks: Buffer[] = []; for await (const chunk of stream) chunks.push(Buffer.from(chunk)); @@ -110,7 +116,7 @@ describe('PluginService', () => { statusRequests.length = 0; downloadRequests.length = 0; respondToGenerate = (res) => json(res, 202, { id: GENERATION_ID }); - respondToStatus = statusBody({ status: 'Completed' }); + respondToStatus = redirect; respondToDownload = (res) => { res.writeHead(200, { 'Content-Type': 'application/zip' }); res.end(Buffer.from('PK context-plugin')); @@ -149,9 +155,26 @@ describe('PluginService', () => { expect(downloadRequests[0].headers.authorization).to.equal(`X-Auth-Key ${AUTH_KEY}`); }); + it('treats the redirect to the download endpoint as completion', async () => { + // The status endpoint answers a finished run with a 302, as `/portal/v2` does. Following + // it transparently would hand the zip back as a status body and poll to the timeout. + const result = await generatePlugin(); + + expect(result.isOk()).to.be.true; + expect(statusRequests).to.have.length(1); + expect(downloadRequests).to.have.length(1); + }); + + it('completes on a Completed status body too', async () => { + respondToStatus = statusBody({ status: 'Completed' }); + + expect((await generatePlugin()).isOk()).to.be.true; + }); + it('keeps polling through every in-flight status', async () => { const inFlight = ['Queued', 'ExecutionStarted', 'GeneratingArtifacts']; - respondToStatus = (res, attempt) => json(res, 200, { status: inFlight[attempt - 1] ?? 'Completed' }); + respondToStatus = (res, attempt) => + inFlight[attempt - 1] ? json(res, 200, { status: inFlight[attempt - 1] }) : redirect(res); const result = await generatePlugin(); @@ -210,7 +233,7 @@ describe('PluginService', () => { it('keeps polling through an Unknown status', async () => { // The orchestrator reports Unknown until it writes its first custom status, so a run // polled in that window is healthy rather than broken. - respondToStatus = (res, attempt) => json(res, 200, { status: attempt === 1 ? 'Unknown' : 'Completed' }); + respondToStatus = (res, attempt) => (attempt === 1 ? json(res, 200, { status: 'Unknown' }) : redirect(res)); const result = await generatePlugin(); @@ -242,7 +265,7 @@ describe('PluginService', () => { it('reads one last status before declaring a timeout', async () => { // The budget is spent during the wait, but the run finished in that window. const impatient = new PluginService(20, 10); - respondToStatus = statusBody({ status: 'Completed' }); + respondToStatus = redirect; const result = await impatient.generatePlugin(buildPath, configDir, metadata, AUTH_KEY); diff --git a/test/infrastructure/services/portal-service.test.ts b/test/infrastructure/services/portal-service.test.ts index bf8efac1..71ae9c86 100644 --- a/test/infrastructure/services/portal-service.test.ts +++ b/test/infrastructure/services/portal-service.test.ts @@ -305,4 +305,52 @@ describe("PortalService generation status polling", () => { expect(statusRequests.map((request) => request.url)).to.deep.equal([`/sdk/v2/${GENERATION_ID}/status`]); }); }); + + // Each flow names itself in the timeout message, so each one's wiring is pinned separately. + // Before the shared poller these three polled a stuck generation forever. + describe("giving up on a generation that never finishes", () => { + const impatient = () => new PortalService(1, 15); + + beforeEach(() => { + respondToStatus = statusBody({ status: Status.InProgress }); + }); + + it("bounds portal generation", async () => { + const result = await impatient().generatePortal(buildPath, configDir, metadata, AUTH_KEY); + + expect(errorFrom(result).code).to.equal(ServiceErrorCode.Timeout); + expect(errorFrom(result).errorMessage).to.equal("Portal generation timed out."); + }); + + it("bounds sdk generation", async () => { + const result = await impatient().generateSdk(buildPath, Language.TYPESCRIPT, configDir, metadata, AUTH_KEY); + + expect(errorFrom(result).code).to.equal(ServiceErrorCode.Timeout); + expect(errorFrom(result).errorMessage).to.equal("SDK generation timed out."); + }); + + it("bounds v4 sdk generation", async () => { + const result = await impatient().generateV4Sdk( + buildPath, + Language.CSHARP, + Stability.BETA, + configDir, + metadata, + AUTH_KEY + ); + + expect(errorFrom(result).code).to.equal(ServiceErrorCode.Timeout); + expect(errorFrom(result).errorMessage).to.equal("SDK generation timed out."); + }); + + it("gives up on a status request that never answers", async () => { + // The budget above is only read once a status call returns, so it cannot end a run + // whose poll hangs. Only the request timeout can, which is why one is set. + respondToStatus = () => {}; + + const result = await new PortalService(1, 5_000, 30).generatePortal(buildPath, configDir, metadata, AUTH_KEY); + + expect(errorFrom(result).code).to.equal(ServiceErrorCode.NetworkError); + }); + }); }); From 0f12f0a041911ea96888c152e08808dc68673dfd Mon Sep 17 00:00:00 2001 From: saeedjamshaid Date: Thu, 13 Aug 2026 16:05:06 +0500 Subject: [PATCH 12/12] refactor: take generation timings as one named argument MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `new PortalService(1, 5_000, 30)` said nothing about which number was the poll interval, which the generation budget and which the per-request bound. No production caller passed any of them either — all three parameters existed only so tests could shrink the pacing, which is the worst reason to have a positional seam. Both services now take a single optional `GenerationTimings`, defaulted by spreading over a `TIMING_DEFAULTS` object: one assignment instead of three fields, and `Partial` derives the accepted options from the defaults so the two cannot drift. `TIMING_DEFAULTS` is declared per service rather than shared — two of its values belong to the poll loop and one to axios config, so a shared home would make the poller import transport config it does not use. The type is exported because `declaration: true` rejects a private name in a public constructor signature. Two cleanups while here: the `sdkMergeFailed` note moves onto `formatSdkValidationError`, which is what it describes rather than `createAuthorizationHeader` several methods above it, and the plugin action loses its `displayMessages` parameter, which no caller ever passed. Co-Authored-By: Claude Opus 5 (1M context) --- src/actions/plugin/generate.ts | 7 +-- src/infrastructure/services/plugin-service.ts | 31 ++++++------ src/infrastructure/services/portal-service.ts | 50 +++++++++---------- .../services/plugin-service.test.ts | 8 +-- .../services/portal-service.test.ts | 8 +-- 5 files changed, 52 insertions(+), 52 deletions(-) diff --git a/src/actions/plugin/generate.ts b/src/actions/plugin/generate.ts index bae9c876..7427f9b2 100644 --- a/src/actions/plugin/generate.ts +++ b/src/actions/plugin/generate.ts @@ -25,8 +25,7 @@ export class PluginGenerateAction { buildDirectory: DirectoryPath, pluginDirectory: DirectoryPath, force: boolean, - zipPlugin: boolean, - displayMessages: boolean = true + zipPlugin: boolean ): Promise => { if (buildDirectory.isEqual(pluginDirectory)) { this.prompts.directoryCannotBeSame(pluginDirectory); @@ -61,9 +60,7 @@ export class PluginGenerateAction { const tempPluginZipPath = await tempContext.save(response.value); await pluginContext.save(tempPluginZipPath, zipPlugin); - if (displayMessages) { - this.prompts.pluginGenerated(pluginDirectory); - } + this.prompts.pluginGenerated(pluginDirectory); return ActionResult.success(); }); diff --git a/src/infrastructure/services/plugin-service.ts b/src/infrastructure/services/plugin-service.ts index 55829653..d14890c2 100644 --- a/src/infrastructure/services/plugin-service.ts +++ b/src/infrastructure/services/plugin-service.ts @@ -21,21 +21,22 @@ import { import { FileService } from '../file-service.js'; import { mapRequestError, mapTransportError, ServiceError } from '../service-error.js'; +const TIMING_DEFAULTS = { + pollIntervalMs: STATUS_POLL_INTERVAL_MS, + generationTimeoutMs: GENERATION_TIMEOUT_MS, + requestTimeoutMs: REQUEST_TIMEOUT_MS +}; + +/** Overridable so tests are not paced by the production defaults; nothing else overrides them. */ +export type GenerationTimings = Partial; + export class PluginService { private readonly apiBaseUrl = 'https://api.apimatic.io' as const; private readonly fileService = new FileService(); - private readonly statusPollIntervalMs: number; - private readonly generationTimeoutMs: number; - private readonly requestTimeoutMs: number; - - constructor( - statusPollIntervalMs: number = STATUS_POLL_INTERVAL_MS, - generationTimeoutMs: number = GENERATION_TIMEOUT_MS, - requestTimeoutMs: number = REQUEST_TIMEOUT_MS - ) { - this.statusPollIntervalMs = statusPollIntervalMs; - this.generationTimeoutMs = generationTimeoutMs; - this.requestTimeoutMs = requestTimeoutMs; + private readonly timings: typeof TIMING_DEFAULTS; + + constructor(timings: GenerationTimings = {}) { + this.timings = { ...TIMING_DEFAULTS, ...timings }; } public async generatePlugin( @@ -59,9 +60,9 @@ export class PluginService { const generationId = initiated.value.id; const completed = await pollUntilCompleted({ - pollIntervalMs: this.statusPollIntervalMs, + pollIntervalMs: this.timings.pollIntervalMs, fetchStatus: () => this.getGenerationStatus(generationId, commandMetadata.shell, token), - timeout: { budgetMs: this.generationTimeoutMs, label: 'Plugin generation' } + timeout: { budgetMs: this.timings.generationTimeoutMs, label: 'Plugin generation' } }); if (completed.isErr()) { return err(completed.error); @@ -157,7 +158,7 @@ export class PluginService { private axiosInstance(shell: string, apiKey: string) { return axios.create({ baseURL: envInfo.getBaseUrl() ?? this.apiBaseUrl, - timeout: this.requestTimeoutMs, + timeout: this.timings.requestTimeoutMs, headers: { 'User-Agent': envInfo.getUserAgent(shell), Authorization: `X-Auth-Key ${apiKey}` diff --git a/src/infrastructure/services/portal-service.ts b/src/infrastructure/services/portal-service.ts index 9556b6ef..3101fa62 100644 --- a/src/infrastructure/services/portal-service.ts +++ b/src/infrastructure/services/portal-service.ts @@ -47,22 +47,23 @@ export interface GeneratedSdkResult { sdkSourceTree: NodeJS.ReadableStream; } +const TIMING_DEFAULTS = { + pollIntervalMs: STATUS_POLL_INTERVAL_MS, + generationTimeoutMs: GENERATION_TIMEOUT_MS, + requestTimeoutMs: REQUEST_TIMEOUT_MS +}; + +/** Overridable so tests are not paced by the production defaults; nothing else overrides them. */ +export type GenerationTimings = Partial; + export class PortalService { private readonly CONTENT_TYPE = ContentType.EnumMultipartformdata; private readonly apiBaseUrl = "https://api.apimatic.io" as const; private readonly fileService = new FileService(); - private readonly statusPollIntervalMs: number; - private readonly generationTimeoutMs: number; - private readonly requestTimeoutMs: number; - - constructor( - statusPollIntervalMs: number = STATUS_POLL_INTERVAL_MS, - generationTimeoutMs: number = GENERATION_TIMEOUT_MS, - requestTimeoutMs: number = REQUEST_TIMEOUT_MS - ) { - this.statusPollIntervalMs = statusPollIntervalMs; - this.generationTimeoutMs = generationTimeoutMs; - this.requestTimeoutMs = requestTimeoutMs; + private readonly timings: typeof TIMING_DEFAULTS; + + constructor(timings: GenerationTimings = {}) { + this.timings = { ...TIMING_DEFAULTS, ...timings }; } // TODO: Pass stream as parameter instead of file path. @@ -95,10 +96,10 @@ export class PortalService { } const statusResult = await pollUntilCompleted({ - pollIntervalMs: this.statusPollIntervalMs, + pollIntervalMs: this.timings.pollIntervalMs, fetchStatus: () => this.getPortalGenerationStatus(generationId, commandMetadata.shell, this.resolveToken(authInfo, authKey)), - timeout: { budgetMs: this.generationTimeoutMs, label: "Portal generation" } + timeout: { budgetMs: this.timings.generationTimeoutMs, label: "Portal generation" } }); if (statusResult.isErr()) { return err(statusResult.error); @@ -149,10 +150,10 @@ export class PortalService { } const statusResult = await pollUntilCompleted({ - pollIntervalMs: this.statusPollIntervalMs, + pollIntervalMs: this.timings.pollIntervalMs, fetchStatus: () => this.getSdkGenerationStatus(generationId, commandMetadata.shell, this.resolveToken(authInfo, authKey)), - timeout: { budgetMs: this.generationTimeoutMs, label: "SDK generation" }, + timeout: { budgetMs: this.timings.generationTimeoutMs, label: "SDK generation" }, formatValidationError: formatSdkValidationError }); if (statusResult.isErr()) { @@ -204,10 +205,10 @@ export class PortalService { } const statusResult = await pollUntilCompleted({ - pollIntervalMs: this.statusPollIntervalMs, + pollIntervalMs: this.timings.pollIntervalMs, fetchStatus: () => this.getV4SdkGenerationStatus(generationId, commandMetadata.shell, this.resolveToken(authInfo, authKey)), - timeout: { budgetMs: this.generationTimeoutMs, label: "SDK generation" } + timeout: { budgetMs: this.timings.generationTimeoutMs, label: "SDK generation" } }); if (statusResult.isErr()) { return err(statusResult.error); @@ -284,12 +285,7 @@ export class PortalService { } } - /** - * SDK generation reports per-language merge conflicts under a dedicated - * `sdkMergeFailed` key, which needs its own wording. Everything else falls - * back to the shared format. - */ - private createAuthorizationHeader =(authInfo: AuthInfo | null, overrideAuthKey: string | null): string => { + private createAuthorizationHeader = (authInfo: AuthInfo | null, overrideAuthKey: string | null): string => { return `X-Auth-Key ${this.resolveToken(authInfo, overrideAuthKey) ?? ""}`; }; @@ -432,7 +428,7 @@ export class PortalService { private axiosInstance(shell: string, apiKey: string) { return axios.create({ baseURL: envInfo.getBaseUrl() ?? this.apiBaseUrl, - timeout: this.requestTimeoutMs, + timeout: this.timings.requestTimeoutMs, headers: { "User-Agent": envInfo.getUserAgent(shell), Authorization: `X-Auth-Key ${apiKey}` @@ -462,6 +458,10 @@ export class PortalService { }; } +/** + * SDK generation reports per-language merge conflicts under a dedicated `sdkMergeFailed` + * key, which needs its own wording. Everything else falls back to the shared format. + */ const formatSdkValidationError: ValidationErrorFormatter = (errors) => { const sdkMergeFailedLanguages = errors.sdkMergeFailed; if (sdkMergeFailedLanguages?.length) { diff --git a/test/infrastructure/services/plugin-service.test.ts b/test/infrastructure/services/plugin-service.test.ts index 49676417..281296d9 100644 --- a/test/infrastructure/services/plugin-service.test.ts +++ b/test/infrastructure/services/plugin-service.test.ts @@ -100,7 +100,7 @@ describe('PluginService', () => { // No config.json here, so the explicit authKey is used. configDir = new DirectoryPath(workDir); // Near-zero interval and budget so the suite is not paced by the production 3s / 5min. - service = new PluginService(1, 5000); + service = new PluginService({ pollIntervalMs: 1, generationTimeoutMs: 5000 }); }); after(async () => { @@ -242,7 +242,7 @@ describe('PluginService', () => { }); it('gives up once the generation budget is spent', async () => { - const impatient = new PluginService(1, 15); + const impatient = new PluginService({ pollIntervalMs: 1, generationTimeoutMs: 15 }); respondToStatus = statusBody({ status: 'GeneratingArtifacts' }); const result = await impatient.generatePlugin(buildPath, configDir, metadata, AUTH_KEY); @@ -254,7 +254,7 @@ describe('PluginService', () => { it('gives up on a status request that never answers', async () => { // Without a request timeout this hangs rather than fails: the generation budget is // only read once a status call returns, and this one never does. - const impatient = new PluginService(1, 5_000, 30); + const impatient = new PluginService({ pollIntervalMs: 1, generationTimeoutMs: 5_000, requestTimeoutMs: 30 }); respondToStatus = () => {}; const result = await impatient.generatePlugin(buildPath, configDir, metadata, AUTH_KEY); @@ -264,7 +264,7 @@ describe('PluginService', () => { it('reads one last status before declaring a timeout', async () => { // The budget is spent during the wait, but the run finished in that window. - const impatient = new PluginService(20, 10); + const impatient = new PluginService({ pollIntervalMs: 20, generationTimeoutMs: 10 }); respondToStatus = redirect; const result = await impatient.generatePlugin(buildPath, configDir, metadata, AUTH_KEY); diff --git a/test/infrastructure/services/portal-service.test.ts b/test/infrastructure/services/portal-service.test.ts index 71ae9c86..0cb2fd3f 100644 --- a/test/infrastructure/services/portal-service.test.ts +++ b/test/infrastructure/services/portal-service.test.ts @@ -91,7 +91,7 @@ describe("PortalService generation status polling", () => { // No config.json here, so the explicit authKey is used. configDir = new DirectoryPath(workDir); // Near-zero poll interval so the suite is not paced by the 3s production default. - service = new PortalService(1); + service = new PortalService({ pollIntervalMs: 1 }); }); after(async () => { @@ -309,7 +309,7 @@ describe("PortalService generation status polling", () => { // Each flow names itself in the timeout message, so each one's wiring is pinned separately. // Before the shared poller these three polled a stuck generation forever. describe("giving up on a generation that never finishes", () => { - const impatient = () => new PortalService(1, 15); + const impatient = () => new PortalService({ pollIntervalMs: 1, generationTimeoutMs: 15 }); beforeEach(() => { respondToStatus = statusBody({ status: Status.InProgress }); @@ -348,7 +348,9 @@ describe("PortalService generation status polling", () => { // whose poll hangs. Only the request timeout can, which is why one is set. respondToStatus = () => {}; - const result = await new PortalService(1, 5_000, 30).generatePortal(buildPath, configDir, metadata, AUTH_KEY); + const bounded = new PortalService({ pollIntervalMs: 1, generationTimeoutMs: 5_000, requestTimeoutMs: 30 }); + + const result = await bounded.generatePortal(buildPath, configDir, metadata, AUTH_KEY); expect(errorFrom(result).code).to.equal(ServiceErrorCode.NetworkError); });