From 2042d4810896ad7afd804cdf874491cf34e70ca2 Mon Sep 17 00:00:00 2001 From: Dominic Griesel Date: Wed, 6 May 2026 09:24:37 +0200 Subject: [PATCH 1/7] feat: restore working tree when release fails locally --- packages/core/src/index.ts | 1 + packages/core/src/lib/context.ts | 28 +++ packages/core/src/lib/planner.ts | 10 + packages/core/src/lib/rollback.test.ts | 314 +++++++++++++++++++++++++ packages/core/src/lib/rollback.ts | 198 ++++++++++++++++ packages/core/src/types.ts | 2 +- packages/plugin-git/src/index.test.ts | 50 ++++ packages/plugin-git/src/index.ts | 11 + packages/release-script/src/index.ts | 38 ++- 9 files changed, 648 insertions(+), 4 deletions(-) create mode 100644 packages/core/src/lib/rollback.test.ts create mode 100644 packages/core/src/lib/rollback.ts diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 9ba7813..8fdd785 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -3,5 +3,6 @@ export { pathExists, readJson, writeJson } from "./lib/fs.js"; export * from "./lib/error.js"; export * from "./lib/exec.js"; export { execute, resolvePlugins } from "./lib/planner.js"; +export { captureRollbackSnapshot, finalizeRollback } from "./lib/rollback.js"; export { DefaultStages } from "./lib/stage.js"; export type * from "./types.js"; diff --git a/packages/core/src/lib/context.ts b/packages/core/src/lib/context.ts index f1636ee..572cb1e 100644 --- a/packages/core/src/lib/context.ts +++ b/packages/core/src/lib/context.ts @@ -43,4 +43,32 @@ export interface Context { getData(key: string): T; hasData(key: string): boolean; setData(key: string, value: any): void; + + /** + * State used to roll back local changes if the release fails. + * Populated by `captureRollbackSnapshot` once the edit stage is about to run. + */ + rollback?: RollbackState; +} + +export interface RollbackState { + /** SHA of HEAD before any release-related modifications were made */ + originalHead: string; + /** + * SHA of the stash commit that holds the user's pre-release uncommitted + * changes (only set when the working tree was dirty, i.e. with --all). + */ + stashSha?: string; + /** + * Unique message used to identify the snapshot stash in `git stash list`, + * since `git stash drop` requires a `stash@{N}` ref rather than a SHA. + */ + stashMessage?: string; + /** Set to true once the push stage starts. Disables rollback afterwards. */ + pushAttempted: boolean; + /** + * Name of the release tag created during this run (e.g. "v1.2.3"). Only set + * after `git tag` succeeds, so rollback never deletes a pre-existing tag. + */ + createdTag?: string; } diff --git a/packages/core/src/lib/planner.ts b/packages/core/src/lib/planner.ts index 12e54c0..d4ee72c 100644 --- a/packages/core/src/lib/planner.ts +++ b/packages/core/src/lib/planner.ts @@ -2,6 +2,7 @@ import { ReleaseError } from "../index.js"; import type { Context } from "./context.js"; import { GraphNode, topologicalSort } from "./graph.js"; import type { Plugin } from "./plugin.js"; +import { captureRollbackSnapshot } from "./rollback.js"; import { DefaultStages, type Stage } from "./stage.js"; /** Resolve all plugins that are required by the chosen plugins */ @@ -165,8 +166,17 @@ export async function execute(context: Context): Promise { ), ); } + let snapshotTaken = false; for (const stage of stages) { context.cli.prefix = `${stage.id}`; + // Snapshot the pre-release state before the first stage that may mutate + // the working tree. The `check` stage is read-only by convention; any + // later stage (including exec hooks like `after_check` / `before_edit`) + // might write files, so we capture before any of those run. + if (!snapshotTaken && stage.id !== "check") { + await captureRollbackSnapshot(context); + snapshotTaken = true; + } const plugins = await planStage(context, stage); if (context.argv.verbose) { context.cli.log( diff --git a/packages/core/src/lib/rollback.test.ts b/packages/core/src/lib/rollback.test.ts new file mode 100644 index 0000000..bb040c6 --- /dev/null +++ b/packages/core/src/lib/rollback.test.ts @@ -0,0 +1,314 @@ +import { createMockContext } from "@alcalzone/release-script-testing"; +import { describe, expect, it } from "vitest"; +import { captureRollbackSnapshot, finalizeRollback } from "./rollback.js"; + +const HEAD_SHA = "1111111111111111111111111111111111111111"; +const STASH_SHA = "2222222222222222222222222222222222222222"; +const STASH_MSG = "release-script-rollback-12345"; + +// MockSystem is shared across tests via the default context. Reset call +// history (and any previous mockExec implementation) before each test so +// "not called" assertions are accurate. +function freshContext(...args: Parameters) { + const context = createMockContext(...args); + context.sys.exec.mockReset(); + context.sys.execRaw.mockReset(); + return context; +} + +describe("captureRollbackSnapshot", () => { + it("records HEAD when the working tree is clean", async () => { + const context = freshContext({}); + context.sys.mockExec({ + "git rev-parse HEAD": HEAD_SHA, + "git status --porcelain": "", + }); + + await captureRollbackSnapshot(context); + + expect(context.rollback).toBeDefined(); + expect(context.rollback?.originalHead).toBe(HEAD_SHA); + expect(context.rollback?.stashSha).toBeUndefined(); + expect(context.rollback?.stashMessage).toBeUndefined(); + expect(context.rollback?.pushAttempted).toBe(false); + }); + + it("creates and re-applies a stash when the working tree is dirty", async () => { + const context = freshContext({}); + context.sys.mockExec((cmd) => { + if (cmd === "git rev-parse HEAD") return HEAD_SHA; + if (cmd === "git status --porcelain") return " M package.json\n"; + if (cmd.startsWith("git stash push")) return ""; + if (cmd === "git rev-parse stash@{0}") return STASH_SHA; + if (cmd === `git stash apply ${STASH_SHA}`) return ""; + throw new Error(`unexpected command: ${cmd}`); + }); + + await captureRollbackSnapshot(context); + + expect(context.rollback?.stashSha).toBe(STASH_SHA); + expect(context.rollback?.stashMessage).toMatch(/^release-script-rollback-/); + // Stash apply must be called so plugin edits + user edits coexist before commit + expect(context.sys.exec).toHaveBeenCalledWith( + "git", + ["stash", "apply", STASH_SHA], + expect.anything(), + ); + }); + + it("does nothing when --dryRun is set", async () => { + const context = freshContext({ argv: { dryRun: true } }); + await captureRollbackSnapshot(context); + expect(context.rollback).toBeUndefined(); + expect(context.sys.exec).not.toHaveBeenCalled(); + }); + + it("does nothing when --no-rollback is set", async () => { + const context = freshContext({ argv: { noRollback: true } }); + await captureRollbackSnapshot(context); + expect(context.rollback).toBeUndefined(); + expect(context.sys.exec).not.toHaveBeenCalled(); + }); + + it("silently skips when not in a git repo", async () => { + const context = freshContext({}); + context.sys.mockExec(() => { + throw new Error("fatal: not a git repository"); + }); + await captureRollbackSnapshot(context); + expect(context.rollback).toBeUndefined(); + }); +}); + +describe("finalizeRollback (failure path)", () => { + it("is a no-op when no snapshot was captured", async () => { + const context = freshContext({}); + await finalizeRollback(context, { failed: true }); + expect(context.sys.exec).not.toHaveBeenCalled(); + }); + + it("resets HEAD and cleans untracked files when no tag and no stash", async () => { + const context = freshContext({}); + context.rollback = { originalHead: HEAD_SHA, pushAttempted: false }; + context.sys.mockExec(() => ""); + + await finalizeRollback(context, { failed: true }); + + expect(context.sys.exec).toHaveBeenCalledWith( + "git", + ["reset", "--hard", HEAD_SHA], + expect.anything(), + ); + expect(context.sys.exec).toHaveBeenCalledWith("git", ["clean", "-fd"], expect.anything()); + }); + + it("deletes the release tag only if this run created it", async () => { + const context = freshContext({}); + context.rollback = { + originalHead: HEAD_SHA, + pushAttempted: false, + createdTag: "v1.2.3", + }; + context.sys.mockExec(() => ""); + + await finalizeRollback(context, { failed: true }); + + expect(context.sys.exec).toHaveBeenCalledWith( + "git", + ["tag", "-d", "v1.2.3"], + expect.anything(), + ); + }); + + it("does NOT delete a pre-existing tag when this run did not create one", async () => { + const context = freshContext({}); + context.rollback = { originalHead: HEAD_SHA, pushAttempted: false }; + // version_new is set but createdTag is not — tag pre-existed. + context.setData("version_new", "1.2.3"); + context.sys.mockExec(() => ""); + + await finalizeRollback(context, { failed: true }); + + expect(context.sys.exec).not.toHaveBeenCalledWith( + "git", + ["tag", "-d", "v1.2.3"], + expect.anything(), + ); + }); + + it("restores the user's stash and drops it by index after a clean rollback", async () => { + const context = freshContext({}); + context.rollback = { + originalHead: HEAD_SHA, + stashSha: STASH_SHA, + stashMessage: STASH_MSG, + pushAttempted: false, + }; + context.sys.mockExec((cmd) => { + if (cmd === "git stash list --pretty=format:%gd %gs") { + return `stash@{0} On main: ${STASH_MSG}`; + } + return ""; + }); + + await finalizeRollback(context, { failed: true }); + + expect(context.sys.exec).toHaveBeenCalledWith( + "git", + ["stash", "apply", STASH_SHA], + expect.anything(), + ); + // Drop must use the stash@{N} index, not the SHA + expect(context.sys.exec).toHaveBeenCalledWith( + "git", + ["stash", "drop", "stash@{0}"], + expect.anything(), + ); + expect(context.sys.exec).not.toHaveBeenCalledWith( + "git", + ["stash", "drop", STASH_SHA], + expect.anything(), + ); + }); + + it("does NOT drop the stash if applying it failed", async () => { + const context = freshContext({}); + context.rollback = { + originalHead: HEAD_SHA, + stashSha: STASH_SHA, + stashMessage: STASH_MSG, + pushAttempted: false, + }; + context.sys.mockExec((cmd) => { + if (cmd === `git stash apply ${STASH_SHA}`) { + throw new Error("conflict"); + } + if (cmd === "git stash list --pretty=format:%gd %gs") { + return `stash@{0} On main: ${STASH_MSG}`; + } + return ""; + }); + + await finalizeRollback(context, { failed: true }); + + expect(context.sys.exec).not.toHaveBeenCalledWith( + "git", + ["stash", "drop", "stash@{0}"], + expect.anything(), + ); + expect(context.warnings.some((w) => /uncommitted changes/i.test(w))).toBe(true); + }); + + it("drops the snapshot stash even when push was already attempted", async () => { + const context = freshContext({}); + context.rollback = { + originalHead: HEAD_SHA, + stashSha: STASH_SHA, + stashMessage: STASH_MSG, + pushAttempted: true, + }; + context.sys.mockExec((cmd) => { + if (cmd === "git stash list --pretty=format:%gd %gs") { + return `stash@{0} On main: ${STASH_MSG}`; + } + return ""; + }); + + await finalizeRollback(context, { failed: true }); + + // Did not roll back HEAD + expect(context.sys.exec).not.toHaveBeenCalledWith( + "git", + ["reset", "--hard", HEAD_SHA], + expect.anything(), + ); + // But did clean up the stash (its contents are in the local commit) + expect(context.sys.exec).toHaveBeenCalledWith( + "git", + ["stash", "drop", "stash@{0}"], + expect.anything(), + ); + expect(context.warnings.some((w) => /push.*already.*attempted/i.test(w))).toBe(true); + }); + + it("does nothing when --dryRun is set", async () => { + const context = freshContext({ argv: { dryRun: true } }); + context.rollback = { originalHead: HEAD_SHA, pushAttempted: false }; + await finalizeRollback(context, { failed: true }); + expect(context.sys.exec).not.toHaveBeenCalled(); + }); + + it("does nothing when --no-rollback is set", async () => { + const context = freshContext({ argv: { noRollback: true } }); + context.rollback = { originalHead: HEAD_SHA, pushAttempted: false }; + await finalizeRollback(context, { failed: true }); + expect(context.sys.exec).not.toHaveBeenCalled(); + }); +}); + +describe("finalizeRollback (success path)", () => { + it("drops the snapshot stash by index", async () => { + const context = freshContext({}); + context.rollback = { + originalHead: HEAD_SHA, + stashSha: STASH_SHA, + stashMessage: STASH_MSG, + pushAttempted: false, + }; + context.sys.mockExec((cmd) => { + if (cmd === "git stash list --pretty=format:%gd %gs") { + return `stash@{2} On main: unrelated\nstash@{1} On main: ${STASH_MSG}\nstash@{0} On main: other`; + } + return ""; + }); + + await finalizeRollback(context, { failed: false }); + + expect(context.sys.exec).toHaveBeenCalledWith( + "git", + ["stash", "drop", "stash@{1}"], + expect.anything(), + ); + // Must not touch HEAD or the working tree on the success path + expect(context.sys.exec).not.toHaveBeenCalledWith( + "git", + ["reset", "--hard", HEAD_SHA], + expect.anything(), + ); + }); + + it("is a no-op when no snapshot was taken", async () => { + const context = freshContext({}); + await finalizeRollback(context, { failed: false }); + expect(context.sys.exec).not.toHaveBeenCalled(); + }); + + it("is a no-op when no stash was created (clean working tree)", async () => { + const context = freshContext({}); + context.rollback = { originalHead: HEAD_SHA, pushAttempted: false }; + await finalizeRollback(context, { failed: false }); + expect(context.sys.exec).not.toHaveBeenCalled(); + }); + + it("is a no-op when --dryRun or --no-rollback is set", async () => { + const dry = freshContext({ argv: { dryRun: true } }); + dry.rollback = { + originalHead: HEAD_SHA, + stashSha: STASH_SHA, + stashMessage: STASH_MSG, + pushAttempted: false, + }; + await finalizeRollback(dry, { failed: false }); + expect(dry.sys.exec).not.toHaveBeenCalled(); + + const noRb = freshContext({ argv: { noRollback: true } }); + noRb.rollback = { + originalHead: HEAD_SHA, + stashSha: STASH_SHA, + stashMessage: STASH_MSG, + pushAttempted: false, + }; + await finalizeRollback(noRb, { failed: false }); + expect(noRb.sys.exec).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/core/src/lib/rollback.ts b/packages/core/src/lib/rollback.ts new file mode 100644 index 0000000..cc5f1db --- /dev/null +++ b/packages/core/src/lib/rollback.ts @@ -0,0 +1,198 @@ +import type { Context } from "./context.js"; + +const ROLLBACK_STASH_MESSAGE_PREFIX = "release-script-rollback-"; + +async function execGit( + context: Context, + args: string[], +): Promise<{ stdout: string; stderr: string }> { + const result = await context.sys.exec("git", args, { cwd: context.cwd }); + return { stdout: result.stdout ?? "", stderr: result.stderr ?? "" }; +} + +/** Look up the `stash@{N}` ref of a stash entry by its message. */ +async function findStashIndexByMessage( + context: Context, + stashMessage: string, +): Promise { + try { + const { stdout } = await execGit(context, ["stash", "list", "--pretty=format:%gd %gs"]); + for (const line of stdout.split(/\r?\n/)) { + if (!line) continue; + const match = /^(stash@\{\d+\})\s/.exec(line); + if (match && line.includes(stashMessage)) { + return match[1]; + } + } + } catch { + /* ignore */ + } + return undefined; +} + +async function dropRollbackStash(context: Context, state: RollbackStateInternal): Promise { + if (!state.stashMessage) return; + const index = await findStashIndexByMessage(context, state.stashMessage); + if (!index) { + // Already gone or never created — nothing to do. + return; + } + try { + await execGit(context, ["stash", "drop", index]); + } catch (e: any) { + context.cli.warn(`Failed to drop rollback stash ${index}: ${e?.message ?? e}`); + } +} + +// Local alias to avoid an import cycle and keep call sites readable. +type RollbackStateInternal = NonNullable; + +/** + * Captures the state required to roll back any release-induced changes. + * Should be called once, just before the first stage that mutates the working tree. + */ +export async function captureRollbackSnapshot(context: Context): Promise { + if (context.argv.dryRun) return; + if (context.argv.noRollback) return; + if (context.rollback) return; // already captured + + let originalHead: string; + try { + const { stdout } = await execGit(context, ["rev-parse", "HEAD"]); + originalHead = stdout.trim(); + } catch { + // Not a git repo (or git unavailable) — skip rollback support entirely. + return; + } + + let stashSha: string | undefined; + let stashMessage: string | undefined; + try { + const { stdout } = await execGit(context, ["status", "--porcelain"]); + const isDirty = stdout.trim() !== ""; + if (isDirty) { + stashMessage = `${ROLLBACK_STASH_MESSAGE_PREFIX}${Date.now()}`; + await execGit(context, ["stash", "push", "--include-untracked", "-m", stashMessage]); + const { stdout: stashRef } = await execGit(context, ["rev-parse", "stash@{0}"]); + stashSha = stashRef.trim(); + // Re-apply the stash so the working tree is restored. The stash entry + // itself stays in the stash list as a recovery snapshot. + await execGit(context, ["stash", "apply", stashSha]); + } + } catch (e: any) { + context.cli.warn( + `Failed to snapshot uncommitted changes for rollback: ${e?.message ?? e}. ` + + `Rollback will still undo committed and edited files.`, + ); + // Don't carry a half-baked stash forward. + stashSha = undefined; + stashMessage = undefined; + } + + context.rollback = { + originalHead, + stashSha, + stashMessage, + pushAttempted: false, + }; +} + +export interface FinalizeRollbackOptions { + /** + * Whether the release failed. If true, the working tree, HEAD, and any + * release tag are reverted. If false, only the snapshot stash is dropped. + */ + failed: boolean; +} + +/** + * Single entry point for the rollback lifecycle, called once after the release + * finishes (success or failure). Acts as the inverse of `captureRollbackSnapshot`. + */ +export async function finalizeRollback( + context: Context, + options: FinalizeRollbackOptions, +): Promise { + if (context.argv.dryRun) return; + if (context.argv.noRollback) return; + + const state = context.rollback; + if (!state) return; + + if (!options.failed) { + // Success path: the snapshot stash (if any) holds changes that are now + // part of the release commit, so just drop it to keep `git stash list` + // clean. + await dropRollbackStash(context, state); + return; + } + + if (state.pushAttempted) { + context.cli.warn( + `Skipping rollback because a push to the remote has already been attempted. ` + + `Local commit and tag have been kept so you can decide how to proceed.`, + ); + // The user's pre-release changes (if any) are already in the local + // release commit, so the snapshot stash is no longer useful. + await dropRollbackStash(context, state); + return; + } + + context.cli.log("Rolling back local changes..."); + + // Only delete the tag if this run actually created it. Tags that pre-date + // this attempt (e.g. left over from a previous failed run) must be left + // alone for the user to inspect. + if (state.createdTag) { + try { + await execGit(context, ["tag", "-d", state.createdTag]); + context.cli.log(`Deleted release tag ${state.createdTag}`); + } catch (e: any) { + context.cli.warn( + `Failed to delete release tag ${state.createdTag}: ${e?.message ?? e}`, + ); + } + } + + // Reset HEAD and working tree to the snapshot commit + try { + await execGit(context, ["reset", "--hard", state.originalHead]); + context.cli.log(`Reset HEAD to ${state.originalHead.slice(0, 8)}`); + } catch (e: any) { + context.cli.warn(`Failed to reset to original HEAD: ${e?.message ?? e}`); + return; + } + + // Drop untracked files left behind by plugins (e.g. .commitmessage). Do not + // pass -x so files ignored by .gitignore (node_modules etc.) are preserved. + try { + await execGit(context, ["clean", "-fd"]); + } catch (e: any) { + context.cli.warn(`Failed to clean untracked files: ${e?.message ?? e}`); + } + + // Restore the user's pre-release uncommitted changes from the stash snapshot + let stashApplied = false; + if (state.stashSha) { + try { + await execGit(context, ["stash", "apply", state.stashSha]); + stashApplied = true; + context.cli.log("Restored your pre-release uncommitted changes"); + } catch (e: any) { + const recoveryHint = state.stashMessage + ? `Recover them manually with: git stash list (look for "${state.stashMessage}")` + : `Recover them manually via the stash list.`; + context.cli.warn( + `Failed to restore pre-release uncommitted changes: ${e?.message ?? e}. ` + + recoveryHint, + ); + } + } + + // Only drop the stash entry if we successfully restored its contents (or if + // there was nothing to restore). Keep it around if apply failed so the user + // can recover manually. + if (!state.stashSha || stashApplied) { + await dropRollbackStash(context, state); + } +} diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index d55eba8..5bf3c2e 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -1,5 +1,5 @@ export type { CLI, SelectOption } from "./lib/cli.js"; -export type { Context } from "./lib/context.js"; +export type { Context, RollbackState } from "./lib/context.js"; export type { Plugin } from "./lib/plugin.js"; export type { ConstOrDynamic } from "./lib/shared.js"; export type { Stage } from "./lib/stage.js"; diff --git a/packages/plugin-git/src/index.test.ts b/packages/plugin-git/src/index.test.ts index b5a3bba..1d8baee 100644 --- a/packages/plugin-git/src/index.test.ts +++ b/packages/plugin-git/src/index.test.ts @@ -435,6 +435,56 @@ This is the changelog.`); expect(context.sys.execRaw).toHaveBeenCalledWith(cmd, expect.anything()); } }); + + it("marks rollback.pushAttempted=true when entering the push stage", async () => { + const gitPlugin = new GitPlugin(); + const context = createMockContext({ plugins: [gitPlugin] }); + context.setData("version_new", "1.2.9"); + context.rollback = { + originalHead: "deadbeef", + pushAttempted: false, + }; + context.sys.mockExec(() => ""); + + await gitPlugin.executeStage(context, DefaultStages.push); + + expect(context.rollback.pushAttempted).toBe(true); + }); + + it("does not mark rollback.pushAttempted in dry run", async () => { + const gitPlugin = new GitPlugin(); + const context = createMockContext({ + plugins: [gitPlugin], + argv: { dryRun: true }, + }); + context.setData("version_new", "1.3.0"); + context.rollback = { + originalHead: "deadbeef", + pushAttempted: false, + }; + context.sys.mockExec(() => ""); + + await gitPlugin.executeStage(context, DefaultStages.push); + + expect(context.rollback.pushAttempted).toBe(false); + }); + + it("does not mark rollback.pushAttempted when --noPush is set", async () => { + const gitPlugin = new GitPlugin(); + const context = createMockContext({ + plugins: [gitPlugin], + argv: { noPush: true }, + }); + context.setData("version_new", "1.3.1"); + context.rollback = { + originalHead: "deadbeef", + pushAttempted: false, + }; + + await gitPlugin.executeStage(context, DefaultStages.push); + + expect(context.rollback.pushAttempted).toBe(false); + }); }); describe("cleanup stage", () => { diff --git a/packages/plugin-git/src/index.ts b/packages/plugin-git/src/index.ts index 038b67f..32173f3 100644 --- a/packages/plugin-git/src/index.ts +++ b/packages/plugin-git/src/index.ts @@ -259,6 +259,11 @@ ${context.getData("changelog_new")}`; context.cli.logCommand(cmd, args); if (!context.argv.dryRun) { await context.sys.exec(cmd, args, { cwd: context.cwd }); + // Record successful tag creation so rollback can safely delete it + // without ever touching a pre-existing tag of the same name. + if (cmd === "git" && args[0] === "tag" && context.rollback) { + context.rollback.createdTag = `v${newVersion}`; + } } } } @@ -269,6 +274,12 @@ ${context.getData("changelog_new")}`; return; } + // Once we start pushing to the remote, we can no longer safely roll back + // locally without risking divergence with what the remote has accepted. + if (context.rollback && !context.argv.dryRun) { + context.rollback.pushAttempted = true; + } + const upstream = (context.argv.remote as string | undefined) || (await getUpstream(context)); const [remote, branch] = upstream.split("/", 2); diff --git a/packages/release-script/src/index.ts b/packages/release-script/src/index.ts index 36ee629..e113d71 100644 --- a/packages/release-script/src/index.ts +++ b/packages/release-script/src/index.ts @@ -1,9 +1,10 @@ import { - type CLI as ICLI, type Context, exec, execRaw, execute, + finalizeRollback, + type CLI as ICLI, isReleaseError, type Plugin, ReleaseError, @@ -199,6 +200,12 @@ export async function main(): Promise { description: `Bump and publish all non-private packages in monorepos, even if they didn't change`, default: false, }, + noRollback: { + type: "boolean", + description: + "Do not roll back local changes (file edits, commits, tags) if making the release commit fails", + default: false, + }, }); // We do two-pass parsing: @@ -276,6 +283,9 @@ export async function main(): Promise { }; context.cli = new CLI(context); + let failed = false; + let exitCode: number | undefined; + try { // Initialize plugins for (const plugin of plugins) { @@ -300,7 +310,8 @@ export async function main(): Promise { message += "!"; console.error(); console.error(message); - process.exit(1); + failed = true; + exitCode = 1; } } catch (e: any) { if (isReleaseError(e)) { @@ -323,7 +334,28 @@ export async function main(): Promise { ), ); } - process.exit((e as any).code ?? 1); + failed = true; + exitCode = (e as any).code ?? 1; + } + + try { + await finalizeRollback(context, { failed }); + } catch (rollbackError: any) { + const msg = rollbackError?.stack ?? rollbackError?.message ?? String(rollbackError); + console.error( + prependPrefix( + context.cli.prefix, + colorizeTextAndTags( + `[FATAL] Rollback itself failed: ${msg}`, + colors.red, + colors.bgRed, + ), + ), + ); + } + + if (exitCode !== undefined) { + process.exit(exitCode); } } From 301bdbdf5a12177ff5cae63838ab6ed941d52bc1 Mon Sep 17 00:00:00 2001 From: Dominic Griesel Date: Wed, 6 May 2026 09:46:48 +0200 Subject: [PATCH 2/7] address review feedback --- packages/core/src/lib/context.ts | 7 ++ packages/core/src/lib/rollback.test.ts | 112 +++++++++++++++++++++++-- packages/core/src/lib/rollback.ts | 98 +++++++++++++++++----- packages/plugin-git/src/index.test.ts | 34 +++++++- packages/plugin-git/src/index.ts | 11 ++- 5 files changed, 227 insertions(+), 35 deletions(-) diff --git a/packages/core/src/lib/context.ts b/packages/core/src/lib/context.ts index 572cb1e..0c3bd1d 100644 --- a/packages/core/src/lib/context.ts +++ b/packages/core/src/lib/context.ts @@ -71,4 +71,11 @@ export interface RollbackState { * after `git tag` succeeds, so rollback never deletes a pre-existing tag. */ createdTag?: string; + /** + * Whether `git clean -fd` is safe to run during rollback. False when the + * working tree was dirty at snapshot time but the snapshot stash could not + * be created — running clean in that case would permanently lose the user's + * pre-existing untracked files. + */ + cleanAllowedDuringRollback: boolean; } diff --git a/packages/core/src/lib/rollback.test.ts b/packages/core/src/lib/rollback.test.ts index bb040c6..2efd9cd 100644 --- a/packages/core/src/lib/rollback.test.ts +++ b/packages/core/src/lib/rollback.test.ts @@ -48,6 +48,7 @@ describe("captureRollbackSnapshot", () => { expect(context.rollback?.stashSha).toBe(STASH_SHA); expect(context.rollback?.stashMessage).toMatch(/^release-script-rollback-/); + expect(context.rollback?.cleanAllowedDuringRollback).toBe(true); // Stash apply must be called so plugin edits + user edits coexist before commit expect(context.sys.exec).toHaveBeenCalledWith( "git", @@ -56,6 +57,54 @@ describe("captureRollbackSnapshot", () => { ); }); + it("marks cleanAllowedDuringRollback=true when the working tree is clean", async () => { + const context = freshContext({}); + context.sys.mockExec({ + "git rev-parse HEAD": HEAD_SHA, + "git status --porcelain": "", + }); + + await captureRollbackSnapshot(context); + + expect(context.rollback?.cleanAllowedDuringRollback).toBe(true); + }); + + it("marks cleanAllowedDuringRollback=false and skips stash when push fails on dirty tree", async () => { + const context = freshContext({}); + context.sys.mockExec((cmd) => { + if (cmd === "git rev-parse HEAD") return HEAD_SHA; + if (cmd === "git status --porcelain") return " M package.json\n"; + if (cmd.startsWith("git stash push")) throw new Error("stash push failed"); + throw new Error(`unexpected command: ${cmd}`); + }); + + await captureRollbackSnapshot(context); + + expect(context.rollback).toBeDefined(); + expect(context.rollback?.cleanAllowedDuringRollback).toBe(false); + expect(context.rollback?.stashMessage).toBeUndefined(); + expect(context.rollback?.stashSha).toBeUndefined(); + }); + + it("preserves stashMessage when stash push succeeded but a later step failed", async () => { + const context = freshContext({}); + context.sys.mockExec((cmd) => { + if (cmd === "git rev-parse HEAD") return HEAD_SHA; + if (cmd === "git status --porcelain") return " M package.json\n"; + if (cmd.startsWith("git stash push")) return ""; + if (cmd === "git rev-parse stash@{0}") throw new Error("rev-parse failed"); + throw new Error(`unexpected command: ${cmd}`); + }); + + await captureRollbackSnapshot(context); + + // Stash exists, so we must remember its message for recovery + expect(context.rollback?.stashMessage).toMatch(/^release-script-rollback-/); + expect(context.rollback?.stashSha).toBeUndefined(); + // And cleanAllowedDuringRollback must be true (we have a recoverable backup) + expect(context.rollback?.cleanAllowedDuringRollback).toBe(true); + }); + it("does nothing when --dryRun is set", async () => { const context = freshContext({ argv: { dryRun: true } }); await captureRollbackSnapshot(context); @@ -89,7 +138,11 @@ describe("finalizeRollback (failure path)", () => { it("resets HEAD and cleans untracked files when no tag and no stash", async () => { const context = freshContext({}); - context.rollback = { originalHead: HEAD_SHA, pushAttempted: false }; + context.rollback = { + originalHead: HEAD_SHA, + pushAttempted: false, + cleanAllowedDuringRollback: true, + }; context.sys.mockExec(() => ""); await finalizeRollback(context, { failed: true }); @@ -102,11 +155,38 @@ describe("finalizeRollback (failure path)", () => { expect(context.sys.exec).toHaveBeenCalledWith("git", ["clean", "-fd"], expect.anything()); }); + it("does NOT run git clean when cleanAllowedDuringRollback is false", async () => { + const context = freshContext({}); + context.rollback = { + originalHead: HEAD_SHA, + pushAttempted: false, + cleanAllowedDuringRollback: false, + }; + context.sys.mockExec(() => ""); + + await finalizeRollback(context, { failed: true }); + + // Reset still runs + expect(context.sys.exec).toHaveBeenCalledWith( + "git", + ["reset", "--hard", HEAD_SHA], + expect.anything(), + ); + // But clean does not — pre-existing untracked files might be unrecoverable + expect(context.sys.exec).not.toHaveBeenCalledWith( + "git", + ["clean", "-fd"], + expect.anything(), + ); + expect(context.warnings.some((w) => /untracked files/i.test(w))).toBe(true); + }); + it("deletes the release tag only if this run created it", async () => { const context = freshContext({}); context.rollback = { originalHead: HEAD_SHA, pushAttempted: false, + cleanAllowedDuringRollback: true, createdTag: "v1.2.3", }; context.sys.mockExec(() => ""); @@ -122,7 +202,11 @@ describe("finalizeRollback (failure path)", () => { it("does NOT delete a pre-existing tag when this run did not create one", async () => { const context = freshContext({}); - context.rollback = { originalHead: HEAD_SHA, pushAttempted: false }; + context.rollback = { + originalHead: HEAD_SHA, + pushAttempted: false, + cleanAllowedDuringRollback: true, + }; // version_new is set but createdTag is not — tag pre-existed. context.setData("version_new", "1.2.3"); context.sys.mockExec(() => ""); @@ -143,6 +227,7 @@ describe("finalizeRollback (failure path)", () => { stashSha: STASH_SHA, stashMessage: STASH_MSG, pushAttempted: false, + cleanAllowedDuringRollback: true, }; context.sys.mockExec((cmd) => { if (cmd === "git stash list --pretty=format:%gd %gs") { @@ -178,6 +263,7 @@ describe("finalizeRollback (failure path)", () => { stashSha: STASH_SHA, stashMessage: STASH_MSG, pushAttempted: false, + cleanAllowedDuringRollback: true, }; context.sys.mockExec((cmd) => { if (cmd === `git stash apply ${STASH_SHA}`) { @@ -206,6 +292,7 @@ describe("finalizeRollback (failure path)", () => { stashSha: STASH_SHA, stashMessage: STASH_MSG, pushAttempted: true, + cleanAllowedDuringRollback: true, }; context.sys.mockExec((cmd) => { if (cmd === "git stash list --pretty=format:%gd %gs") { @@ -233,14 +320,22 @@ describe("finalizeRollback (failure path)", () => { it("does nothing when --dryRun is set", async () => { const context = freshContext({ argv: { dryRun: true } }); - context.rollback = { originalHead: HEAD_SHA, pushAttempted: false }; + context.rollback = { + originalHead: HEAD_SHA, + pushAttempted: false, + cleanAllowedDuringRollback: true, + }; await finalizeRollback(context, { failed: true }); expect(context.sys.exec).not.toHaveBeenCalled(); }); it("does nothing when --no-rollback is set", async () => { const context = freshContext({ argv: { noRollback: true } }); - context.rollback = { originalHead: HEAD_SHA, pushAttempted: false }; + context.rollback = { + originalHead: HEAD_SHA, + pushAttempted: false, + cleanAllowedDuringRollback: true, + }; await finalizeRollback(context, { failed: true }); expect(context.sys.exec).not.toHaveBeenCalled(); }); @@ -254,6 +349,7 @@ describe("finalizeRollback (success path)", () => { stashSha: STASH_SHA, stashMessage: STASH_MSG, pushAttempted: false, + cleanAllowedDuringRollback: true, }; context.sys.mockExec((cmd) => { if (cmd === "git stash list --pretty=format:%gd %gs") { @@ -285,7 +381,11 @@ describe("finalizeRollback (success path)", () => { it("is a no-op when no stash was created (clean working tree)", async () => { const context = freshContext({}); - context.rollback = { originalHead: HEAD_SHA, pushAttempted: false }; + context.rollback = { + originalHead: HEAD_SHA, + pushAttempted: false, + cleanAllowedDuringRollback: true, + }; await finalizeRollback(context, { failed: false }); expect(context.sys.exec).not.toHaveBeenCalled(); }); @@ -297,6 +397,7 @@ describe("finalizeRollback (success path)", () => { stashSha: STASH_SHA, stashMessage: STASH_MSG, pushAttempted: false, + cleanAllowedDuringRollback: true, }; await finalizeRollback(dry, { failed: false }); expect(dry.sys.exec).not.toHaveBeenCalled(); @@ -307,6 +408,7 @@ describe("finalizeRollback (success path)", () => { stashSha: STASH_SHA, stashMessage: STASH_MSG, pushAttempted: false, + cleanAllowedDuringRollback: true, }; await finalizeRollback(noRb, { failed: false }); expect(noRb.sys.exec).not.toHaveBeenCalled(); diff --git a/packages/core/src/lib/rollback.ts b/packages/core/src/lib/rollback.ts index cc5f1db..1a98249 100644 --- a/packages/core/src/lib/rollback.ts +++ b/packages/core/src/lib/rollback.ts @@ -65,28 +65,70 @@ export async function captureRollbackSnapshot(context: Context): Promise { return; } - let stashSha: string | undefined; - let stashMessage: string | undefined; + let isDirty: boolean; try { const { stdout } = await execGit(context, ["status", "--porcelain"]); - const isDirty = stdout.trim() !== ""; - if (isDirty) { - stashMessage = `${ROLLBACK_STASH_MESSAGE_PREFIX}${Date.now()}`; - await execGit(context, ["stash", "push", "--include-untracked", "-m", stashMessage]); - const { stdout: stashRef } = await execGit(context, ["rev-parse", "stash@{0}"]); - stashSha = stashRef.trim(); - // Re-apply the stash so the working tree is restored. The stash entry - // itself stays in the stash list as a recovery snapshot. - await execGit(context, ["stash", "apply", stashSha]); - } + isDirty = stdout.trim() !== ""; } catch (e: any) { + // Conservatively treat the tree as dirty so rollback won't run a + // destructive `git clean` against state we couldn't observe. context.cli.warn( - `Failed to snapshot uncommitted changes for rollback: ${e?.message ?? e}. ` + - `Rollback will still undo committed and edited files.`, + `Could not determine working tree status for rollback: ${e?.message ?? e}. ` + + `Rollback will skip cleanup of untracked files to avoid data loss.`, ); - // Don't carry a half-baked stash forward. - stashSha = undefined; - stashMessage = undefined; + context.rollback = { + originalHead, + pushAttempted: false, + cleanAllowedDuringRollback: false, + }; + return; + } + + let stashSha: string | undefined; + let stashMessage: string | undefined; + let cleanAllowedDuringRollback: boolean; + + if (!isDirty) { + // No pre-existing untracked files at snapshot — anything untracked at + // rollback time was created by the release process and is safe to clean. + cleanAllowedDuringRollback = true; + } else { + // Working tree is dirty. We need a recoverable backup before the release + // touches it; until that backup exists, rollback must not call + // `git clean -fd` or it will permanently destroy the user's untracked files. + cleanAllowedDuringRollback = false; + const message = `${ROLLBACK_STASH_MESSAGE_PREFIX}${Date.now()}`; + try { + await execGit(context, ["stash", "push", "--include-untracked", "-m", message]); + // From this line on, the stash entry exists in `git stash list` and + // is identifiable by its message. Anything below may fail without + // invalidating that recovery anchor — keep `stashMessage` set even on + // later errors. + stashMessage = message; + cleanAllowedDuringRollback = true; + } catch (e: any) { + context.cli.warn( + `Failed to snapshot uncommitted changes for rollback: ${e?.message ?? e}. ` + + `Rollback will undo committed and edited files but cannot restore ` + + `untracked files; cleanup of untracked files will be skipped.`, + ); + } + + if (stashMessage) { + try { + const { stdout: stashRef } = await execGit(context, ["rev-parse", "stash@{0}"]); + stashSha = stashRef.trim(); + // Re-apply the stash so the working tree is restored for the + // release to operate on. The stash entry itself stays in the + // stash list as a recovery snapshot. + await execGit(context, ["stash", "apply", stashSha]); + } catch (e: any) { + context.cli.warn( + `Snapshot stash was created but could not be re-applied: ${e?.message ?? e}. ` + + `Recover manually with: git stash list (look for "${stashMessage}")`, + ); + } + } } context.rollback = { @@ -94,6 +136,7 @@ export async function captureRollbackSnapshot(context: Context): Promise { stashSha, stashMessage, pushAttempted: false, + cleanAllowedDuringRollback: cleanAllowedDuringRollback, }; } @@ -163,12 +206,21 @@ export async function finalizeRollback( return; } - // Drop untracked files left behind by plugins (e.g. .commitmessage). Do not - // pass -x so files ignored by .gitignore (node_modules etc.) are preserved. - try { - await execGit(context, ["clean", "-fd"]); - } catch (e: any) { - context.cli.warn(`Failed to clean untracked files: ${e?.message ?? e}`); + // Drop untracked files left behind by plugins (e.g. .commitmessage). Skip + // when we don't have a recoverable backup of pre-existing untracked files, + // since `git clean -fd` would otherwise destroy them. Do not pass -x — that + // would also remove .gitignore'd files like node_modules. + if (state.cleanAllowedDuringRollback) { + try { + await execGit(context, ["clean", "-fd"]); + } catch (e: any) { + context.cli.warn(`Failed to clean untracked files: ${e?.message ?? e}`); + } + } else { + context.cli.warn( + `Skipping cleanup of untracked files because no recovery snapshot is available. ` + + `You may need to remove leftover release artifacts manually.`, + ); } // Restore the user's pre-release uncommitted changes from the stash snapshot diff --git a/packages/plugin-git/src/index.test.ts b/packages/plugin-git/src/index.test.ts index 1d8baee..d26eaf6 100644 --- a/packages/plugin-git/src/index.test.ts +++ b/packages/plugin-git/src/index.test.ts @@ -436,13 +436,14 @@ This is the changelog.`); } }); - it("marks rollback.pushAttempted=true when entering the push stage", async () => { + it("marks rollback.pushAttempted=true once a real push is dispatched", async () => { const gitPlugin = new GitPlugin(); const context = createMockContext({ plugins: [gitPlugin] }); context.setData("version_new", "1.2.9"); context.rollback = { originalHead: "deadbeef", pushAttempted: false, + cleanAllowedDuringRollback: true, }; context.sys.mockExec(() => ""); @@ -451,6 +452,35 @@ This is the changelog.`); expect(context.rollback.pushAttempted).toBe(true); }); + it("leaves rollback.pushAttempted=false when pre-push setup fails", async () => { + // Regression: the flag must not be set on stage entry, otherwise a + // failure in `getUpstream()` (or any other pre-push step) would + // disable rollback for a release that never actually contacted the + // remote. + const gitPlugin = new GitPlugin(); + const context = createMockContext({ + plugins: [gitPlugin], + // Force getUpstream() to be called by clearing the configured remote. + argv: { remote: "" }, + }); + context.setData("version_new", "1.2.9b"); + context.rollback = { + originalHead: "deadbeef", + pushAttempted: false, + cleanAllowedDuringRollback: true, + }; + context.sys.mockExec((cmd) => { + if (cmd.includes("rev-parse --abbrev-ref --symbolic-full-name")) { + throw new Error("no upstream configured"); + } + return ""; + }); + + await expect(gitPlugin.executeStage(context, DefaultStages.push)).rejects.toThrow(); + + expect(context.rollback.pushAttempted).toBe(false); + }); + it("does not mark rollback.pushAttempted in dry run", async () => { const gitPlugin = new GitPlugin(); const context = createMockContext({ @@ -461,6 +491,7 @@ This is the changelog.`); context.rollback = { originalHead: "deadbeef", pushAttempted: false, + cleanAllowedDuringRollback: true, }; context.sys.mockExec(() => ""); @@ -479,6 +510,7 @@ This is the changelog.`); context.rollback = { originalHead: "deadbeef", pushAttempted: false, + cleanAllowedDuringRollback: true, }; await gitPlugin.executeStage(context, DefaultStages.push); diff --git a/packages/plugin-git/src/index.ts b/packages/plugin-git/src/index.ts index 32173f3..73566e6 100644 --- a/packages/plugin-git/src/index.ts +++ b/packages/plugin-git/src/index.ts @@ -274,12 +274,6 @@ ${context.getData("changelog_new")}`; return; } - // Once we start pushing to the remote, we can no longer safely roll back - // locally without risking divergence with what the remote has accepted. - if (context.rollback && !context.argv.dryRun) { - context.rollback.pushAttempted = true; - } - const upstream = (context.argv.remote as string | undefined) || (await getUpstream(context)); const [remote, branch] = upstream.split("/", 2); @@ -300,6 +294,11 @@ ${context.getData("changelog_new")}`; for (const command of commands) { context.cli.logCommand(command); if (!context.argv.dryRun) { + // Remember that we attempted to push immediately before the first + // actual push command, not during information gathering. + if (context.rollback) { + context.rollback.pushAttempted = true; + } await context.sys.execRaw(command, { cwd: context.cwd }); } } From 96c7140f0aa6b7b8c5c24c07e2f549e97f750dba Mon Sep 17 00:00:00 2001 From: Dominic Griesel Date: Wed, 6 May 2026 13:02:58 +0200 Subject: [PATCH 3/7] cleanup --- packages/core/src/lib/planner.ts | 2 +- packages/core/src/lib/rollback.test.ts | 3 +-- packages/core/src/lib/rollback.ts | 24 ++++++++---------------- packages/plugin-git/src/index.test.ts | 7 +++---- packages/plugin-git/src/index.ts | 15 +++++++++------ 5 files changed, 22 insertions(+), 29 deletions(-) diff --git a/packages/core/src/lib/planner.ts b/packages/core/src/lib/planner.ts index d4ee72c..53fd525 100644 --- a/packages/core/src/lib/planner.ts +++ b/packages/core/src/lib/planner.ts @@ -173,7 +173,7 @@ export async function execute(context: Context): Promise { // the working tree. The `check` stage is read-only by convention; any // later stage (including exec hooks like `after_check` / `before_edit`) // might write files, so we capture before any of those run. - if (!snapshotTaken && stage.id !== "check") { + if (!snapshotTaken && stage.id !== DefaultStages.check.id) { await captureRollbackSnapshot(context); snapshotTaken = true; } diff --git a/packages/core/src/lib/rollback.test.ts b/packages/core/src/lib/rollback.test.ts index 2efd9cd..d442006 100644 --- a/packages/core/src/lib/rollback.test.ts +++ b/packages/core/src/lib/rollback.test.ts @@ -166,13 +166,12 @@ describe("finalizeRollback (failure path)", () => { await finalizeRollback(context, { failed: true }); - // Reset still runs expect(context.sys.exec).toHaveBeenCalledWith( "git", ["reset", "--hard", HEAD_SHA], expect.anything(), ); - // But clean does not — pre-existing untracked files might be unrecoverable + // Clean would destroy pre-existing untracked files we couldn't snapshot expect(context.sys.exec).not.toHaveBeenCalledWith( "git", ["clean", "-fd"], diff --git a/packages/core/src/lib/rollback.ts b/packages/core/src/lib/rollback.ts index 1a98249..b55bfd3 100644 --- a/packages/core/src/lib/rollback.ts +++ b/packages/core/src/lib/rollback.ts @@ -1,4 +1,4 @@ -import type { Context } from "./context.js"; +import type { Context, RollbackState } from "./context.js"; const ROLLBACK_STASH_MESSAGE_PREFIX = "release-script-rollback-"; @@ -30,7 +30,7 @@ async function findStashIndexByMessage( return undefined; } -async function dropRollbackStash(context: Context, state: RollbackStateInternal): Promise { +async function dropRollbackStash(context: Context, state: RollbackState): Promise { if (!state.stashMessage) return; const index = await findStashIndexByMessage(context, state.stashMessage); if (!index) { @@ -44,9 +44,6 @@ async function dropRollbackStash(context: Context, state: RollbackStateInternal) } } -// Local alias to avoid an import cycle and keep call sites readable. -type RollbackStateInternal = NonNullable; - /** * Captures the state required to roll back any release-induced changes. * Should be called once, just before the first stage that mutates the working tree. @@ -86,17 +83,12 @@ export async function captureRollbackSnapshot(context: Context): Promise { let stashSha: string | undefined; let stashMessage: string | undefined; - let cleanAllowedDuringRollback: boolean; + // A clean working tree is automatically safe: anything untracked at rollback + // time must have been created by the release. A dirty working tree only + // becomes safe once we've stashed the user's changes as a recovery anchor. + let cleanAllowedDuringRollback = !isDirty; - if (!isDirty) { - // No pre-existing untracked files at snapshot — anything untracked at - // rollback time was created by the release process and is safe to clean. - cleanAllowedDuringRollback = true; - } else { - // Working tree is dirty. We need a recoverable backup before the release - // touches it; until that backup exists, rollback must not call - // `git clean -fd` or it will permanently destroy the user's untracked files. - cleanAllowedDuringRollback = false; + if (isDirty) { const message = `${ROLLBACK_STASH_MESSAGE_PREFIX}${Date.now()}`; try { await execGit(context, ["stash", "push", "--include-untracked", "-m", message]); @@ -136,7 +128,7 @@ export async function captureRollbackSnapshot(context: Context): Promise { stashSha, stashMessage, pushAttempted: false, - cleanAllowedDuringRollback: cleanAllowedDuringRollback, + cleanAllowedDuringRollback, }; } diff --git a/packages/plugin-git/src/index.test.ts b/packages/plugin-git/src/index.test.ts index d26eaf6..d690b12 100644 --- a/packages/plugin-git/src/index.test.ts +++ b/packages/plugin-git/src/index.test.ts @@ -453,10 +453,9 @@ This is the changelog.`); }); it("leaves rollback.pushAttempted=false when pre-push setup fails", async () => { - // Regression: the flag must not be set on stage entry, otherwise a - // failure in `getUpstream()` (or any other pre-push step) would - // disable rollback for a release that never actually contacted the - // remote. + // If a pre-push step (e.g. `getUpstream()`) throws before any push is + // dispatched, the release never contacted the remote — rollback must + // still be allowed. const gitPlugin = new GitPlugin(); const context = createMockContext({ plugins: [gitPlugin], diff --git a/packages/plugin-git/src/index.ts b/packages/plugin-git/src/index.ts index 73566e6..e9ce3b3 100644 --- a/packages/plugin-git/src/index.ts +++ b/packages/plugin-git/src/index.ts @@ -249,20 +249,23 @@ ${context.getData("changelog_new")}`; } // And commit stuff + const tagName = `v${newVersion}`; + const tagCommand = ["git", "tag", "-a", tagName, "-m", tagName]; const commands = [ ["git", "add", "-A", "--", ":(exclude).commitmessage"], ["git", "commit", "-F", ".commitmessage"], - ["git", "tag", "-a", `v${newVersion}`, "-m", `v${newVersion}`], + tagCommand, ]; - for (const [cmd, ...args] of commands) { + for (const command of commands) { + const [cmd, ...args] = command; context.cli.logCommand(cmd, args); if (!context.argv.dryRun) { await context.sys.exec(cmd, args, { cwd: context.cwd }); - // Record successful tag creation so rollback can safely delete it - // without ever touching a pre-existing tag of the same name. - if (cmd === "git" && args[0] === "tag" && context.rollback) { - context.rollback.createdTag = `v${newVersion}`; + // Record the tag name only after `git tag` succeeds, so rollback + // never deletes a pre-existing tag of the same name. + if (command === tagCommand && context.rollback) { + context.rollback.createdTag = tagName; } } } From d269a166f2a1b278c2b186d6e95bf09421d2bd61 Mon Sep 17 00:00:00 2001 From: Dominic Griesel Date: Wed, 6 May 2026 14:17:02 +0200 Subject: [PATCH 4/7] address review feedback --- packages/core/src/lib/rollback.test.ts | 89 ++++++++++++++++++++++++-- packages/core/src/lib/rollback.ts | 83 ++++++++++++++---------- packages/release-script/src/index.ts | 2 +- 3 files changed, 135 insertions(+), 39 deletions(-) diff --git a/packages/core/src/lib/rollback.test.ts b/packages/core/src/lib/rollback.test.ts index d442006..18b5b04 100644 --- a/packages/core/src/lib/rollback.test.ts +++ b/packages/core/src/lib/rollback.test.ts @@ -39,8 +39,8 @@ describe("captureRollbackSnapshot", () => { if (cmd === "git rev-parse HEAD") return HEAD_SHA; if (cmd === "git status --porcelain") return " M package.json\n"; if (cmd.startsWith("git stash push")) return ""; + if (cmd === "git stash apply stash@{0}") return ""; if (cmd === "git rev-parse stash@{0}") return STASH_SHA; - if (cmd === `git stash apply ${STASH_SHA}`) return ""; throw new Error(`unexpected command: ${cmd}`); }); @@ -52,7 +52,7 @@ describe("captureRollbackSnapshot", () => { // Stash apply must be called so plugin edits + user edits coexist before commit expect(context.sys.exec).toHaveBeenCalledWith( "git", - ["stash", "apply", STASH_SHA], + ["stash", "apply", "stash@{0}"], expect.anything(), ); }); @@ -86,25 +86,44 @@ describe("captureRollbackSnapshot", () => { expect(context.rollback?.stashSha).toBeUndefined(); }); - it("preserves stashMessage when stash push succeeded but a later step failed", async () => { + it("preserves stashMessage when SHA capture fails after a successful apply", async () => { const context = freshContext({}); context.sys.mockExec((cmd) => { if (cmd === "git rev-parse HEAD") return HEAD_SHA; if (cmd === "git status --porcelain") return " M package.json\n"; if (cmd.startsWith("git stash push")) return ""; + if (cmd === "git stash apply stash@{0}") return ""; if (cmd === "git rev-parse stash@{0}") throw new Error("rev-parse failed"); throw new Error(`unexpected command: ${cmd}`); }); await captureRollbackSnapshot(context); - // Stash exists, so we must remember its message for recovery + // Apply succeeded, so the working tree matches the user's pre-release state. + // The stash entry remains as a recovery anchor identified by message. expect(context.rollback?.stashMessage).toMatch(/^release-script-rollback-/); expect(context.rollback?.stashSha).toBeUndefined(); - // And cleanAllowedDuringRollback must be true (we have a recoverable backup) expect(context.rollback?.cleanAllowedDuringRollback).toBe(true); }); + it("throws if apply fails after the snapshot stash was created", async () => { + // A failed apply leaves the working tree clean while the user's changes + // only live in the stash — proceeding would silently change the release + // contents, so capture must abort and let the caller surface the error. + const context = freshContext({}); + context.sys.mockExec((cmd) => { + if (cmd === "git rev-parse HEAD") return HEAD_SHA; + if (cmd === "git status --porcelain") return " M package.json\n"; + if (cmd.startsWith("git stash push")) return ""; + if (cmd === "git stash apply stash@{0}") throw new Error("conflict"); + throw new Error(`unexpected command: ${cmd}`); + }); + + await expect(captureRollbackSnapshot(context)).rejects.toThrow( + /Could not re-apply your uncommitted changes/, + ); + }); + it("does nothing when --dryRun is set", async () => { const context = freshContext({ argv: { dryRun: true } }); await captureRollbackSnapshot(context); @@ -284,6 +303,66 @@ describe("finalizeRollback (failure path)", () => { expect(context.warnings.some((w) => /uncommitted changes/i.test(w))).toBe(true); }); + it("restores the stash via message-based lookup when stashSha is missing", async () => { + // Captures the case where the SHA couldn't be resolved at snapshot time + // (e.g. transient `git rev-parse` failure) but the stash entry exists + // and is identifiable by its message. + const context = freshContext({}); + context.rollback = { + originalHead: HEAD_SHA, + stashMessage: STASH_MSG, + pushAttempted: false, + cleanAllowedDuringRollback: true, + }; + context.sys.mockExec((cmd) => { + if (cmd === "git stash list --pretty=format:%gd %gs") { + return `stash@{0} On main: ${STASH_MSG}`; + } + return ""; + }); + + await finalizeRollback(context, { failed: true }); + + expect(context.sys.exec).toHaveBeenCalledWith( + "git", + ["stash", "apply", "stash@{0}"], + expect.anything(), + ); + expect(context.sys.exec).toHaveBeenCalledWith( + "git", + ["stash", "drop", "stash@{0}"], + expect.anything(), + ); + }); + + it("does NOT drop the stash when message-based restore fails", async () => { + const context = freshContext({}); + context.rollback = { + originalHead: HEAD_SHA, + stashMessage: STASH_MSG, + pushAttempted: false, + cleanAllowedDuringRollback: true, + }; + context.sys.mockExec((cmd) => { + if (cmd === "git stash list --pretty=format:%gd %gs") { + return `stash@{0} On main: ${STASH_MSG}`; + } + if (cmd === "git stash apply stash@{0}") { + throw new Error("conflict"); + } + return ""; + }); + + await finalizeRollback(context, { failed: true }); + + expect(context.sys.exec).not.toHaveBeenCalledWith( + "git", + ["stash", "drop", "stash@{0}"], + expect.anything(), + ); + expect(context.warnings.some((w) => /uncommitted changes/i.test(w))).toBe(true); + }); + it("drops the snapshot stash even when push was already attempted", async () => { const context = freshContext({}); context.rollback = { diff --git a/packages/core/src/lib/rollback.ts b/packages/core/src/lib/rollback.ts index b55bfd3..808518f 100644 --- a/packages/core/src/lib/rollback.ts +++ b/packages/core/src/lib/rollback.ts @@ -93,9 +93,7 @@ export async function captureRollbackSnapshot(context: Context): Promise { try { await execGit(context, ["stash", "push", "--include-untracked", "-m", message]); // From this line on, the stash entry exists in `git stash list` and - // is identifiable by its message. Anything below may fail without - // invalidating that recovery anchor — keep `stashMessage` set even on - // later errors. + // is identifiable by its message. stashMessage = message; cleanAllowedDuringRollback = true; } catch (e: any) { @@ -107,19 +105,30 @@ export async function captureRollbackSnapshot(context: Context): Promise { } if (stashMessage) { + // Re-apply immediately so the release operates on the same working + // tree the user had. If apply fails the working tree is now clean + // while the user's changes only live in the stash — proceeding would + // silently change what the release contains, so abort instead. try { - const { stdout: stashRef } = await execGit(context, ["rev-parse", "stash@{0}"]); - stashSha = stashRef.trim(); - // Re-apply the stash so the working tree is restored for the - // release to operate on. The stash entry itself stays in the - // stash list as a recovery snapshot. - await execGit(context, ["stash", "apply", stashSha]); + await execGit(context, ["stash", "apply", "stash@{0}"]); } catch (e: any) { - context.cli.warn( - `Snapshot stash was created but could not be re-applied: ${e?.message ?? e}. ` + - `Recover manually with: git stash list (look for "${stashMessage}")`, + throw new Error( + `Could not re-apply your uncommitted changes after snapshotting them ` + + `for rollback: ${e?.message ?? e}. ` + + `Your changes are preserved in the stash list (look for "${stashMessage}"); ` + + `recover them with: git stash apply stash^{/${stashMessage}}`, ); } + + // Capture the SHA as a stable handle for the later restore in + // finalizeRollback. Best-effort: if this fails we can fall back to + // looking the entry up by its unique message. + try { + const { stdout } = await execGit(context, ["rev-parse", "stash@{0}"]); + stashSha = stdout.trim(); + } catch { + /* fall back to message-based lookup */ + } } } @@ -155,9 +164,9 @@ export async function finalizeRollback( if (!state) return; if (!options.failed) { - // Success path: the snapshot stash (if any) holds changes that are now - // part of the release commit, so just drop it to keep `git stash list` - // clean. + // Success path: any snapshot stash was re-applied to the working tree + // during capture (captureRollbackSnapshot aborts otherwise), so its + // contents are part of the release commit and the entry can be dropped. await dropRollbackStash(context, state); return; } @@ -215,28 +224,36 @@ export async function finalizeRollback( ); } - // Restore the user's pre-release uncommitted changes from the stash snapshot + // Restore the user's pre-release uncommitted changes from the stash + // snapshot. Prefer the SHA (stable across other stash operations); if it + // wasn't captured, look the entry up by its unique message. let stashApplied = false; - if (state.stashSha) { - try { - await execGit(context, ["stash", "apply", state.stashSha]); - stashApplied = true; - context.cli.log("Restored your pre-release uncommitted changes"); - } catch (e: any) { - const recoveryHint = state.stashMessage - ? `Recover them manually with: git stash list (look for "${state.stashMessage}")` - : `Recover them manually via the stash list.`; - context.cli.warn( - `Failed to restore pre-release uncommitted changes: ${e?.message ?? e}. ` + - recoveryHint, - ); + if (state.stashMessage || state.stashSha) { + const ref = + state.stashSha ?? + (state.stashMessage + ? await findStashIndexByMessage(context, state.stashMessage) + : undefined); + if (ref) { + try { + await execGit(context, ["stash", "apply", ref]); + stashApplied = true; + context.cli.log("Restored your pre-release uncommitted changes"); + } catch (e: any) { + const recoveryHint = state.stashMessage + ? `Recover them manually with: git stash list (look for "${state.stashMessage}")` + : `Recover them manually via the stash list.`; + context.cli.warn( + `Failed to restore pre-release uncommitted changes: ${e?.message ?? e}. ` + + recoveryHint, + ); + } } } - // Only drop the stash entry if we successfully restored its contents (or if - // there was nothing to restore). Keep it around if apply failed so the user - // can recover manually. - if (!state.stashSha || stashApplied) { + // Drop the snapshot stash only when there was nothing to restore, or the + // restore succeeded. Otherwise keep it as the user's recovery anchor. + if (!state.stashMessage || stashApplied) { await dropRollbackStash(context, state); } } diff --git a/packages/release-script/src/index.ts b/packages/release-script/src/index.ts index e113d71..27c2dd2 100644 --- a/packages/release-script/src/index.ts +++ b/packages/release-script/src/index.ts @@ -203,7 +203,7 @@ export async function main(): Promise { noRollback: { type: "boolean", description: - "Do not roll back local changes (file edits, commits, tags) if making the release commit fails", + "Do not roll back local changes (file edits, commits, tags) if the release fails before pushing", default: false, }, }); From a087e2b0bbf28a572191fdc8a13289aa5ec28e4a Mon Sep 17 00:00:00 2001 From: Dominic Griesel Date: Wed, 6 May 2026 14:30:28 +0200 Subject: [PATCH 5/7] address review feedback --- packages/core/src/lib/rollback.test.ts | 24 ++++++++++++------------ packages/core/src/lib/rollback.ts | 12 +++++++----- packages/release-script/src/index.ts | 4 ++++ 3 files changed, 23 insertions(+), 17 deletions(-) diff --git a/packages/core/src/lib/rollback.test.ts b/packages/core/src/lib/rollback.test.ts index 18b5b04..29784a9 100644 --- a/packages/core/src/lib/rollback.test.ts +++ b/packages/core/src/lib/rollback.test.ts @@ -39,7 +39,7 @@ describe("captureRollbackSnapshot", () => { if (cmd === "git rev-parse HEAD") return HEAD_SHA; if (cmd === "git status --porcelain") return " M package.json\n"; if (cmd.startsWith("git stash push")) return ""; - if (cmd === "git stash apply stash@{0}") return ""; + if (cmd === "git stash apply --index stash@{0}") return ""; if (cmd === "git rev-parse stash@{0}") return STASH_SHA; throw new Error(`unexpected command: ${cmd}`); }); @@ -52,7 +52,7 @@ describe("captureRollbackSnapshot", () => { // Stash apply must be called so plugin edits + user edits coexist before commit expect(context.sys.exec).toHaveBeenCalledWith( "git", - ["stash", "apply", "stash@{0}"], + ["stash", "apply", "--index", "stash@{0}"], expect.anything(), ); }); @@ -69,7 +69,7 @@ describe("captureRollbackSnapshot", () => { expect(context.rollback?.cleanAllowedDuringRollback).toBe(true); }); - it("marks cleanAllowedDuringRollback=false and skips stash when push fails on dirty tree", async () => { + it("marks cleanAllowedDuringRollback=false when stash creation fails on dirty tree", async () => { const context = freshContext({}); context.sys.mockExec((cmd) => { if (cmd === "git rev-parse HEAD") return HEAD_SHA; @@ -92,7 +92,7 @@ describe("captureRollbackSnapshot", () => { if (cmd === "git rev-parse HEAD") return HEAD_SHA; if (cmd === "git status --porcelain") return " M package.json\n"; if (cmd.startsWith("git stash push")) return ""; - if (cmd === "git stash apply stash@{0}") return ""; + if (cmd === "git stash apply --index stash@{0}") return ""; if (cmd === "git rev-parse stash@{0}") throw new Error("rev-parse failed"); throw new Error(`unexpected command: ${cmd}`); }); @@ -115,7 +115,7 @@ describe("captureRollbackSnapshot", () => { if (cmd === "git rev-parse HEAD") return HEAD_SHA; if (cmd === "git status --porcelain") return " M package.json\n"; if (cmd.startsWith("git stash push")) return ""; - if (cmd === "git stash apply stash@{0}") throw new Error("conflict"); + if (cmd === "git stash apply --index stash@{0}") throw new Error("conflict"); throw new Error(`unexpected command: ${cmd}`); }); @@ -131,7 +131,7 @@ describe("captureRollbackSnapshot", () => { expect(context.sys.exec).not.toHaveBeenCalled(); }); - it("does nothing when --no-rollback is set", async () => { + it("does nothing when --noRollback is set", async () => { const context = freshContext({ argv: { noRollback: true } }); await captureRollbackSnapshot(context); expect(context.rollback).toBeUndefined(); @@ -258,7 +258,7 @@ describe("finalizeRollback (failure path)", () => { expect(context.sys.exec).toHaveBeenCalledWith( "git", - ["stash", "apply", STASH_SHA], + ["stash", "apply", "--index", STASH_SHA], expect.anything(), ); // Drop must use the stash@{N} index, not the SHA @@ -284,7 +284,7 @@ describe("finalizeRollback (failure path)", () => { cleanAllowedDuringRollback: true, }; context.sys.mockExec((cmd) => { - if (cmd === `git stash apply ${STASH_SHA}`) { + if (cmd === `git stash apply --index ${STASH_SHA}`) { throw new Error("conflict"); } if (cmd === "git stash list --pretty=format:%gd %gs") { @@ -325,7 +325,7 @@ describe("finalizeRollback (failure path)", () => { expect(context.sys.exec).toHaveBeenCalledWith( "git", - ["stash", "apply", "stash@{0}"], + ["stash", "apply", "--index", "stash@{0}"], expect.anything(), ); expect(context.sys.exec).toHaveBeenCalledWith( @@ -347,7 +347,7 @@ describe("finalizeRollback (failure path)", () => { if (cmd === "git stash list --pretty=format:%gd %gs") { return `stash@{0} On main: ${STASH_MSG}`; } - if (cmd === "git stash apply stash@{0}") { + if (cmd === "git stash apply --index stash@{0}") { throw new Error("conflict"); } return ""; @@ -407,7 +407,7 @@ describe("finalizeRollback (failure path)", () => { expect(context.sys.exec).not.toHaveBeenCalled(); }); - it("does nothing when --no-rollback is set", async () => { + it("does nothing when --noRollback is set", async () => { const context = freshContext({ argv: { noRollback: true } }); context.rollback = { originalHead: HEAD_SHA, @@ -468,7 +468,7 @@ describe("finalizeRollback (success path)", () => { expect(context.sys.exec).not.toHaveBeenCalled(); }); - it("is a no-op when --dryRun or --no-rollback is set", async () => { + it("is a no-op when --dryRun or --noRollback is set", async () => { const dry = freshContext({ argv: { dryRun: true } }); dry.rollback = { originalHead: HEAD_SHA, diff --git a/packages/core/src/lib/rollback.ts b/packages/core/src/lib/rollback.ts index 808518f..38d44c2 100644 --- a/packages/core/src/lib/rollback.ts +++ b/packages/core/src/lib/rollback.ts @@ -106,11 +106,12 @@ export async function captureRollbackSnapshot(context: Context): Promise { if (stashMessage) { // Re-apply immediately so the release operates on the same working - // tree the user had. If apply fails the working tree is now clean - // while the user's changes only live in the stash — proceeding would - // silently change what the release contains, so abort instead. + // tree the user had. `--index` preserves the staged/unstaged split. + // If apply fails the working tree is now clean while the user's + // changes only live in the stash — proceeding would silently change + // what the release contains, so abort instead. try { - await execGit(context, ["stash", "apply", "stash@{0}"]); + await execGit(context, ["stash", "apply", "--index", "stash@{0}"]); } catch (e: any) { throw new Error( `Could not re-apply your uncommitted changes after snapshotting them ` + @@ -236,7 +237,8 @@ export async function finalizeRollback( : undefined); if (ref) { try { - await execGit(context, ["stash", "apply", ref]); + // `--index` preserves the staged/unstaged split the user had. + await execGit(context, ["stash", "apply", "--index", ref]); stashApplied = true; context.cli.log("Restored your pre-release uncommitted changes"); } catch (e: any) { diff --git a/packages/release-script/src/index.ts b/packages/release-script/src/index.ts index 27c2dd2..6236fea 100644 --- a/packages/release-script/src/index.ts +++ b/packages/release-script/src/index.ts @@ -352,6 +352,10 @@ export async function main(): Promise { ), ), ); + // A rollback that throws leaves the working tree in an unknown state — + // always surface a non-zero exit so automation can detect it, even if + // the release itself was successful up to that point. + exitCode ??= 1; } if (exitCode !== undefined) { From 83b684dd4352291031846c0c5799295823a3c699 Mon Sep 17 00:00:00 2001 From: Dominic Griesel Date: Wed, 6 May 2026 15:51:52 +0200 Subject: [PATCH 6/7] address review feedback --- packages/core/src/lib/context.ts | 10 +--- packages/core/src/lib/rollback.test.ts | 65 ++++++-------------------- packages/core/src/lib/rollback.ts | 41 ++++------------ 3 files changed, 25 insertions(+), 91 deletions(-) diff --git a/packages/core/src/lib/context.ts b/packages/core/src/lib/context.ts index 0c3bd1d..134b672 100644 --- a/packages/core/src/lib/context.ts +++ b/packages/core/src/lib/context.ts @@ -46,7 +46,8 @@ export interface Context { /** * State used to roll back local changes if the release fails. - * Populated by `captureRollbackSnapshot` once the edit stage is about to run. + * Populated by `captureRollbackSnapshot` before the first non-`check` stage + * that may mutate the working tree. */ rollback?: RollbackState; } @@ -71,11 +72,4 @@ export interface RollbackState { * after `git tag` succeeds, so rollback never deletes a pre-existing tag. */ createdTag?: string; - /** - * Whether `git clean -fd` is safe to run during rollback. False when the - * working tree was dirty at snapshot time but the snapshot stash could not - * be created — running clean in that case would permanently lose the user's - * pre-existing untracked files. - */ - cleanAllowedDuringRollback: boolean; } diff --git a/packages/core/src/lib/rollback.test.ts b/packages/core/src/lib/rollback.test.ts index 29784a9..b565f0b 100644 --- a/packages/core/src/lib/rollback.test.ts +++ b/packages/core/src/lib/rollback.test.ts @@ -48,7 +48,6 @@ describe("captureRollbackSnapshot", () => { expect(context.rollback?.stashSha).toBe(STASH_SHA); expect(context.rollback?.stashMessage).toMatch(/^release-script-rollback-/); - expect(context.rollback?.cleanAllowedDuringRollback).toBe(true); // Stash apply must be called so plugin edits + user edits coexist before commit expect(context.sys.exec).toHaveBeenCalledWith( "git", @@ -57,19 +56,23 @@ describe("captureRollbackSnapshot", () => { ); }); - it("marks cleanAllowedDuringRollback=true when the working tree is clean", async () => { + it("does not capture rollback state when git status cannot be determined", async () => { const context = freshContext({}); - context.sys.mockExec({ - "git rev-parse HEAD": HEAD_SHA, - "git status --porcelain": "", + context.sys.mockExec((cmd) => { + if (cmd === "git rev-parse HEAD") return HEAD_SHA; + if (cmd === "git status --porcelain") throw new Error("status failed"); + throw new Error(`unexpected command: ${cmd}`); }); await captureRollbackSnapshot(context); - expect(context.rollback?.cleanAllowedDuringRollback).toBe(true); + expect(context.rollback).toBeUndefined(); + expect( + context.warnings.some((w) => /rollback has been disabled for this run/i.test(w)), + ).toBe(true); }); - it("marks cleanAllowedDuringRollback=false when stash creation fails on dirty tree", async () => { + it("does not capture rollback state when stash creation fails on a dirty tree", async () => { const context = freshContext({}); context.sys.mockExec((cmd) => { if (cmd === "git rev-parse HEAD") return HEAD_SHA; @@ -80,10 +83,10 @@ describe("captureRollbackSnapshot", () => { await captureRollbackSnapshot(context); - expect(context.rollback).toBeDefined(); - expect(context.rollback?.cleanAllowedDuringRollback).toBe(false); - expect(context.rollback?.stashMessage).toBeUndefined(); - expect(context.rollback?.stashSha).toBeUndefined(); + expect(context.rollback).toBeUndefined(); + expect( + context.warnings.some((w) => /rollback has been disabled for this run/i.test(w)), + ).toBe(true); }); it("preserves stashMessage when SHA capture fails after a successful apply", async () => { @@ -103,7 +106,6 @@ describe("captureRollbackSnapshot", () => { // The stash entry remains as a recovery anchor identified by message. expect(context.rollback?.stashMessage).toMatch(/^release-script-rollback-/); expect(context.rollback?.stashSha).toBeUndefined(); - expect(context.rollback?.cleanAllowedDuringRollback).toBe(true); }); it("throws if apply fails after the snapshot stash was created", async () => { @@ -160,7 +162,6 @@ describe("finalizeRollback (failure path)", () => { context.rollback = { originalHead: HEAD_SHA, pushAttempted: false, - cleanAllowedDuringRollback: true, }; context.sys.mockExec(() => ""); @@ -174,37 +175,11 @@ describe("finalizeRollback (failure path)", () => { expect(context.sys.exec).toHaveBeenCalledWith("git", ["clean", "-fd"], expect.anything()); }); - it("does NOT run git clean when cleanAllowedDuringRollback is false", async () => { - const context = freshContext({}); - context.rollback = { - originalHead: HEAD_SHA, - pushAttempted: false, - cleanAllowedDuringRollback: false, - }; - context.sys.mockExec(() => ""); - - await finalizeRollback(context, { failed: true }); - - expect(context.sys.exec).toHaveBeenCalledWith( - "git", - ["reset", "--hard", HEAD_SHA], - expect.anything(), - ); - // Clean would destroy pre-existing untracked files we couldn't snapshot - expect(context.sys.exec).not.toHaveBeenCalledWith( - "git", - ["clean", "-fd"], - expect.anything(), - ); - expect(context.warnings.some((w) => /untracked files/i.test(w))).toBe(true); - }); - it("deletes the release tag only if this run created it", async () => { const context = freshContext({}); context.rollback = { originalHead: HEAD_SHA, pushAttempted: false, - cleanAllowedDuringRollback: true, createdTag: "v1.2.3", }; context.sys.mockExec(() => ""); @@ -223,7 +198,6 @@ describe("finalizeRollback (failure path)", () => { context.rollback = { originalHead: HEAD_SHA, pushAttempted: false, - cleanAllowedDuringRollback: true, }; // version_new is set but createdTag is not — tag pre-existed. context.setData("version_new", "1.2.3"); @@ -245,7 +219,6 @@ describe("finalizeRollback (failure path)", () => { stashSha: STASH_SHA, stashMessage: STASH_MSG, pushAttempted: false, - cleanAllowedDuringRollback: true, }; context.sys.mockExec((cmd) => { if (cmd === "git stash list --pretty=format:%gd %gs") { @@ -281,7 +254,6 @@ describe("finalizeRollback (failure path)", () => { stashSha: STASH_SHA, stashMessage: STASH_MSG, pushAttempted: false, - cleanAllowedDuringRollback: true, }; context.sys.mockExec((cmd) => { if (cmd === `git stash apply --index ${STASH_SHA}`) { @@ -312,7 +284,6 @@ describe("finalizeRollback (failure path)", () => { originalHead: HEAD_SHA, stashMessage: STASH_MSG, pushAttempted: false, - cleanAllowedDuringRollback: true, }; context.sys.mockExec((cmd) => { if (cmd === "git stash list --pretty=format:%gd %gs") { @@ -341,7 +312,6 @@ describe("finalizeRollback (failure path)", () => { originalHead: HEAD_SHA, stashMessage: STASH_MSG, pushAttempted: false, - cleanAllowedDuringRollback: true, }; context.sys.mockExec((cmd) => { if (cmd === "git stash list --pretty=format:%gd %gs") { @@ -370,7 +340,6 @@ describe("finalizeRollback (failure path)", () => { stashSha: STASH_SHA, stashMessage: STASH_MSG, pushAttempted: true, - cleanAllowedDuringRollback: true, }; context.sys.mockExec((cmd) => { if (cmd === "git stash list --pretty=format:%gd %gs") { @@ -401,7 +370,6 @@ describe("finalizeRollback (failure path)", () => { context.rollback = { originalHead: HEAD_SHA, pushAttempted: false, - cleanAllowedDuringRollback: true, }; await finalizeRollback(context, { failed: true }); expect(context.sys.exec).not.toHaveBeenCalled(); @@ -412,7 +380,6 @@ describe("finalizeRollback (failure path)", () => { context.rollback = { originalHead: HEAD_SHA, pushAttempted: false, - cleanAllowedDuringRollback: true, }; await finalizeRollback(context, { failed: true }); expect(context.sys.exec).not.toHaveBeenCalled(); @@ -427,7 +394,6 @@ describe("finalizeRollback (success path)", () => { stashSha: STASH_SHA, stashMessage: STASH_MSG, pushAttempted: false, - cleanAllowedDuringRollback: true, }; context.sys.mockExec((cmd) => { if (cmd === "git stash list --pretty=format:%gd %gs") { @@ -462,7 +428,6 @@ describe("finalizeRollback (success path)", () => { context.rollback = { originalHead: HEAD_SHA, pushAttempted: false, - cleanAllowedDuringRollback: true, }; await finalizeRollback(context, { failed: false }); expect(context.sys.exec).not.toHaveBeenCalled(); @@ -475,7 +440,6 @@ describe("finalizeRollback (success path)", () => { stashSha: STASH_SHA, stashMessage: STASH_MSG, pushAttempted: false, - cleanAllowedDuringRollback: true, }; await finalizeRollback(dry, { failed: false }); expect(dry.sys.exec).not.toHaveBeenCalled(); @@ -486,7 +450,6 @@ describe("finalizeRollback (success path)", () => { stashSha: STASH_SHA, stashMessage: STASH_MSG, pushAttempted: false, - cleanAllowedDuringRollback: true, }; await finalizeRollback(noRb, { failed: false }); expect(noRb.sys.exec).not.toHaveBeenCalled(); diff --git a/packages/core/src/lib/rollback.ts b/packages/core/src/lib/rollback.ts index 38d44c2..b4da249 100644 --- a/packages/core/src/lib/rollback.ts +++ b/packages/core/src/lib/rollback.ts @@ -67,27 +67,15 @@ export async function captureRollbackSnapshot(context: Context): Promise { const { stdout } = await execGit(context, ["status", "--porcelain"]); isDirty = stdout.trim() !== ""; } catch (e: any) { - // Conservatively treat the tree as dirty so rollback won't run a - // destructive `git clean` against state we couldn't observe. context.cli.warn( `Could not determine working tree status for rollback: ${e?.message ?? e}. ` + - `Rollback will skip cleanup of untracked files to avoid data loss.`, + `Local rollback has been disabled for this run to avoid data loss if the release fails.`, ); - context.rollback = { - originalHead, - pushAttempted: false, - cleanAllowedDuringRollback: false, - }; return; } let stashSha: string | undefined; let stashMessage: string | undefined; - // A clean working tree is automatically safe: anything untracked at rollback - // time must have been created by the release. A dirty working tree only - // becomes safe once we've stashed the user's changes as a recovery anchor. - let cleanAllowedDuringRollback = !isDirty; - if (isDirty) { const message = `${ROLLBACK_STASH_MESSAGE_PREFIX}${Date.now()}`; try { @@ -95,13 +83,12 @@ export async function captureRollbackSnapshot(context: Context): Promise { // From this line on, the stash entry exists in `git stash list` and // is identifiable by its message. stashMessage = message; - cleanAllowedDuringRollback = true; } catch (e: any) { context.cli.warn( `Failed to snapshot uncommitted changes for rollback: ${e?.message ?? e}. ` + - `Rollback will undo committed and edited files but cannot restore ` + - `untracked files; cleanup of untracked files will be skipped.`, + `Local rollback has been disabled for this run to avoid data loss if the release fails.`, ); + return; } if (stashMessage) { @@ -138,7 +125,6 @@ export async function captureRollbackSnapshot(context: Context): Promise { stashSha, stashMessage, pushAttempted: false, - cleanAllowedDuringRollback, }; } @@ -208,21 +194,12 @@ export async function finalizeRollback( return; } - // Drop untracked files left behind by plugins (e.g. .commitmessage). Skip - // when we don't have a recoverable backup of pre-existing untracked files, - // since `git clean -fd` would otherwise destroy them. Do not pass -x — that - // would also remove .gitignore'd files like node_modules. - if (state.cleanAllowedDuringRollback) { - try { - await execGit(context, ["clean", "-fd"]); - } catch (e: any) { - context.cli.warn(`Failed to clean untracked files: ${e?.message ?? e}`); - } - } else { - context.cli.warn( - `Skipping cleanup of untracked files because no recovery snapshot is available. ` + - `You may need to remove leftover release artifacts manually.`, - ); + // Drop untracked files left behind by plugins (e.g. .commitmessage). Do not + // pass -x — that would also remove .gitignore'd files like node_modules. + try { + await execGit(context, ["clean", "-fd"]); + } catch (e: any) { + context.cli.warn(`Failed to clean untracked files: ${e?.message ?? e}`); } // Restore the user's pre-release uncommitted changes from the stash From 8fdf9c5f12b8afccc4bacca58f262a3d30325e95 Mon Sep 17 00:00:00 2001 From: Dominic Griesel Date: Wed, 6 May 2026 16:13:25 +0200 Subject: [PATCH 7/7] address review feedback --- packages/plugin-git/src/index.test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/plugin-git/src/index.test.ts b/packages/plugin-git/src/index.test.ts index d690b12..523960c 100644 --- a/packages/plugin-git/src/index.test.ts +++ b/packages/plugin-git/src/index.test.ts @@ -443,7 +443,6 @@ This is the changelog.`); context.rollback = { originalHead: "deadbeef", pushAttempted: false, - cleanAllowedDuringRollback: true, }; context.sys.mockExec(() => ""); @@ -466,7 +465,6 @@ This is the changelog.`); context.rollback = { originalHead: "deadbeef", pushAttempted: false, - cleanAllowedDuringRollback: true, }; context.sys.mockExec((cmd) => { if (cmd.includes("rev-parse --abbrev-ref --symbolic-full-name")) { @@ -490,7 +488,6 @@ This is the changelog.`); context.rollback = { originalHead: "deadbeef", pushAttempted: false, - cleanAllowedDuringRollback: true, }; context.sys.mockExec(() => ""); @@ -509,7 +506,6 @@ This is the changelog.`); context.rollback = { originalHead: "deadbeef", pushAttempted: false, - cleanAllowedDuringRollback: true, }; await gitPlugin.executeStage(context, DefaultStages.push);