-
Notifications
You must be signed in to change notification settings - Fork 292
feat(cli): auto-download protoc-gen-openapi binary when not on PATH #13667
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Swimburger
merged 15 commits into
main
from
devin/1773856030-auto-download-protoc-gen-openapi
Mar 18, 2026
Merged
Changes from all commits
Commits
Show all changes
15 commits
Select commit
Hold shift + click to select a range
032e916
feat(cli): auto-download protoc-gen-openapi binary when not on PATH
devin-ai-integration[bot] f5eaaab
fix: address compile error and review comments (path.delimiter, empty…
devin-ai-integration[bot] 4f6c776
Merge branch 'main' into devin/1773856030-auto-download-protoc-gen-op…
devin-ai-integration[bot] 247db1a
fix: add versioning marker and filesystem lock for race-safe protoc-g…
devin-ai-integration[bot] 48331b9
fix: add logging to catch blocks and handle EEXIST race in lock force…
devin-ai-integration[bot] 41afd95
test: add end-to-end tests for ProtocGenOpenAPIDownloader
devin-ai-integration[bot] 0da9fca
fix: wrap resolveProtocGenOpenAPI in top-level try-catch to always re…
devin-ai-integration[bot] 36c4b88
fix: biome formatting for catch block logger.debug
devin-ai-integration[bot] b6bdaa0
fix: use async writeFile, clean up temp on failure, move unstubAllGlo…
devin-ai-integration[bot] 9932cd7
fix: use Uint8Array instead of Buffer for writeFile compatibility
devin-ai-integration[bot] 1ad3437
fix: add logging to empty catch blocks per REVIEW.md rules
devin-ai-integration[bot] 61c3b5e
fix: biome formatting for cleanup catch block
devin-ai-integration[bot] 278c109
chore: bump protoc-gen-openapi version to v0.1.13
devin-ai-integration[bot] 8793b8b
test: add real e2e tests for protoc-gen-openapi auto-download from Gi…
devin-ai-integration[bot] 7d402e2
Merge branch 'main' into devin/1773856030-auto-download-protoc-gen-op…
devin-ai-integration[bot] File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
258 changes: 258 additions & 0 deletions
258
packages/cli/workspace/lazy-fern-workspace/src/protobuf/ProtocGenOpenAPIDownloader.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,258 @@ | ||
| import { AbsoluteFilePath, join, RelativeFilePath } from "@fern-api/fs-utils"; | ||
| import { Logger } from "@fern-api/logger"; | ||
| import { access, chmod, copyFile, mkdir, readFile, rename, rm, writeFile } from "fs/promises"; | ||
| import os from "os"; | ||
| import path from "path"; | ||
|
|
||
| const PROTOC_GEN_OPENAPI_VERSION = "v0.1.13"; | ||
| const GITHUB_RELEASE_URL_BASE = "https://github.com/fern-api/protoc-gen-openapi/releases/download"; | ||
| const BINARY_NAME = "protoc-gen-openapi"; | ||
| const CACHE_DIR_NAME = ".fern"; | ||
| const BIN_DIR_NAME = "bin"; | ||
| const LOCK_TIMEOUT_MS = 120_000; | ||
| const LOCK_RETRY_INTERVAL_MS = 200; | ||
|
|
||
| interface PlatformInfo { | ||
| os: string; | ||
| arch: string; | ||
| extension: string; | ||
| } | ||
|
|
||
| function getPlatformInfo(): PlatformInfo { | ||
| const platform = os.platform(); | ||
| const arch = os.arch(); | ||
|
|
||
| let osName: string; | ||
| switch (platform) { | ||
| case "linux": | ||
| osName = "linux"; | ||
| break; | ||
| case "darwin": | ||
| osName = "darwin"; | ||
| break; | ||
| case "win32": | ||
| osName = "windows"; | ||
| break; | ||
| default: | ||
| throw new Error(`Unsupported platform: ${platform}`); | ||
| } | ||
|
|
||
| let archName: string; | ||
| switch (arch) { | ||
| case "x64": | ||
| archName = "amd64"; | ||
| break; | ||
| case "arm64": | ||
| archName = "arm64"; | ||
| break; | ||
| default: | ||
| throw new Error(`Unsupported architecture: ${arch}`); | ||
| } | ||
|
|
||
| return { | ||
| os: osName, | ||
| arch: archName, | ||
| extension: osName === "windows" ? ".exe" : "" | ||
| }; | ||
| } | ||
|
|
||
| function getCacheDir(): AbsoluteFilePath { | ||
| const homeDir = os.homedir(); | ||
| return AbsoluteFilePath.of(path.join(homeDir, CACHE_DIR_NAME, BIN_DIR_NAME)); | ||
| } | ||
|
|
||
| function getVersionedBinaryPath(): AbsoluteFilePath { | ||
| const { extension } = getPlatformInfo(); | ||
| const cacheDir = getCacheDir(); | ||
| return join(cacheDir, RelativeFilePath.of(`${BINARY_NAME}-${PROTOC_GEN_OPENAPI_VERSION}${extension}`)); | ||
| } | ||
|
|
||
| function getCanonicalBinaryPath(): AbsoluteFilePath { | ||
| const { extension } = getPlatformInfo(); | ||
| const cacheDir = getCacheDir(); | ||
| return join(cacheDir, RelativeFilePath.of(`${BINARY_NAME}${extension}`)); | ||
| } | ||
|
|
||
| function getVersionMarkerPath(): AbsoluteFilePath { | ||
| const cacheDir = getCacheDir(); | ||
| return join(cacheDir, RelativeFilePath.of(`${BINARY_NAME}.version`)); | ||
| } | ||
|
|
||
| function getLockDirPath(): string { | ||
| const cacheDir = getCacheDir(); | ||
| return path.join(cacheDir, `${BINARY_NAME}.lock`); | ||
| } | ||
|
|
||
| function getDownloadUrl(): string { | ||
| const { os: osName, arch, extension } = getPlatformInfo(); | ||
| return `${GITHUB_RELEASE_URL_BASE}/${PROTOC_GEN_OPENAPI_VERSION}/${BINARY_NAME}-${osName}-${arch}${extension}`; | ||
| } | ||
|
|
||
| async function fileExists(filePath: AbsoluteFilePath): Promise<boolean> { | ||
| try { | ||
| await access(filePath); | ||
| return true; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Acquires an exclusive filesystem lock using mkdir (atomic on all platforms). | ||
| * Returns a release function that removes the lock directory. | ||
| * If the lock cannot be acquired within LOCK_TIMEOUT_MS, force-breaks it | ||
| * (assumes the holder crashed) and retries once. | ||
| */ | ||
| async function acquireLock(logger: Logger): Promise<() => Promise<void>> { | ||
| const lockPath = getLockDirPath(); | ||
| const deadline = Date.now() + LOCK_TIMEOUT_MS; | ||
|
|
||
| while (Date.now() < deadline) { | ||
| try { | ||
| await mkdir(lockPath, { recursive: false }); | ||
| return createLockReleaser(lockPath, logger); | ||
| } catch { | ||
| logger.debug(`Waiting for lock on ${lockPath}...`); | ||
| await new Promise((resolve) => setTimeout(resolve, LOCK_RETRY_INTERVAL_MS)); | ||
| } | ||
| } | ||
|
|
||
| // Timeout — force-break the presumed-stale lock and retry once | ||
| logger.debug(`Lock timed out after ${LOCK_TIMEOUT_MS}ms, breaking stale lock`); | ||
| try { | ||
| await rm(lockPath, { recursive: true }); | ||
| } catch (err) { | ||
| logger.debug(`Failed to remove stale lock: ${err instanceof Error ? err.message : String(err)}`); | ||
| } | ||
| try { | ||
| await mkdir(lockPath, { recursive: false }); | ||
| } catch (err) { | ||
| // Another process grabbed the lock between our rm and mkdir — wait briefly and retry | ||
| logger.debug(`Failed to re-acquire lock after break: ${err instanceof Error ? err.message : String(err)}`); | ||
| await new Promise((resolve) => setTimeout(resolve, LOCK_RETRY_INTERVAL_MS)); | ||
| await mkdir(lockPath, { recursive: false }); | ||
| } | ||
| return createLockReleaser(lockPath, logger); | ||
| } | ||
|
|
||
| function createLockReleaser(lockPath: string, logger: Logger): () => Promise<void> { | ||
| return async () => { | ||
| try { | ||
| await rm(lockPath, { recursive: true }); | ||
| } catch (err) { | ||
| logger.debug(`Failed to release lock: ${err instanceof Error ? err.message : String(err)}`); | ||
| } | ||
| }; | ||
| } | ||
|
|
||
| /** | ||
| * Resolves the protoc-gen-openapi binary, downloading it from GitHub Releases if needed. | ||
| * | ||
| * **Versioning**: Binaries are cached with a versioned filename (e.g. `protoc-gen-openapi-v0.1.13`). | ||
| * A `.version` marker file tracks which version the canonical `protoc-gen-openapi` binary corresponds to. | ||
| * When `PROTOC_GEN_OPENAPI_VERSION` is bumped, the canonical binary is atomically replaced on the | ||
| * next invocation. | ||
| * | ||
| * **Race conditions**: An exclusive filesystem lock (mkdir-based) is held during download and | ||
| * file operations to prevent concurrent processes from corrupting the cache. All file replacements | ||
| * use write-to-temp + atomic rename. | ||
| * | ||
| * @returns The directory containing the binary (for PATH injection), or `undefined` if download fails. | ||
| */ | ||
| export async function resolveProtocGenOpenAPI(logger: Logger): Promise<AbsoluteFilePath | undefined> { | ||
| try { | ||
| const cacheDir = getCacheDir(); | ||
| await mkdir(cacheDir, { recursive: true }); | ||
|
|
||
| const releaseLock = await acquireLock(logger); | ||
| try { | ||
| return await resolveUnderLock(logger); | ||
| } finally { | ||
| await releaseLock(); | ||
| } | ||
| } catch (error) { | ||
| logger.debug(`Failed to resolve protoc-gen-openapi: ${error instanceof Error ? error.message : String(error)}`); | ||
| return undefined; | ||
| } | ||
| } | ||
devin-ai-integration[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| async function resolveUnderLock(logger: Logger): Promise<AbsoluteFilePath | undefined> { | ||
| const versionedPath = getVersionedBinaryPath(); | ||
| const canonicalPath = getCanonicalBinaryPath(); | ||
| const versionMarkerPath = getVersionMarkerPath(); | ||
|
|
||
| // Fast path: versioned binary already downloaded | ||
| if (await fileExists(versionedPath)) { | ||
| const currentMarker = await readVersionMarker(versionMarkerPath, logger); | ||
| if (currentMarker === PROTOC_GEN_OPENAPI_VERSION && (await fileExists(canonicalPath))) { | ||
| logger.debug(`Using cached protoc-gen-openapi ${PROTOC_GEN_OPENAPI_VERSION}`); | ||
| return getCacheDir(); | ||
| } | ||
| // Version marker is stale or canonical binary is missing — refresh atomically | ||
| await atomicCopyBinary(versionedPath, canonicalPath); | ||
| await writeFile(versionMarkerPath, PROTOC_GEN_OPENAPI_VERSION); | ||
| logger.debug(`Updated canonical protoc-gen-openapi to ${PROTOC_GEN_OPENAPI_VERSION}`); | ||
| return getCacheDir(); | ||
| } | ||
|
|
||
| // Download the binary | ||
| const downloadUrl = getDownloadUrl(); | ||
| logger.debug(`Downloading protoc-gen-openapi from ${downloadUrl}`); | ||
|
|
||
| const tmpDownloadPath = AbsoluteFilePath.of(`${versionedPath}.download`); | ||
| try { | ||
| const response = await fetch(downloadUrl, { redirect: "follow" }); | ||
| if (!response.ok) { | ||
| logger.debug(`Failed to download protoc-gen-openapi: ${response.status} ${response.statusText}`); | ||
| return undefined; | ||
| } | ||
|
|
||
| const arrayBuffer = await response.arrayBuffer(); | ||
| await writeFile(tmpDownloadPath, new Uint8Array(arrayBuffer)); | ||
| await chmod(tmpDownloadPath, 0o755); | ||
|
|
||
| // Atomic rename to versioned path | ||
| await rename(tmpDownloadPath, versionedPath); | ||
|
|
||
| // Atomic copy to canonical path + update version marker | ||
| await atomicCopyBinary(versionedPath, canonicalPath); | ||
| await writeFile(versionMarkerPath, PROTOC_GEN_OPENAPI_VERSION); | ||
|
|
||
| logger.debug(`Downloaded protoc-gen-openapi ${PROTOC_GEN_OPENAPI_VERSION}`); | ||
| return getCacheDir(); | ||
| } catch (error) { | ||
| logger.debug( | ||
| `Failed to download protoc-gen-openapi: ${error instanceof Error ? error.message : String(error)}` | ||
| ); | ||
| // Clean up partial download if it exists | ||
| try { | ||
| await rm(tmpDownloadPath, { force: true }); | ||
| } catch (cleanupErr) { | ||
| logger.debug( | ||
| `Failed to clean up partial download: ${cleanupErr instanceof Error ? cleanupErr.message : String(cleanupErr)}` | ||
| ); | ||
| } | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
||
| async function readVersionMarker(markerPath: AbsoluteFilePath, logger: Logger): Promise<string | undefined> { | ||
| try { | ||
| return (await readFile(markerPath, "utf-8")).trim(); | ||
| } catch (err) { | ||
| logger.debug(`Failed to read version marker: ${err instanceof Error ? err.message : String(err)}`); | ||
| return undefined; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Atomically copies src to dest by writing to a temp file first, then renaming. | ||
| * rename() is atomic on the same filesystem, so no other process can observe a | ||
| * partially-written binary. | ||
| */ | ||
| async function atomicCopyBinary(src: AbsoluteFilePath, dest: AbsoluteFilePath): Promise<void> { | ||
| const tmpDest = AbsoluteFilePath.of(`${dest}.tmp`); | ||
| await copyFile(src, tmpDest); | ||
| await chmod(tmpDest, 0o755); | ||
| await rename(tmpDest, dest); | ||
| } | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Catch block in
fileExistssilently swallows errors without logging (REVIEW.md violation)The
catchblock atpackages/cli/workspace/lazy-fern-workspace/src/protobuf/ProtocGenOpenAPIDownloader.ts:96returnsfalsewithout logging the error. REVIEW.md mandates: "No emptycatchblocks -- at minimum log the error." While the block isn't literally empty (it hasreturn false), it does not log the caught error at all. Other catch blocks in this same file (e.g. lines 125-126, 130-132, 143-144, 224-227) properly capture and log the error, making this an inconsistency.Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is intentional —
fileExistsis a thin wrapper aroundfs.accesswhere the expected failure case isENOENT(file not found). Logging every "file not found" check at debug level would be noisy since this function is called frequently in the hot path (checking versioned binary, canonical binary, version marker). The other catch blocks in this file handle truly unexpected errors (download failures, lock failures, cleanup failures), which is why they log.That said, if a human reviewer feels strongly about consistency here, happy to add a debug log.