fix(cli-generator): patch two lockfile advisories and make releases re-runnable - #17472
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}
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
|
||
| const trial = [...selected]; | ||
| trial[index] = replacement; | ||
| if (closeDependencyGraph(trial, previous.packages)) { | ||
| selected.splice(0, selected.length, ...trial); | ||
| } | ||
| } |
There was a problem hiding this comment.
🔴 Generated project can end up with a dependency list cargo rejects, breaking builds
When a customer's newer dependency version no longer needs one of the packages the shipped version needed, that now-unused package is still kept in the generated dependency list (closeDependencyGraph at generators/cli/src/mergeCargoLock.ts:47 only checks for missing entries, never for leftovers), so the produced lockfile no longer matches what the project actually needs.
Impact: Builds and CI in the generated repository that require an exact, unchanged lockfile fail after regeneration until someone regenerates the lockfile by hand.
Merge keeps unreachable package stanzas after a version swap drops a dependency edge
mergeCargoLocks (generators/cli/src/mergeCargoLock.ts:34-53) replaces a template stanza with the customer's higher-version stanza and then validates only completeness: closeDependencyGraph/ensureDependencies (generators/cli/src/mergeCargoLock.ts:81-118) pull in any dependency stanza that is referenced but absent. Nothing prunes stanzas that became unreachable.
Concretely: template has X 1.2.1 depending on Y, and Y has no other dependents. The customer's lock has X 1.2.4 which dropped the Y dependency. The swap is accepted (graph closes) and Y's stanza is still emitted from the template package set. cargo build --locked / cargo metadata --locked re-resolves, finds the lock must change (remove Y), and errors — exactly the class of failure the author guarded against for the additive direction ("orphan stanzas would break cargo metadata --locked"). Dependency removals in patch/minor upgrades are common (the PR's own vendored refresh mentions transitive changes).
A fix would compute reachability from the workspace/local packages (packages with no source) after the swaps and drop unreachable registry stanzas, or abandon a swap whose acceptance orphans a stanza.
Prompt for agents
mergeCargoLocks in generators/cli/src/mergeCargoLock.ts validates a preserved customer upgrade only for graph completeness (closeDependencyGraph/ensureDependencies add missing dependency stanzas pulled from the prior lock). It never removes stanzas that become unreachable when the higher customer version drops a dependency edge that the template version had. The emitted lockfile then contains package entries that cargo's re-resolution would delete, so `cargo build --locked` / `cargo metadata --locked` in the generated repo fails with 'the lock file needs to be updated'. Consider, after all swaps are selected, computing reachability from the local/workspace packages (those without a `source`) over the dependency edges and dropping unreachable registry stanzas — or rejecting a swap that would orphan a stanza. The dependency-reference resolution helpers (parseDependencyReference/findDependency) already exist and can be reused for the traversal.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Agreed — the closure check only validates completeness, so a customer version that dropped a dependency edge leaves the template's now-unreachable stanza in the emitted lock and cargo metadata --locked demands an update. Fixing by computing reachability from the source-less local/workspace packages after selection and dropping unreachable registry stanzas, with a merge test covering a swap that drops an edge.
| for (const dependency of cargoPackage.dependencies) { | ||
| const reference = parseDependencyReference(dependency); | ||
| if (findDependency(selected, reference) != null) { | ||
| continue; | ||
| } | ||
|
|
||
| const previousDependency = findDependency(previousPackages, reference); | ||
| if (previousDependency == null) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
🟡 Customer dependency upgrades are silently thrown away for packages that appear at more than one version
An upgrade the customer made is discarded whenever the package appears at two different versions in the project's dependency list (the graph check at generators/cli/src/mergeCargoLock.ts:99-106 cannot resolve the other packages' version-qualified references to the old version), so the regeneration reverts their fix without any warning.
Impact: For commonly duplicated packages (getrandom, rand, thiserror, windows-sys, security-framework, …) a customer's security patch keeps getting undone on every regeneration — the very problem this change is meant to solve.
Version-qualified dependency references make the closure check fail after the swap
In the vendored generators/cli/sdk/Cargo.lock 19 package names appear at multiple versions, and cargo therefore writes their dependency references version-qualified, e.g. "thiserror 2.0.18", "getrandom 0.3.4".
If the customer bumps thiserror 2.0.18 → 2.0.19, findCompatibleReplacement (generators/cli/src/mergeCargoLock.ts:56-79) picks the 2.0.19 stanza. closeDependencyGraph then walks every selected package: dependents still carry the reference "thiserror 2.0.18", findDependency(selected, {name, version: "2.0.18"}) (generators/cli/src/mergeCargoLock.ts:120-126) finds nothing, and the customer lock has no 2.0.18 stanza either, so ensureDependencies returns false and the swap is abandoned — the template's 2.0.18 is re-emitted and the customer's upgrade is lost.
Worse, if the customer lock happens to still contain the old version (because something else needs it), the closure "succeeds" by re-adding the old stanza, producing three versions of the package with the dependents still pinned to the old one.
Handling this requires rewriting version-qualified dependency references of swapped packages in the dependents' stanzas (or at minimum detecting the case and reporting it rather than silently reverting).
Prompt for agents
In generators/cli/src/mergeCargoLock.ts, when a template registry package is swapped for the customer's higher version, dependents' dependency strings are left untouched. Cargo writes dependency references as "name version" whenever a package name occurs at multiple versions in the lock, and the vendored generators/cli/sdk/Cargo.lock has 19 such names (getrandom, rand, rand_core, thiserror, thiserror-impl, windows-sys, windows-targets, security-framework, core-foundation, ...). For those packages the closure check in closeDependencyGraph/ensureDependencies can no longer resolve "name oldVersion", so the swap is abandoned and the customer's upgrade is silently reverted; if the old stanza still exists in the prior lock the closure instead succeeds while leaving dependents pinned to the old version, yielding an extra duplicate stanza. Consider rewriting version-qualified references to the swapped package inside the dependents' raw stanzas (name+old version -> name+new version) as part of accepting a swap, and/or surfacing a log line when a swap is abandoned so silent reverts are observable.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Confirmed and fixing. Verified against the vendored lock: 19 names appear at multiple versions, and cargo qualifies dependency references for exactly that set and leaves all others bare — so a customer bump of any of those crates currently fails the closure check and gets silently reverted, which is the reversion this PR exists to stop. Accepting a swap will now rewrite the dependents' references to the swapped package and keep the emitted lock in cargo's canonical form (qualified only while a name still has multiple versions in the result, de-qualified when a swap collapses it to one), abandoning the swap if that can't be done unambiguously — plus a log line so abandoned swaps aren't silent. Adding merge tests for the duplicated-name cases and redoing the seed regeneration check with a duplicated-name crate instead of webbrowser.
devin-ai-integration
Bot
changed the title
rishabh-fern
changed the title
769e6c3 to
846722f
Compare
…e-runnable The vendored generators/cli/sdk/Cargo.lock ships verbatim into every generated CLI, so a stale pin there reaches every consumer. Two advisories were live in it: RUSTSEC-2026-0258 (h2 <0.4.16) and RUSTSEC-2026-0257 (webbrowser <1.2.2). Refreshed both, plus the transitive additions webbrowser pulls in. No other package moved. The generated release.yml ended in a bare `gh release create`, so a release that already existed for the tag — cut from the GitHub UI, or a re-run of a failed `host` job — failed the release after every build had already succeeded. The step now checks `gh release view` first and edits + uploads into an existing release, creating only when there is none. `--draft=false` publishes a release drafted in the UI, since `gh release view` finds drafts and without it the assets upload somewhere nobody can see while the job reports success; `--target` puts the tag on the built commit, since a draft has no tag yet. Added the cargo ecosystem to .github/dependabot.yml for the vendored SDK. The repo already covers npm and gomod; this is the gap that let the advisories rot. Preferred over a scheduled cargo-audit job because Dependabot opens a PR with the fix rather than turning a cron red with no owner. An earlier revision also added a Cargo.lock merge inside the generator, to stop regeneration discarding a customer's dependency upgrades. It was dropped: the generator's /fern/output is a freshly mkdir'd temp directory (runGenerator.ts:170-172), not the customer's repo, so it never sees a prior lockfile and the merge is a no-op in every flow. The clobbering happens in the CLI's copy-back step. Letting generators read prior output is a cross-cutting change to the CLI/generator contract and wants an ADR. Co-Authored-By: Claude <noreply@anthropic.com>
846722f to
3fd8f69
Compare
2 participants
Description
Linear ticket: Refs
Two independent fixes to the CLI generator, both hit by ElevenLabs.
Vendored lockfile advisories.
generators/cli/sdk/Cargo.lockships verbatim into every generated CLI, so a stale pin there reaches every consumer. Two advisories were live in it: RUSTSEC-2026-0258 (h2 <0.4.16) and RUSTSEC-2026-0257 (webbrowser <1.2.2).Releases failed when the tag already had a release. The generated
release.ymlended in a baregh release create, so a release cut from the GitHub UI — or a re-run of a failedhostjob — failed after every build had already succeeded. ElevenLabs hit exactly this onv1.0.0-alpha.1.Not in this PR: preserving customer lockfile upgrades
An earlier revision of this PR added a Cargo.lock merge inside the generator, to stop regeneration discarding dependency upgrades a customer had made. It was removed, because it could never have run.
The generator's
/fern/outputis bound to a freshlymkdir'd temp directory (runGenerator.ts:170-172), not the customer's repo; the CLI copies results into the repo afterwards viaLocalTaskHandler.copyGeneratedFiles(), preserving only.fernignorepaths. So the generator never sees a priorCargo.lock,previousCargoLockis alwaysundefined, and the merge is a no-op in every flow. The clobbering happens in the CLI's copy step, not incopySdk.Shipping it would have closed the ticket while leaving the bug live. The work is preserved on the branch history if the "generators can see prior output" question is ever taken up — that is a cross-cutting change to the CLI/generator contract and wants an ADR, not a rider on this PR.
The available fix today is customer-side: add
Cargo.lockto.fernignore. Tradeoff is that the generated-crate stanzas then go stale when the API adds a dependency, surfacing as a loudcargo build --lockedfailure fixed by onecargo update— versus today's silent security regression.Changes Made
generators/cli/sdk/Cargo.lock:h2 0.4.15 -> 0.4.16,webbrowser 1.2.1 -> 1.2.4, plus the transitive additions webbrowser pulls in (objc2-app-kit,objc2-core-foundation,dispatch2). No other package moved.emitReleaseWorkflow: theCreate GitHub Releasestep now checksgh release viewfirst. If a release exists it edits title/notes/prerelease andgh release upload --clobbers the artifacts in; otherwise it creates as before.--draft=falsepublishes a release drafted in the UI (gh release viewfinds drafts, so without it assets upload into something nobody can see while the job reports success), and--targetputs the tag on the built commit, since a draft has no tag yet.cargoecosystem to.github/dependabot.ymlfor/generators/cli/sdk. The repo already covers npm and gomod; this closes the gap that let the advisories rot. Preferred over a bespoke scheduledcargo auditjob because Dependabot opens a PR with the fix rather than turning a cron red with no owner.Testing
emitReleaseWorkflow.test.tscovers the existing-release branch and asserts everygh release editcarries--target. Full suite: 414 passing.Lockfile. In
generators/cli/sdk:cargo metadata --locked,cargo fetch --locked(validates every checksum), andcargo build --locked --all-featuresall pass.Release workflow — verified end to end on a real release, not just asserted. Generated a CLI from the ElevenLabs spec with a locally built image into a scratch repo, then cut
v0.0.5from the GitHub UI. The release was created at 02:14; the workflow started at 02:26:39 — sohosthit an already-existing release and took theedit+upload --clobberbranch, which is the exact case that used to fail. All 7 build targets green, 21 assets attached,targetmatching the built commit, andpublish-homebrew-formula/publish-scoopboth published.The emitted script was also executed directly against a stubbed
ghacross all branches — fresh tag stable/prerelease, existing release stable/prerelease, and upload failure — confirming correct dispatch, argument quoting (titles containing"and$arrive as one argument), and that a failed upload fails the job. Flag syntax checked against realgh2.95:--prerelease=falseparses as a bool, while--prerelease=notabooland--drafttboth fail at parse time.Note for reviewers
The 48 committed
seed/cli/*/Cargo.lockfixtures are stale against the refreshed lock and need regenerating (pnpm seed test --generator cli) plus a seed image rebuild.