feat(cli): rename --pack to --package and add --package-only - #17307
Conversation
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
![nitpickybot[bot]](https://avatars.githubusercontent.com/in/2901896?s=60&v=4){kind=small}
There was a problem hiding this comment.
AI Review Summary
Renames --pack/--pack-mode to --package/--package-mode and adds --package-only, which wipes everything except fern-dist/ in the output directory. Plumbing and tests look fine, but the new destructive cleanup deserves guardrails: it recursively deletes arbitrary user-configured output directories, and a cleanup failure is currently reported as a packaging failure. The changelog also classifies a breaking flag rename as feat.
- 🔴 1 critical issue(s)
- 🟡 2 warning(s)
- 🔵 1 suggestion(s)
| for (const entry of await readdir(outputPath)) { | ||
| if (entry === PACK_OUTPUT_DIRECTORY) { | ||
| continue; | ||
| } | ||
| await rm(join(outputPath, RelativeFilePath.of(entry)), { recursive: true, force: true }); | ||
| } |
There was a problem hiding this comment.
🔴 critical
This recursively force-deletes every entry in a user-configured output-path. If someone points a local generator at a directory that isn't exclusively generator output (a checked-out SDK repo with .git, a folder with hand-written files, ./), --package-only silently destroys it — no dry run, no confirmation, no undo.
At minimum, refuse to run when the directory looks like it contains non-generated content (e.g. a .git directory) or preserve dotfiles, and log which entries are being removed at debug level. A safety check against outputPath resolving to a repo root / home dir would also be cheap insurance.
| if (packOnly) { | ||
| await removeEverythingExceptDist({ outputPath, context }); | ||
| } |
There was a problem hiding this comment.
🟡 warning
The cleanup is inside the packaging try, so a failed rm (very plausible in --package-mode docker, where artifacts can be root-owned — force: true doesn't help with EACCES) is reported as "Failed to package …" even though the artifact built fine, and the whole group fails. Separate the concerns:
| if (packOnly) { | |
| await removeEverythingExceptDist({ outputPath, context }); | |
| } | |
| } | |
| if (packOnly) { | |
| try { | |
| await removeEverythingExceptDist({ outputPath, context }); | |
| } catch (error) { | |
| const message = error instanceof Error ? error.message : String(error); | |
| context.logger.warn( | |
| `Packaged ${generator.name} but failed to remove generated source at ${outputPath}: ${message}` | |
| ); | |
| } | |
| } |
(Note: this suggestion assumes the try/catch block is restructured so the catch precedes it — adjust placement accordingly.)
| `--package-mode`, and add `--package-only`, which builds the distributable artifact into | ||
| `fern-dist/` and removes the generated SDK source, leaving only the package in the output | ||
| directory. | ||
| type: feat |
There was a problem hiding this comment.
🟡 warning
Removing --pack/--pack-mode with no alias is a breaking CLI change; type: feat will bury it in a minor release note. Use the breaking type (break) so users who scripted against --pack actually see it — or keep hidden aliases for one release.
| return cliContext.failWithoutThrowing("The --pack flag cannot be used with --preview.", undefined, { | ||
| const shouldPackage = argv.package || argv.packageOnly; | ||
| if (shouldPackage && argv.preview) { | ||
| return cliContext.failWithoutThrowing("The --package flag cannot be used with --preview.", undefined, { |
There was a problem hiding this comment.
🔵 suggestion
When the user passed --package-only, these messages say --package, which they never typed. Consider deriving the flag name from which option was set, e.g. const packageFlag = argv.packageOnly ? "--package-only" : "--package"; and interpolating it into the three validation messages.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| if (packOnly) { | ||
| await removeEverythingExceptDist({ outputPath, context }); | ||
| } |
There was a problem hiding this comment.
🔴 Swift SDK output is completely erased when only the package is requested
The whole generated Swift output folder is wiped (removeEverythingExceptDist at packages/cli/cli/src/commands/generate/packLocalOutput.ts:68-70) even though no shareable package was ever produced for Swift, so the user is left with an empty directory and no artifact.
Impact: Anyone running Swift generation with the package-only option loses their entire generated SDK and gets nothing back.
Swift packaging is a no-op warning, so nothing is created in fern-dist before deletion
In packOutputForLanguage, the swift branch (packages/cli/cli/src/commands/generate/packLocalOutput.ts:174-179) only logs a warning and returns — it never creates fern-dist/ nor any artifact. Because it doesn't throw, the packOnly path immediately runs removeEverythingExceptDist, which deletes every entry in the output dir (no fern-dist entry exists to skip), leaving the directory empty while logging "only fern-dist/ remains".
A fix would be to make packaging report whether an artifact was actually produced (or to skip the cleanup for languages with no package format), and only remove the source when fern-dist/ contains at least one artifact.
Prompt for agents
In packages/cli/cli/src/commands/generate/packLocalOutput.ts, when packOnly is set, removeEverythingExceptDist is called unconditionally after packOutputForLanguage resolves. For the `swift` language, packOutputForLanguage just logs a warning and returns without creating fern-dist/ or any artifact, so the cleanup deletes the entire generated output directory and leaves nothing behind (while logging that only fern-dist/ remains). Consider having packOutputForLanguage signal whether an artifact was produced (e.g. return a boolean or throw/skip for languages with no package format), or verify that fern-dist/ exists and is non-empty before removing anything, so a no-op packaging step can never wipe the generated source.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in a1bfc44: packOutputForLanguage now returns whether an artifact was produced (false for Swift), and --package-only only removes source when one was — otherwise it warns and keeps the generated output. Covered by a new test.
| for (const entry of await readdir(outputPath)) { | ||
| if (entry === PACK_OUTPUT_DIRECTORY) { | ||
| continue; | ||
| } | ||
| await rm(join(outputPath, RelativeFilePath.of(entry)), { recursive: true, force: true }); | ||
| } | ||
| context.logger.info(`Removed generated SDK source from ${outputPath}; only ${PACK_OUTPUT_DIRECTORY}/ remains.`); |
There was a problem hiding this comment.
🟡 Hand-written and version-control files in the output directory are deleted along with generated code
Every entry in the output directory is deleted (rm(..., { recursive: true, force: true }) at packages/cli/cli/src/commands/generate/packLocalOutput.ts:286), including files the user authored and asked to preserve as well as version-control data, so work unrelated to the generated SDK is lost.
Impact: Users can permanently lose custom code and repository history that lived next to the generated SDK.
Mechanism: cleanup does not exclude preserved or VCS entries
removeEverythingExceptDist (packages/cli/cli/src/commands/generate/packLocalOutput.ts:282-288) iterates all entries of the output dir and skips only PACK_OUTPUT_DIRECTORY. Local-file-system generation intentionally preserves files listed in the output directory's .fernignore (custom, hand-written code), and output directories are commonly checked-in repos containing .git. Note that zipDirectory deliberately excludes .git from artifacts (packages/cli/cli/src/commands/generate/packLocalOutput.ts:245-247), showing that .git is not considered generated content, yet the cleanup removes it. Excluding .git (and, ideally, .fernignore-protected paths) would keep the flag's intent while avoiding destroying non-generated data.
Prompt for agents
removeEverythingExceptDist in packages/cli/cli/src/commands/generate/packLocalOutput.ts deletes every entry of the generator's output directory except fern-dist/. This also removes .git (output directories are often git checkouts) and files preserved via .fernignore (hand-written custom code that local generation intentionally keeps). Consider excluding .git — consistent with zipDirectory, which already skips .git when building the source zip — and consider honoring the output directory's .fernignore entries so only generated SDK source is removed.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Fixed in a1bfc44: the cleanup now always preserves .git, .fernignore, and every top-level path covered by a .fernignore entry (nested/glob entries are handled conservatively by keeping their whole top-level directory). Covered by a new test.
…ve .git and .fernignore paths
2 participants
Description
Renames the packaging flags on
fern generateand adds an artifact-only mode:--pack→--package,--pack-mode→--package-mode(no aliases kept;--packshipped very recently and known users are migrating their scripts).--package-only: after generating and packaging, everything in the local output directory exceptfern-dist/is deleted, so only the shareable artifact remains — no SDK source is left behind.fern generate --group node-sdk-local --version 0.0.1 --package-only --package-mode docker # → SDKs/twilio-core-ts-sdk/ contains only fern-dist/twilio-core-0.0.1.tgzChanges Made
cli.ts:package,package-mode,package-onlyoptions;--package-onlyimplies--package(shouldPackage = argv.package || argv.packageOnly); validation messages updated.packLocalOutputForGrouptakespackOnly; after a generator's packaging succeeds,removeEverythingExceptDistremoves every entry in that output dir exceptfern-dist/(per-generator, so a failed sibling keeps its source for debugging).packOnlythroughgenerateAPIWorkspaces/generateWorkspace.packages/cli/cli/changes/unreleased/.Testing
packOnlyleaves onlyfern-dist/; default keeps source); 10/10 pass.generate --group go-sdk-local --package-onlyagainst fern-demo/twilio-core-fern-config: output dir ends up containing onlyfern-dist/twilio-core-go-sdk-source.zip, with log "Removed generated SDK source from …; only fern-dist/ remains."Link to Devin session: https://app.devin.ai/sessions/0076a65e5b6743f78ba15496f2573f41
Requested by: @iamnamananand996