feat(cli): auto-download protoc-gen-openapi binary when not on PATH

[Swimburger]

Description

Refs fern-api/protoc-gen-openapi#22

When generating SDKs from proto files, the CLI currently requires protoc-gen-openapi to be pre-installed on PATH. In CI, this means installing Go and compiling from source via go install on every run (~31s). This PR adds automatic download of a pre-built binary from GitHub Releases, caching it at ~/.fern/bin/.

The companion PR (fern-api/protoc-gen-openapi#22) is merged and v0.1.13 binaries are published.

Changes Made

• New ProtocGenOpenAPIDownloader.ts: Downloads the protoc-gen-openapi v0.1.13 binary for the current platform/arch from GitHub Releases. Supports linux/darwin/windows on amd64/arm64.
  • Versioning: Binaries cached with versioned filenames (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 in code, the canonical binary is atomically replaced on next invocation.
  • Race-safety: An exclusive filesystem lock (mkdir-based, atomic on all platforms) is held during download and file operations. All file replacements use write-to-temp + rename() for atomicity. Lock force-break handles concurrent EEXIST with retry.
  • Top-level try-catch: resolveProtocGenOpenAPI wraps all logic (including lock acquisition) so unexpected errors return undefined instead of crashing the caller.
• Modified ProtobufOpenAPIGenerator.ts: When protoc-gen-openapi is not on PATH, attempts auto-download before failing. Prepends the cache directory to buf's PATH using path.delimiter (cross-platform).
• New ProtocGenOpenAPIDownloader.test.ts: 12 tests total:
  • 10 unit tests (mocked fetch): fresh download, cache hit, version upgrade, download failure (404 + network), lock release on error, concurrent access, cache dir creation, binary permissions.
  • 2 real e2e tests (no mocking, real network): downloads v0.1.13 binary from GitHub Releases, verifies cache structure/size/permissions/version marker, verifies cache hit on second call, validates binary has correct ELF/Mach-O/PE magic bytes.
• versions.yml: Added v4.35.0 changelog entry.

Review Checklist

• acquireLock can throw through resolveProtocGenOpenAPI — Fixed: top-level try-catch in resolveProtocGenOpenAPI ensures lock errors return undefined gracefully.
• writeFileSync for download — Fixed: now uses async writeFile with new Uint8Array(arrayBuffer).
• Hardcoded version v0.1.13 — pinned in ProtocGenOpenAPIDownloader.ts. Future protoc-gen-openapi releases require a manual CLI version bump. Is this acceptable or should it be configurable?
• No cleanup of old versioned binaries — when the version constant is bumped, old binaries (e.g. protoc-gen-openapi-v0.1.12) remain on disk. Could accumulate over time (~11MB each).
• Lock timeout is 2 minutes — if a process crashes while holding the lock, other processes wait up to 2 minutes before force-breaking. Reasonable for CI; could be surprising in local dev.
• Real e2e tests make network calls — the 2 e2e tests download from GitHub Releases on every CI run (~11MB). Could flake if GitHub is unavailable or rate-limited. 60s timeout configured.
• fileExists catch block has no logging — intentional: it handles expected ENOENT errors on a hot path; logging every file-not-found check would be noisy.

Testing

• Lint passes (pnpm run check)
• Full build passes (pnpm run fern:build)
• Cross-compilation of all 6 platform binaries verified locally (linux/darwin/windows × amd64/arm64)
• 10 unit tests with mocked fetch (no network)
• 2 real e2e tests downloading v0.1.13 from GitHub Releases (verified cache, permissions, magic bytes, cache hit)
• Manual e2e: ran fern check --api csharp-grpc-proto --log-level debug with local CLI build, confirmed download → cache → reuse flow works

Link to Devin session: https://app.devin.ai/sessions/9aa3a5abd77d4e069bcfade48423e986
Requested by: @Swimburger

---

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[github-actions]

🌱 Seed Test Selector

Select languages to run seed tests for:

• Python
• TypeScript
• Java
• Go
• Ruby
• C#
• PHP
• Swift
• Rust
• OpenAPI
• Postman

---

How to use: Click the ⋯ menu above → "Edit" → check the boxes you want → click "Update comment". Tests will run automatically and snapshots will be committed to this PR.

[graphite-app]

Uses hardcoded : as PATH separator, which breaks on Windows. Windows requires ; as the PATH separator.

On Windows systems, this will result in a malformed PATH that won't find the downloaded binary, causing the buf command to fail even though the binary was successfully downloaded.

// Fix: Use platform-specific PATH separator
const pathSeparator = process.platform === 'win32' ? ';' : ':';
const envOverride =
    this.protocGenOpenAPIBinDir != null
        ? { PATH: `${this.protocGenOpenAPIBinDir}${pathSeparator}${process.env.PATH ?? ""}` }
        : undefined;

Suggested change

	? { PATH: `${this.protocGenOpenAPIBinDir}:${process.env.PATH ?? ""}` }
	? { PATH: `${this.protocGenOpenAPIBinDir}${process.platform === 'win32' ? ';' : ':'}${process.env.PATH ?? ""}` }

Spotted by Graphite



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

Critical Race Condition: When multiple CLI processes run concurrently without the cached binary, they all attempt to download to the same temporary file path ${cachedBinaryPath}.tmp. This causes:

1. File corruption from concurrent writes
2. Potential failures when one process tries to rename while another is writing
3. Incomplete or corrupted binary being moved to the final cached location

Fix: Use a unique temporary filename for each process:

const tmpPath = AbsoluteFilePath.of(`${cachedBinaryPath}.tmp.${process.pid}.${Date.now()}`);

Or use a proper temporary file library that guarantees unique names. This ensures each concurrent download uses its own temporary file.

Suggested change

	const tmpPath = AbsoluteFilePath.of(`${cachedBinaryPath}.tmp`);
	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();
	writeFileSync(tmpPath, new Uint8Array(arrayBuffer));
	await chmod(tmpPath, 0o755);
	await rename(tmpPath, cachedBinaryPath);
	const tmpPath = AbsoluteFilePath.of(`${cachedBinaryPath}.tmp.${process.pid}.${Date.now()}`);
	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();
	writeFileSync(tmpPath, new Uint8Array(arrayBuffer));
	await chmod(tmpPath, 0o755);
	await rename(tmpPath, cachedBinaryPath);

Spotted by Graphite



Is this helpful? React 👍 or 👎 to let us know.

[graphite-app]

Race condition in lock breaking logic causes crashes. After the timeout, the code removes the stale lock and immediately tries to acquire it. However, between rm() and mkdir(), another process could acquire the lock first, causing mkdir() to throw an EEXIST error that crashes this process.

// Fix: Retry the normal acquisition loop instead of assuming we get the lock
try {
    await rm(lockPath, { recursive: true });
} catch {
    // Ignore
}
// Retry acquisition instead of assuming we get the lock
return acquireLock(logger); // Or loop back to the start

Suggested change

	// 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 {
	// Ignore — another process may have already removed it
	}
	await mkdir(lockPath, { recursive: false });
	// 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 {
	// Ignore — another process may have already removed it
	}
	// Retry acquisition instead of assuming we get the lock
	return acquireLock(logger);

Spotted by Graphite



Is this helpful? React 👍 or 👎 to let us know.

[devin-ai-integration]

Devin Review found 2 new potential issues.

View 11 additional findings in Devin Review.

[devin-ai-integration]

🟡 Catch block in fileExists silently swallows errors without logging (REVIEW.md violation)

The catch block at packages/cli/workspace/lazy-fern-workspace/src/protobuf/ProtocGenOpenAPIDownloader.ts:96 returns false without logging the error. REVIEW.md mandates: "No empty catch blocks -- at minimum log the error." While the block isn't literally empty (it has return 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.

[devin-ai-integration]

This is intentional — fileExists is a thin wrapper around fs.access where the expected failure case is ENOENT (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.

[devin-ai-integration]

Self-Review

Issues found and fixed (commit b6bdaa0a):

1. writeFileSync → async writeFile (ProtocGenOpenAPIDownloader.ts:212): Was using synchronous writeFileSync (imported separately from "fs") in an otherwise fully async function. This blocks the event loop during the ~11MB binary write. Fixed to use await writeFile(tmpDownloadPath, Buffer.from(arrayBuffer)) and removed the fs sync import entirely.

2. Temp file cleanup on failure (ProtocGenOpenAPIDownloader.ts:227-232): If the download succeeded but chmod/rename failed, the .download temp file was left on disk. Added cleanup in the catch block: await rm(tmpDownloadPath, { force: true }).

3. vi.unstubAllGlobals() not in afterEach (ProtocGenOpenAPIDownloader.test.ts:56): Each test manually called vi.unstubAllGlobals() at the end. If a test threw before reaching that line, global stubs (e.g. fetch) would leak into subsequent tests. Moved to afterEach for reliability.

Observations (no action needed):

• Unrelated changes in diff: The group-listing error message improvement (v4.34.1) was merged in from main via commit 4f6c776. It's a separate feature (PR fix(cli): list available groups in error when no group specified #13665) that landed on main while this branch was open. The merge conflict in versions.yml was resolved correctly.

• Hardcoded version v0.1.12 in tests: Tests reference the version string directly rather than importing it from the source module. This is acceptable since PROTOC_GEN_OPENAPI_VERSION is a private constant — when it's bumped, tests need a corresponding update. Could extract to a shared constant in the future if version bumps become frequent.

• Multiple getPlatformInfo() calls per resolution: getVersionedBinaryPath(), getCanonicalBinaryPath(), and getDownloadUrl() each call getPlatformInfo() independently. Not a bug (the platform doesn't change mid-execution), but could be optimized by computing once and passing through. Low priority since this runs at most once per CLI invocation.