fix(cli): record changelog entries for explicit --version and never wipe changelog.md#17005
fix(cli): record changelog entries for explicit --version and never wipe changelog.md#17005iamnamananand996 wants to merge 3 commits into
Conversation
…d never wipe changelog.md
🤖 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
The PR preserves changelog history and records explicit-version entries across three code paths (local runner, GithubStep, GitHub push/PR). The core logic is sound and well-tested, but there are a few correctness concerns: a subtle regression in ensureChangelogFile when an existing changelog has an entry but no matching version, a potential path-matching bug in getChangelogFilesToPreserve when the source directory is nested, and prependChangelogBlock not deduplicating within its own logic (relies on callers).
- 🟡 2 warning(s)
- 🔵 3 suggestion(s)
| if (existing != null) { | ||
| if (version == null || changelogContainsVersion(existing, version)) { | ||
| // Already written (e.g. by AutoVersionStep) — don't duplicate the entry. | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
🟡 warning
Behavior change: previously, if changelog.md already existed, ensureChangelogFile returned false unconditionally (assumed AutoVersionStep wrote it). Now, if the existing changelog doesn't contain version, it will prepend a new (possibly empty-description) block. In the normal AutoVersionStep flow, AutoVersionStep writes the entry with the version header, so changelogContainsVersion should match and skip — good. But if resolved.newVersion differs from what AutoVersionStep recorded, or timing differs, you could double-write. Confirm version here always matches the header AutoVersionStep used (both use ## [${version}]), otherwise you'll get duplicate blocks.
| const tracked = await repository.listTrackedFiles(); | ||
| const trackedChangelogs = tracked.filter((file) => file.toLowerCase() === "changelog.md"); | ||
| if (trackedChangelogs.length === 0) { | ||
| return []; | ||
| } | ||
| try { | ||
| const sourceFiles = await readdir(sourceDirectory); | ||
| const sourceHasChangelog = sourceFiles.some((file) => file.toLowerCase() === "changelog.md"); |
There was a problem hiding this comment.
🟡 warning
listTrackedFiles() likely returns repo-relative paths (possibly nested), while readdir(sourceDirectory) returns bare filenames of only the top level. The file.toLowerCase() === "changelog.md" match on tracked files only catches a changelog at the repo root, and the source check only looks at the top of sourceDirectory. If the changelog lives in a subdirectory, or the tracked path is like sub/changelog.md, neither comparison matches. Confirm the changelog is always at repo root; otherwise use path.basename and a recursive source scan.
| * so callers can avoid prepending a duplicate block on regeneration. | ||
| */ | ||
| export function changelogContainsVersion(content: string, version: string): boolean { | ||
| return content.includes(`## [${version}]`); |
There was a problem hiding this comment.
🔵 suggestion
changelogContainsVersion uses a naive includes("## [${version}]"). Version 1.0.0 will match a substring of e.g. ## [1.0.0-beta]? No — the ] guards that. But 1.0.0 would NOT match ## [11.0.0]... actually ## [11.0.0] does not contain ## [1.0.0], so fine. Just note it also matches inside code blocks or prose lines that happen to contain that string. Low risk, but a line-anchored regex (^## \[version\]) would be more robust.
| if (current != null) { | ||
| return; | ||
| } | ||
| await writeFile(join(this.absolutePathToLocalOutput, RelativeFilePath.of(prior.filename)), prior.content); |
There was a problem hiding this comment.
🔵 suggestion
restoreChangelogFile writes back using prior.filename (original casing). Good — preserves CHANGELOG.md vs changelog.md. But note prependExplicitVersionChangelogEntry runs immediately after and re-reads via readChangelogFile (case-insensitive), so casing is retained there too. No bug, just confirming the two steps are consistent — they are.
| }; | ||
| return { | ||
| logger: { info: noop, debug: noop, warn: noop, error: noop } | ||
| } as LocalTaskHandler.Init["context"]; |
There was a problem hiding this comment.
🔵 suggestion
The mock context only stubs logger. If copyGeneratedFiles touches any other context method on these paths (e.g. failAndThrow, logger.log), the cast-to-type will let it compile but throw at runtime. Tests pass now, but this stub is brittle — consider a fuller fake or documenting the assumption.
devin-ai-integration
Bot
changed the title
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
SDK Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on Full benchmark table (click to expand)
main (generator): generator-only time via --skip-scripts (includes Docker image build, container startup, IR parsing, and code generation — this is the same Docker-based flow customers use via |
Docs Generation Benchmark ResultsComparing PR branch against median of 5 nightly run(s) on
Docs generation runs |
2 participants
Description
Linear ticket: N/A
fern generate --group <group> --version 0.1.0(explicit version, not AUTO) skipped the autoversioning pipeline entirely, so no changelog entry was written — and regeneration copy steps could wipe an existingchangelog.md, losing version history forever.Now:
## [0.1.0] - <date>with an empty description, prepended to the existing changelog.--version AUTOkeeps its AI-generated changelog flow unchanged.changelog.mdis never wiped: existing entries are always preserved across regenerations.Changes Made
autoversion/changelogUtils.tswithprependChangelogBlock()(prepends a## [version] - dateblock while preserving existing content and the# Changelogtitle) andchangelogContainsVersion()(duplicate-entry guard);AutoVersionStep.prependChangelogEntry()now delegates to it.LocalTaskHandler.copyGeneratedFiles():changelog.md(case-insensitive) before copy operations and restores it if the copy removed it (generators don't emit changelog.md);GithubStep.ensureChangelogFile()now prepends into an existing changelog instead of only creating a missing file, and writes a version-only entry when a new version has no changelog entry (explicit-version runs); duplicate versions are skipped.GitHub.push()/GitHub.pr()(generator-agent flow) restore a trackedchangelog.mdafteroverwriteLocalContentswhen the generated output doesn't include one, so existing entries are never deleted.packages/cli/cli/changes/unreleased/andpackages/generator-cli/changes/unreleased/(type: fix).Testing
changelog-utils.test.ts(prepend/duplicate/format cases) andLocalTaskHandler.explicitVersionChangelog.test.ts(preserve + prepend in git repos, no duplicates on regeneration with same version, restore of wiped changelog, no file created for plain local output)pnpm turbo run test --filter @fern-api/generator-cli --filter @fern-api/local-workspace-runner— all 626 tests pass;pnpm lint:biomeandpnpm formatcleanLink to Devin session: https://app.devin.ai/sessions/1d8601f644ca491592d79696962aa730
Requested by: @iamnamananand996