rebase mode's force-push auto-closes open sync PRs — branch content survives, the PR silently leaves the open set #280

Activity

1. mmcky commented on Aug 19, 2026

   

   mmcky

   on Aug 19, 2026

   ContributorAuthor

   More actions

   Severity upgrade: this has already caused undelivered content in production, and it goes back further than the two instances above.

   Found while tidying up after the drain — an orphaned branch on lecture-python.zh-cn with no open PR: translation-sync-2026-08-05T23-15-41-pr-760, 15 commits ahead of main. It belongs to lecture-python.zh-cn#242 ("Add Job Search V lecture on risk-sensitive preferences"), and its event log carries the same signature as the two 2026-08-18 instances:

   Event	Timestamp
   head_ref_force_pushed	2026-08-07T01:34:27Z
   closed	2026-08-07T01:34:27Z
   automation comment "♻️ Automatically rebased after #230 was merged"	2026-08-07T01:34:36Z

   There is no separate manual close event — the only close is the one coincident with the force-push, and the automation's own comment lands nine seconds later announcing the rebase it had just performed.

   The consequence is a missing lecture. A full set comparison of source against target as of today:

   	Count
   lectures/*.md in QuantEcon/lecture-python.myst	145
   lectures/*.md in QuantEcon/lecture-python.zh-cn	143
   Missing from the edition	learning_approximation.md, mccall_risk.md

   learning_approximation is known and tracked (lecture-python.zh-cn#210, blocked on #118). mccall_risk is not tracked anywhere — it is absent from the edition's lectures/ and absent from its _toc.yml, while present in both upstream. It was never delivered by any other route: the PR's other five files were updates to lectures that already existed and have since been superseded, so nothing flagged the gap. The edition has been one lecture short of source for twelve days, and the only reason it surfaced now is that the branch outlived its PR and I went looking for stray refs.

   This is the failure mode the issue describes, realised: the PR left the open set, the branch retained the content, and no stage — not CI, not review mode, not the sync run, not a coverage check — noticed that a lecture the source had was never delivered. It is worth noting the detection gap independently of the race, since any path that drops a lecture would be equally invisible today; a periodic source-vs-target set comparison would catch this class regardless of cause.

   Recovery here should probably not be a straight recreate-the-PR: that branch is 37 commits behind main and, per #276, regenerating or merging stale snapshot content risks reverting newer target edits to the five other lectures it touches. A targeted gap-fill of mccall_risk alone at the current engine looks like the cleaner route.
2. mmcky

   added a commit that references this issue on Aug 21, 2026

   Seed mccall_risk (gap-fill via translate init -f at engine e6e9176) (#…

   ...

   b136ab3
3. quantecon-services

   added

   bugSomething isn't workingSomething isn't working

   on Oct 6, 2026
4. quantecon-services

   added the

   Task

   issue type on Oct 6, 2026
5. quantecon-services

   added a parent issue on Oct 6, 2026

   • Hardening from the 2026-10-05 codebase review: forward resyncs carry edition policy, rebase moves a PR branch in one step #349
6. quantecon-services commented on Oct 6, 2026

   

   quantecon-services

   on Oct 6, 2026 – with Claude

   Collaborator

   More actions

   Adopted on 2026-10-06 as a work item of the project #349, Hardening from the 2026-10-05 codebase review: forward resyncs carry edition policy, rebase moves a PR branch in one step. The 2026-10-05 codebase review found the mechanism in the code. rebaseSinglePR moves the PR branch to main's own SHA before it writes a single file, so every overlapping rebase passes through the state GitHub closes a PR on. The same-second timelines above come from that window being hit. The report and the comment above stand as written; this comment adds the evidence and fixes the scope.

   The item also covers two more ways the same write step loses content: partial writes on an unretried error, and a sibling rebase that races a merge and silently reverts it. It also covers the rebase workflow's concurrency block, which today cancels the one run that would repair the second. Sync PR creation is out of scope.

   Where

   At 415cf87, the write step of rebaseSinglePR (src/index.ts:551-614), condensed:

   // :556-562 — main is read only now, after translation has finished
   const { data: mainRef } = await octokit.rest.git.getRef({ owner, repo, ref: `heads/${defaultBranchData.default_branch}` });
   const newBaseSha = mainRef.object.sha;
   
   // :564-571 — "Reset the branch to current main": zero commits ahead from here
   await octokit.rest.git.updateRef({ owner, repo, ref: branchRef, sha: newBaseSha, force: true });
   
   // :573-593 — then a GET and a commit per file; :595-610 — a deleteFile commit per removal
   for (const file of result.translatedFiles) {
     /* fetchFileContent(..., branchName) for the SHA */
     await octokit.rest.repos.createOrUpdateFileContents({ /* ... */ branch: branchName, sha: fileSha });
   }

   • src/index.ts:291, :326, :378 and :403 read the sibling's target files with no ref. fetchFileContent (:901-913) then reads whatever the default branch holds at that moment. That need not be the main the branch is later reset onto.

   • src/index.ts:238 is the failure comment: "You may need to manually resolve conflicts or run /translate-resync on the source PR."

   • src/index.ts:116-122 and :165-166: each run checks overlap only against the merged PR that triggered it.

   • examples/rebase-translations.yml:43-46 has no queue: key:

         # Prevent concurrent rebases from overlapping
         concurrency:
           group: rebase-translations
           cancel-in-progress: false

     translate setup copies the file verbatim (src/cli/commands/setup.ts:319, :372-375), and the E2E harness renders it (tool-test-action-on-github/test-action-on-github.sh:273).

   Two throws guard the reset: the content-fetch throw at :428-434 (#165) and the translation-error throw at :537-544 (#160). Both cover reads and translation, and nothing covers the writes after the reset. Nothing in src/ calls createTree or createCommit. The octokit is a plain github.getOctokit with no retry or throttling plugin. dist-action/index.js ships the same sequence.

   Failure scenario

   1. Auto-close (this issue). From the reset until the first file commit lands, the branch has no commits ahead of main. That window lasts at least two API round trips. If GitHub evaluates the PR inside it, GitHub closes the PR, and it does not reopen it when the branch is repopulated. Most rebases miss the window, but production has four instances, each with head_ref_force_pushed and closed in the same second:

   • lecture-python.zh-cn#240 and lecture-python.zh-cn#247 (2026-08-18, above).
   • lecture-python.zh-cn#242 (2026-08-07, the comment above). mccall_risk.md stayed out of the edition for 12+ days (it is present today).
   • lecture-intro.zh-cn#279 (2026-08-10, Six defects observed syncing a six-PR batch to lecture-intro.zh-cn (declared files silently dropped, toc clobbered, .bib never synced) #256 defect 4). There, "the PR's recorded head became the base branch's own tip", which is exactly the state :564-571 creates.

   2. Partial write. A write in the loop can fail with a 5xx, a secondary rate limit, or a 409 on a blob SHA read just after the reset (.qe/dev/PLAN.md Phase 4 already lists the 409). None of these is retried. That PR's rebase then aborts with the branch at main plus k of N files. At k = 0 the PR auto-closes as in (1). Otherwise it stays open, looks routine, and lacks the remaining translations. The :238 comment advises resolving conflicts and does not say the branch was reset. A failed deletion does not even abort the rebase: the catch at :607-609 logs any error as "Not found on branch — skip deletion", and the file stays on the branch.

   rebaseSinglePR cannot be imported into a test today (#169). The review therefore ran the same per-file pattern in createTranslationPR against a mock octokit, and re-ran it on 2026-10-06 against dist/pr-creator.js at 415cf87. A 409 on the third of four files (lectures/c.md) threw after two commits had landed, leaving the branch holding lectures/a.md and lectures/b.md only.

   3. Concurrency. A concurrency group holds one running and one pending run. Unless queue: max is set, a new arrival cancels the pending run. Suppose B and C merge while run A (triggered by A's merge) is still in progress. Run B is cancelled. A sibling D that shares a file only with B is never rebased, so it stays conflicted. The review modelled this against the run-queue rule and the overlap check at :165-166:

   queue: single (current template) { runs: [ 'A', 'C' ], cancelled: [ 'B' ], D_rebased: false }
   queue: max                       { runs: [ 'A', 'B', 'C' ], cancelled: [], D_rebased: true }
   

   The worse case is silent:

   1. Run A is rebasing D, which overlaps A. It reads D's target files from pre-B main.
   2. B merges.
   3. :556-562 reads the new main, and :564-571 resets D onto it.
   4. D's files, built from pre-B content, are committed on top.

   D now shows no conflict and quietly undoes B's sections in the shared file. The :225 comment still tells the PR that only unchanged sections were updated to match current main. Run B would have rebased D again and repaired it, and run B is the run that C cancels. Nothing in the run reports the revert. Review re-runs on the rebased head only where the rebase workflow passes the PAT (#125); lecture-python-programming.ml and .ja still pass GITHUB_TOKEN, under which it does not. Unless a reviewer spots B's sections going backwards in D's diff, merging D ships the revert to readers.

   Why it matters

   In August 2026, four sync PRs on two public editions were closed silently. One loss kept mccall_risk out of lecture-python.zh-cn for 12+ days without any stage noticing. #287's state drift (mccall_persist_trans.md) followed from the same lost PR. A closed PR reports changed_files: 0, so an audit reads it as already merged. Recovery means recreating the PR by hand, then restoring its labels and body and re-firing review.

   A partial write produces an open PR that looks complete. The concurrency case bites during backlog drains (the 29-PR drain above is the pattern), and #63 measured a 62% conflict rate on fa sync PRs, so overlaps are routine. An org-wide code search on 2026-10-06 found the same concurrency block, with cancel-in-progress: false and no queue: key, in the rebase-translations.yml of all seven production editions: lecture-intro.zh-cn, lecture-python.zh-cn, and lecture-python-programming.{zh-cn,fa,fr,ml,ja}.

   Fix

   Only the rebase write step changes. The fix removes the mechanism instead of detecting it afterwards: the branch never equals main.

   1. A commit module. Add a new module, e.g. src/git-commit.ts, and register it in docs/developer/architecture.md, which docs-module-map.test.ts enforces. From a base SHA, the module builds a tree and a commit:

      const { data: base } = await octokit.rest.git.getCommit({ owner, repo, commit_sha: baseSha });
      const { data: tree } = await octokit.rest.git.createTree({
        owner, repo, base_tree: base.tree.sha,
        tree: [
          ...files.map((f) => ({ path: f.path, mode: '100644', type: 'blob', content: f.content })),
          ...deletionsInBase.map((path) => ({ path, mode: '100644', type: 'blob', sha: null })),
        ],
      });
      if (tree.sha === base.tree.sha) return { unchanged: true };
      const { data: commit } = await octokit.rest.git.createCommit({
        owner, repo, message, tree: tree.sha, parents: [baseSha],
      });

      createTree fails on a sha: null entry for a path that is absent from the base tree. Deletions are therefore filtered to paths present in it, and a dropped path gets an info log, as today's loop does for not-found. None of these calls moves a ref, so they can sit inside a small bounded retry on 5xx and secondary rate limits.

   2. The rebase write. rebaseSinglePR reads main's SHA once, at its start, and uses that SHA as the ref for every target read and as the commit's parent. A single forced git.updateRef then moves the branch from its old head straight to the new commit, and pulls.get confirms the PR is still open.

      With the pin, a merge that races the rebase leaves D's branch behind main. The result is a visible conflict or a clean three-way merge, never a silent revert, and the queued run for that merge then repairs it. PR Preserve localised _toc.yml part captions on sync #291 changes the _toc.yml target read at :326 to carry the target's captions forward, so that read starts feeding content too. Whichever of the two PRs lands second passes the pinned ref there.

   3. The template. Add queue: max to the rebase concurrency block. Per GitHub's changelog of 2026-05-07, it queues up to 100 pending runs in order and cannot be combined with cancel-in-progress: true; the template already sets false. translate setup and the harness pick it up from the template.

   Order matters. Restoring every queued run also restores every force-push, so deployed editions get the queue setting only after the atomic write has been released and the floating tags have moved. This respects D-2026-04-01-rebase-on-merge. The queue serialises workflow runs; it does not serialise PR merges, which is what the merge queue rejected there would have done. The decision's "merge order doesn't matter" also assumes that every merge's run executes, and the queue restores that.

   The fix has two side effects to accept: one commit per rebase instead of one per file, and file.sha / fetchExistingStateShas going unused on this path. The item also settles PLAN.md Phase 4's "[L] Rebase force-push races": no blob SHA is read after a reset, and runs queue instead of being dropped.

   It is also how #259's "rebase refuses to force-push a branch to/behind base" clause (#256.4) gets delivered. As worded, that guard cannot sit in front of today's sequence, because the sequence always passes through base. The atomic write plus the tree-equals-base check does the job.

   Out of scope:

   • sync PR creation (createTranslationPR): PR Remove failed-new-file TOC entries before opening sync PR #289 edits it, and its idempotency belongs to PR creation reports failure when the GitHub API times out after succeeding (false negative; naive retry would duplicate) #92. On its own, a mid-loop failure there fails loudly and leaves only a stray branch;
   • the read step and mode dispatch (tech-debt T1: Make the Action entry point importable #169);
   • the rest of the failure comment's recovery wording, which goes with The failure-issue template prescribes \translate-resync, which is unsafe on 9 of 12 PRs and ineffective on 13 of 15 failures #281;
   • refreshing the PR body after a rebase;
   • removing the unused file.sha plumbing;
   • decomposing index.ts;
   • recovering content already lost to the race.

   Acceptance criteria

   • No code path sets a rebased PR's branch ref to main's own SHA. The branch moves exactly once per rebase, from its old head to a commit whose single parent is the pinned main SHA. No createOrUpdateFileContents or deleteFile call remains on the rebase path.
   • If the rebased tree equals main's tree, nothing is pushed. The PR is closed deliberately, with a comment that says nothing was left to carry and names the files the PR declared (Six defects observed syncing a six-PR batch to lecture-intro.zh-cn (declared files silently dropped, toc clobbered, .bib never synced) #256 defect 4).
   • After the ref move, the run reads the PR back. If the PR is no longer open, that is an error that fails the run and names the PR and the branch. There is no attempt to reopen it.
   • A failure while building the commit leaves the branch where it was, and the failure comment says the branch was not changed.
   • Mock-octokit unit tests cover two areas:
     • the commit module: additions, deletions (including a path absent from the base tree, dropped with an info log), and a retry on 5xx in the ref-free calls only;
     • the rebase ordering: a single updateRef that never targets main's SHA, the tree-equals-main path, a post-check that fails on a closed PR, and a commit-build failure that makes no updateRef call.
   • The concurrency block in examples/rebase-translations.yml declares queue: max alongside cancel-in-progress: false, and src/cli/__tests__/workflow-templates.test.ts asserts it. The key is confirmed accepted on a harness target repository before any edition is patched.
   • CHANGELOG [Unreleased] has a Fixed entry (one commit per rebase instead of one per file), and dist-action/ is rebuilt with the drift check green.
   • The issue closes when, in this order:
     1. The fix ships in a release per the checklist, with the floating tags moved.
     2. Only after that, every deployed edition's rebase-translations.yml is patched with the queue setting, and the list of repositories is recorded on this issue.
     3. On a harness target repository, two sync PRs are merged back to back while a third sync PR overlaps both. The third stays open, rebased and complete, and its timeline shows no same-second head_ref_force_pushed/closed pair.

   The body's suggestion to "reopen it if not" is superseded. A reopen fails once the branch has been re-pushed (see this issue's own "Recovery is not simply reopen" section), and the fix removes the pass through base instead of repairing it afterwards.

   Refs: #349, #256 (defect 4), #259 (the #256.4 clause of its state-not-response box), #92, #169, #281, #287, #276, #125, #63, #160, #165, PR #289, PR #291, .qe/dev/PLAN.md Phase 4, .qe/dev/decisions/D-2026-04-01-rebase-on-merge.md

   ---

   Generated by Claude Code
7. mmcky

   added 3 commits that reference this issue on Oct 9, 2026

   dev: 2026-10-05 codebase review — logs, hardening tracker #349, NEXT-…

   ...

   e9d51a0

   dev: log — handover of the review follow-up's GitHub updates to a loc…

   ...

   de17858

   dev: run the 2026-10-06 review handover after #350 and #353 merged (#358

   ...

   ed7af92