Skip to content

fix: make docs backports idempotent - #163

Open
Piotr1215 wants to merge 2 commits into
mainfrom
devops-1803/backport-idempotence
Open

Piotr1215 wants to merge 2 commits into
mainfrom
devops-1803/backport-idempotence

Conversation

@Piotr1215

Copy link
Copy Markdown
Contributor

Summary

Repeated PR signals can recreate a backport after its prior PR merges or after a run leaves a branch without a PR. Check existing ports and use a stable branch name before writing.

Test plan

  • Jest suite, lint, build, and diff check
  • Regression tests cover replay, merged PRs, pagination, target branches, concurrent branch creation, and lookup failures

Related to DEVOPS-1803

Repeated workflow signals can create new timestamped branches when a prior backport left no pull request. Use a stable source and target branch, check every matching open and closed PR plus that branch before writing, and stop on lookup errors.
@Piotr1215
Piotr1215 requested a review from djwfyi November 12, 2026 13:45

@djwfyi djwfyi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

These are the same two issues raised in the forwardport PR review, both of which were addressed there in a924c31:

  • [P1] Retries skip unfinished backports (src/index.ts:184–187). The branch is created before file processing and PR creation. If either fails, the leftover branch causes every retry to skip the version and report success without completing the backport. Please allow recovery of unfinished work while preserving concurrency protection.
  • [P2] The PR lookup stops working at 10,000 closed PRs (src/index.ts:1136–1137). The search scans all repository PRs and throws after 100 full pages, so new backports without an existing match fail once that threshold is reached. Please paginate until exhaustion or narrow the lookup.

Reviewed commit 449fd978dd2a996cc627c45ad60c246fd3adb22e. The existing 83 tests, lint, and build pass with locally installed dependencies. Three additional regression checks reproduce the failures: retry after a file-write error, retry after a PR-creation error, and lookup with 10,000 unrelated closed PRs.

A file-write or PR-create failure leaves unfinished documentation on a branch. Retrying used to skip that branch and report success while the backport stayed incomplete. Recover its original layout and completed writes under an atomic writer lock so retries can finish and publish the missing PR. Remove the PR-page ceiling so repository growth cannot block new backports.

Related to DEVOPS-1803
@Piotr1215

Copy link
Copy Markdown
Contributor Author

Addressed review 5879818809 in 57aa60d.

  1. P1: Retries resume the existing branch. GitHub's merge base restores the original folder layout and version-specific rename content after earlier deletions. A destination written by a failed attempt is accepted only when it was absent at the base and its bytes match the intended write. Prior branch changes also trigger PR creation when a retry has no new writes. An atomic lock ref serializes writers, including concurrent resumes, and the PR lookup runs again inside the lock.
  2. P2: The REST pull-request listing now paginates until exhaustion. The fixed 100-page ceiling is removed, with a regression that finds a matching closed PR after 10,000 unrelated results. Legacy title matching and lookup failure handling are preserved.

Validation: 93 tests pass, including interrupted file writes, failed PR creation, folder deletion/replacement, partial and completed renames, real destination conflicts, concurrent deliveries, and page 101. Eight recovery regressions failed against 449fd97 before the fix. Lint, build, explicit source ESLint, and diff checks pass; dist/index.js is rebuilt.

The README covers hard cancellation: handled failures release the lock, but a killed runner can leave it behind. Confirm that its writers have stopped before removing that lock and rerunning. Recovery branches stay owned by the action so their merge base remains stable.

@Piotr1215
Piotr1215 requested a review from djwfyi November 12, 2026 18:39

@djwfyi djwfyi left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Both reported issues are resolved in 57aa60d. Failed attempts resume safely under the writer lock, and the PR lookup paginates until exhaustion.

Validation: all 93 tests, lint, and build pass with locally installed dependencies. Three additional checks also pass for modified-file recovery, interrupted content-changing rename recovery preserving version-specific content, and lock release when the inner PR lookup finds an existing PR. No further actionable findings.

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.

2 participants