feat: revert all changes during failed release process #791

iexos · 2026-02-25T17:04:01Z

iexos commented

2026-02-25 17:04:01 +00:00

closes #683

iexos added 1 commit 2026-02-25 17:04:02 +00:00

feat: recipe release reverts completely on failure

continuous-integration/drone/pr Build is failing

Details

84a8fa2254

decentral1se reviewed 2026-02-27 15:46:17 +00:00

decentral1se left a comment

Looking good, tysm @iexos! A few minor comments below.

cli/recipe/release.go Outdated

						
				@ -317,2 +302,2 @@

							if cleanErr := cleanCommit(recipe, preCommitHead); cleanErr != nil {

								log.Fatal(cleanErr)

							if resetErr := resetCommit(recipe, preCommitHead); resetErr != nil {

								log.Fatal(resetErr)

decentral1se commented

2026-02-27 15:41:20 +00:00

In the case of either cleanTag or resetCommit failing while trying to clean up, we'll see a cryptic error without context. Should we adjust the log.Fatal(cleanErr) / log.Fatal(resetErr) to also include something like unable to clean up after failed release attempt: %s

In the case of either `cleanTag` or `resetCommit` failing while trying to clean up, we'll see a cryptic error without context. Should we adjust the `log.Fatal(cleanErr)` / `log.Fatal(resetErr)` to also include something like `unable to clean up after failed release attempt: %s`

👍 1

iexos marked this conversation as resolved

cli/recipe/release.go Outdated

						
				@ -393,2 +364,2 @@

					if tagString == "" {

						tagString = fmt.Sprintf("%s+%s", tag.String(), mainAppVersion)

					mainService := "app"

					label := i18n.G("coop-cloud.${STACK_NAME}.version=%s", tagString)

decentral1se commented

2026-02-27 15:42:05 +00:00

We can remove the i18n.G(...) wrapper since this string doesn't need to be translated.

That was a previous mistake, I think. You can use a standard fmt.Sprintf instead.

We can remove the `i18n.G(...)` wrapper since this string doesn't need to be translated. That was a previous mistake, I think. You can use a standard `fmt.Sprintf` instead.

👍 1

iexos marked this conversation as resolved

cli/recipe/release.go Outdated

						
				@ -601,3 +579,3 @@

					}

					opts := &git.ResetOptions{Commit: head.Hash(), Mode: git.MixedReset}

					opts := &git.ResetOptions{Commit: head.Hash(), Mode: git.HardReset}

decentral1se commented

2026-02-27 15:44:07 +00:00

Does this still guarantee that we still won't lose "non release related changes"? Can this be covered with a separate integration test as well? I'd rather we are safe than sorry in the case of potential data loss issues. abra has a bit of a bad reptuation in this regard 🙃

Does this still guarantee that we still won't lose "non release related changes"? Can this be covered with a separate integration test as well? I'd rather we are safe than sorry in the case of potential data loss issues. `abra` has a bit of a bad reptuation in this regard 🙃

iexos commented

2026-03-02 09:44:12 +00:00

There are already tests covering staged or unstaged changes in which case release will bail out. Is that what you mean or is there something else you want to test?

There are already tests covering staged or unstaged changes in which case `release` will bail out. Is that what you mean or is there something else you want to test?

👍 1

decentral1se marked this conversation as resolved

iexos force-pushed push-opmswyqxplqx from 84a8fa2254 to 039a29f27c

2026-03-02 09:55:05 +00:00

Compare

decentral1se approved these changes 2026-03-03 00:02:10 +00:00

decentral1se left a comment

👏

decentral1se merged commit 0633f24d1b into main

2026-03-03 00:02:16 +00:00

iexos deleted branch push-opmswyqxplqx

2026-03-03 09:08:57 +00:00

decentral1se referenced this pull request

2026-03-05 08:38:29 +00:00

Failed release SSH leaves uncommitted changes #794

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: toolshed/abra#791

Allow edits from maintainers