Skip to content

fix: improve partial publishing recovery for CocoaPods and GitHub targets#821

Merged
BYK merged 8 commits into
masterfrom
fix/partial-publish-recovery
May 22, 2026
Merged

fix: improve partial publishing recovery for CocoaPods and GitHub targets#821
BYK merged 8 commits into
masterfrom
fix/partial-publish-recovery

Conversation

@BYK

@BYK BYK commented May 22, 2026

Copy link
Copy Markdown
Member

Summary

Fixes partial publishing recovery issues where Craft loses publishing progress and fails unrecoverably on re-run. Triggered by this CI failure.

Changes

CocoaPods: selective retry for pod trunk push

  • Wraps pod trunk push in withRetry() with 5 retries and exponential backoff (5s initial, 2x factor)
  • Retries only on transient errors (CDN timeouts, 5xx, ETIMEDOUT, ECONNRESET, etc.)
  • Fails fast on permanent errors (spec validation, authentication)
  • Follows the same pattern as the Crates target

GitHub: detect existing releases on re-run

  • Before createDraftRelease(), checks if a release for the tag already exists via new getReleaseByTag() method
  • If a published release exists (from a previous crashed run), skips creation and returns early
  • If a leftover draft release exists, deletes it and creates a fresh one
  • Prevents 422 errors on re-run after a partially successful publish

GitHub: safe rollback with re-fetch

  • Before attempting to delete an "orphaned" draft release on failure, re-fetches the release from GitHub via new getRelease() method to check its actual state
  • If publishRelease() half-succeeded (server processed it but response timed out), the release is now published — skips deletion to avoid destroying a live release
  • Handles immutable releases gracefully (no crash from failed delete attempts)

GitHub: non-fatal floating tag updates

  • Wraps updateFloatingTags() in try/catch after successful publishRelease()
  • Floating tag failures are logged as warnings instead of throwing
  • Prevents marking the target as failed when the release is already live, which would cause duplicate-release errors on re-run

Tests

  • 7 new tests for CocoaPods selective retry (transient vs permanent errors, retry exhaustion)
  • 14 new tests for GitHub target (existing release detection, safe rollback, non-fatal floating tags, getRelease/getReleaseByTag edge cases)

…gets

- CocoaPods: Add selective retry for `pod trunk push` with exponential
  backoff. Retries on transient errors (CDN timeouts, 5xx, network errors)
  but fails fast on permanent errors (spec validation, auth).

- GitHub: Detect existing releases on re-run. If a previous run published
  the release but crashed before persisting state, skip creation instead
  of failing with 422.

- GitHub: Re-fetch release state before rollback deletion. Prevents
  deleting a live release when `publishRelease()` half-succeeded (server
  processed it but response timed out). Also handles immutable releases
  gracefully.

- GitHub: Make floating tag failures non-fatal after successful publish.
  Prevents marking the target as failed when the release is already live,
  which would cause duplicate-release errors on re-run.
@BYK BYK self-assigned this May 22, 2026
Comment thread src/targets/cocoapods.ts Outdated
Aligns with the Crates target naming convention (MAX_ATTEMPTS) and
clarifies that the value represents total attempts including the
initial one, not just retries after the first failure.

@BYK BYK left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed in 2ee4e2a — renamed COCOAPODS_MAX_RETRIES to COCOAPODS_MAX_ATTEMPTS with updated documentation to match the Crates target convention.

Comment thread src/targets/cocoapods.ts
Replace broad patterns like '502', '503', and 'http error' with more
specific ones like '502 bad gateway', '503 service unavailable', and
'server error (5' to avoid misclassifying permanent errors that happen
to contain those substrings.
Comment thread src/targets/cocoapods.ts
Comment thread src/targets/github.ts
When a pod trunk push attempt succeeds on the server but the response
times out, the retry fails with "has already been pushed". Treat this
as success (same pattern as the Crates target with "is already uploaded")
instead of failing the publish.

@cursor cursor Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f3fc912. Configure here.

Comment thread src/targets/github.ts Outdated
- CRITICAL: getReleaseByTag returns 404 for draft releases. Replaced
  upfront draft detection with 422-recovery pattern: catch the 422
  from createDraftRelease, find the conflicting draft via listReleases,
  delete it, and retry. Published release detection via getReleaseByTag
  still works correctly (main recovery scenario).

- MEDIUM: Removed overly broad 'timeout' and 'cdn:' transient error
  patterns. Added 'trunk.cocoapods.org' alongside 'cdn.cocoapods.org'
  for specificity.

- MEDIUM: Added TOCTOU race condition comment in rollback section.

- MEDIUM: Improved 'already published' comment to document that it
  also handles the re-run-of-entire-pipeline case.

- Fixed test mock isolation: use resetAllMocks instead of clearAllMocks,
  use mockRejectedValueOnce consistently to prevent state leaks between
  tests.
Comment thread src/targets/github.ts
Addresses BugBot feedback about pagination. A leftover draft from a
crashed run will be among the most recent releases, so the first page
is sufficient. Increased from 30 to 100 for extra safety margin.
Comment thread src/targets/github.ts Outdated
If deleteRelease fails for all leftover drafts (e.g., immutable
releases), re-throw the original 422 instead of retrying
createDraftRelease which would just produce another 422.
@BYK BYK merged commit e9a5238 into master May 22, 2026
21 checks passed
@BYK BYK deleted the fix/partial-publish-recovery branch May 22, 2026 19:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant