fix: sync-main-to-experimental action correct base branch

[pravusjif]

fix: sync main into experimental as a real merge

Problem

The sync job reset chore/sync to main's HEAD instead of merging into experimental, so the branch — and the @dcl/protocol package built from it — carried none of experimental's content. Regenerating in the Explorer against that package deleted experimental-only schema, e.g. public enum AvatarEmoteMask in AvatarShape.gen.cs.

Squashing the sync PR breaks it a second way: main stops being an ancestor of experimental, the merge base freezes, and later syncs conflict on content experimental already has.

Changes

sync-main-to-experimental.yml — builds chore/sync as experimental + main

• fetch-depth: 0 and a git identity, so the merges work at all
• branches from experimental, or reuses an in-flight chore/sync so a manual conflict resolution survives later pushes to main
• pushes whenever the recomputed branch differs from the remote; opens the PR only when there is something left to propose — commits ahead of experimental and a differing tree, so a net-zero round of main changes cannot leave an empty PR behind
• a conflict aborts, pushes nothing, and fails with recovery instructions
• the PR body warns against the Squash button

merge-sync-to-experimental.yml — new, manual (workflow_dispatch), lands the sync as a merge commit

allow_merge_commit=false means the PR button can only squash. That setting governs the button, not git push, and experimental is unprotected — so the workflow merges chore/sync into experimental with --no-ff, pushes, and deletes the branch. The repo setting stays false; no more flipping it on and off to land these.

Full cycle

1. Someone merges a PR into main.
2. The sync workflow builds chore/sync and opens the sync PR into experimental. build-deploy publishes the tarball and comments the install URL; validate-compatibility runs.
3. More commits land on main → they are merged onto the same chore/sync, so the open PR just accumulates them.
4. Test the tarball in the Explorer, review the PR.
5. When it's good: Actions → Merge sync PR into experimental → Run workflow. Not the green button.
6. The workflow pushes the merge commit. GitHub closes the PR as Merged once its commits are reachable from experimental, then the workflow deletes chore/sync.
7. The next push to main starts a fresh cycle from experimental.

Step 6's deletion matters: the sync workflow reads "chore/sync exists" as "a sync is still in flight". GitHub's delete_branch_on_merge only fires for button merges, not for a PR closed by a push, so the workflow does it explicitly — including on its already-merged no-op path, so a run whose cleanup failed can be re-run to finish the job.

Known residual (accepted)

If main adds and then reverts a change while a sync PR is already open, that PR stays open showing 0 files changed until the next real commit to main refills it, and merging it in that state records an empty merge commit. The branch is still pushed and the PR stops being commented on, so nothing goes stale or wrong — it is only noise, and it clears itself. Closing the PR automatically was considered and rejected: it would churn a PR that is about to become valid again, and landing those commits keeps the two histories linked.

If it gets squashed anyway

Nothing blocks the button. Restore the ancestry with a no-content merge that records main as a parent:

git checkout experimental
git merge -s ours origin/main -m "chore: record main ancestry"
git push origin experimental

[github-actions]

Test this pull request

• The @dcl/protocol package can be tested in scenes by running

  npm install "https://sdk-team-cdn.decentraland.org/@dcl/protocol/branch//dcl-protocol-1.0.0-34244183810.commit-b5e78ae.tgz"

[decentraland-bot]

Review: fix: sync-main-to-experimental action correct base branch

Files changed: 1 (+67 −5) — .github/workflows/sync-main-to-experimental.yml
CI: All checks passing ✅
ADR-6: PR title (fix: …) and branch (fix/sync-main-to-experimental-action) follow semantic conventions ✅

---

Problem & Fix Assessment

The old workflow was fundamentally broken: git checkout -B chore/sync without specifying a start point created chore/sync at main's HEAD, discarding all experimental-only content. The new approach correctly builds chore/sync as experimental + main — experimental-only protos survive the sync.

Analysis

✅ Architecture & Merge Strategy

• Correct base: branching from origin/experimental (or reusing origin/chore/sync when a PR is already open) ensures experimental-only content is preserved.
• Uniform merge loop: the for REF in origin/experimental origin/main loop handles both fresh and reuse paths cleanly. When starting from origin/experimental, the first merge is a no-op ("Already up to date") — correct and harmless.
• Conflict handling: merge --abort || true + ::error:: + exit 1 fails cleanly instead of pushing broken state.
• No-op detection: git diff --quiet origin/experimental HEAD correctly skips the push and PR creation when experimental already contains everything in main.

✅ Race Condition Prevention

• Concurrency group (cancel-in-progress: false) ensures two pushes to main queue instead of racing on the push to chore/sync.
• --force-with-lease instead of --force guards against overwriting manual conflict resolutions pushed between the fetch and the push. Significant safety improvement over the old --force.

✅ Edge Cases Verified

Scenario	Behavior
First run, no existing PR	Branches from experimental, merges main, creates PR
Existing PR open, chore/sync exists	Reuses branch, preserves manual conflict resolution
PR was closed but chore/sync still exists	PR_NUMBER is empty → falls back to origin/experimental (fresh start)
Nothing to sync	Prints message, sets changed=false, skips push + PR
Merge conflict	Aborts merge, emits ::error::, exits 1
Manual push to chore/sync during run	--force-with-lease rejects the push (correct — protects manual work)
workflow_dispatch trigger	Works identically to push trigger

✅ Security

• No secrets exposure: only GITHUB_TOKEN (automatically provisioned).
• Minimal permissions: contents: write (push), pull-requests: write (PR create/comment), issues: write (needed by gh pr comment/--label which internally hit the Issues API). All justified.
• No injection vectors: no untrusted user input flows into shell commands. The workflow triggers are push (to main) and workflow_dispatch — both trusted.
• No hardcoded credentials or sensitive data in logs.

✅ Shell Scripting Quality

• set -euo pipefail on all run blocks.
• Variables properly quoted ("$PR_NUMBER", "$BASE").
• // empty in jq correctly produces empty string (not null) when no PR exists.
• git rev-parse --verify --quiet safely checks branch existence without error output.
• Step outputs via $GITHUB_OUTPUT (modern GitHub Actions pattern, not deprecated set-output).

ℹ️ Why the git identity is required

The old workflow never created commits — it only did checkout -B and push. The new workflow creates merge commits (git merge --no-edit -m "...") and git requires an author identity for that. The github-actions[bot] user with email 41898282+github-actions[bot]@users.noreply.github.com is the standard GitHub Actions bot identity (41898282 is the bot's numeric user ID). This is the correct and conventional approach.

✅ Consumer Impact

This change modifies an internal CI workflow only. No public API surfaces, exported packages, or schemas are affected. No downstream consumer impact.

Verdict: APPROVE ✅

No P0 or P1 issues found. The fix is well-designed, handles edge cases properly, and includes multiple safety improvements over the original (--force-with-lease, concurrency group, conflict detection, no-op skip). The PR description is thorough and accurately explains both the problem and the solution.

---

Reviewed by Jarvis 🤖 · Requested by Gabriel Díaz (<@U03MGHMAJL8>) via Slack

[decentraland-bot]

✅ Approved by Claude, approved by Codex — fixes the sync-main-to-experimental workflow to build chore/sync as experimental + main instead of force-pushing at main's HEAD, preserving experimental-only schema.

Checked: merge logic for both fresh-branch and existing-PR paths converges to the correct experimental+main tree; --force-with-lease guarded by the wildcard-refspec fetch immediately before it; concurrency group (cancel-in-progress: false) serializes concurrent pushes to main; set -euo pipefail + git merge --abort || true + exit 1 fails safely on conflicts; git diff --quiet origin/experimental HEAD correctly compares final tree state; PR_NUMBER is jq-derived and double-quoted everywhere; permissions block is minimal (contents, pull-requests, issues: write); step outputs (pr_number, changed) are correctly written and consumed; changed left unset on sync-step failure correctly skips the PR step; no shell injection vectors; CI green (Deployment Notification, Validate compatibility, check_and_build).

---

Cross-model review by Jarvis 🤖 · head 5ece7fe · Claude + Codex · Requested by Charly (<@U0747ARK5TM>) via Slack

[charly-bg]

The diagnosis is right and I confirmed the two git facts it rests on: checkout -B chore/sync with no start point did sit at main's HEAD, and bot-created chore/sync PRs do get build-deploy runs (10 historical pull_request runs on headBranch=chore/sync), so the broken package is real. The core fix — fetch-depth: 0, branch from origin/experimental, merge origin/main — produces the same content shape as the manual PRs #475/#480, and validate-compatibility passes on that shape (verified via #480's run).

Three things to settle before merging.

P1 — after a conflict the workflow can never open the PR

Create or update PR is gated on changed == 'true', so a conflict's exit 1 skips it. The reuse path (L55) needs an already-open PR. First conflict is therefore a closed loop: a human must create chore/sync, resolve, push, and open the PR by hand before the job can self-heal — and the ::error:: names a branch that doesn't exist on the remote at that point, because nothing was pushed. main stays red until then (the job runs on push: main).

Either push the conflicted state to a side ref, or spell out all four recovery steps in the annotation.

P1 — the no-op gate suppresses the push, not just the create (L71-75)

git diff --quiet origin/experimental HEAD decides whether to push and whether to touch the PR. Sequence: run 1 pushes chore/sync and opens the PR → main reverts the change → run 2 rebuilds from origin/chore/sync, merges the revert in, tree now matches experimental → changed=false → the correct branch is discarded and the stale remote chore/sync stays as the PR head, still proposing the reverted content. Merging it re-introduces the revert into experimental. No comment, no failure, nothing clears it.

The check exists to dodge gh pr create's "No commits between…", so gate the create on it. If you want a push gate too, the comparand is origin/chore/sync vs HEAD.

P1 (precondition, not a defect in this diff) — the repo is squash-only as of today

allow_merge_commit=false, allow_rebase_merge=false, allow_squash_merge=true right now — but experimental's tip is 4f4e0ab4, a 2-parent Merge pull request #480 merged at 12:52 today, so the flip is hours old. No ruleset or required_linear_history is forcing it.

Real merge commits are what keep main an ancestor of experimental (merge_base == main today) and keep the merge base advancing. Under squash-only that linkage breaks and git merge origin/main starts conflicting on content experimental already has from main — within about two syncs. The old job had the same broken linkage but never merged locally, so it failed silently; this PR turns it into a hard red. Worth confirming merge commits are coming back before this lands.

P3 — non-blocking

• L42-46 are dead code, and the comment's premise is false. actions/checkout@v4 does git remote add origin (which writes the wildcard remote.origin.fetch) and, with fetch-depth: 0, fetches +refs/heads/*:refs/remotes/origin/* via getRefSpecForAllHistory; nothing in checkout ever narrows remote.origin.fetch. The narrowing only happens at the default fetch-depth: 1, which this diff removes. The stale info failure is real (I reproduced it with a narrowed refspec) but unreachable once fetch-depth: 0 is set. Delete both lines and the comment.
• --force-with-lease doesn't protect what L78-79 claims. The lease resolves through the tracking ref that L46 refreshes moments earlier, so it's satisfied by whatever is on the remote. Reproduced: human commit on chore/sync, PR closed → BASE=origin/experimental → + f5c9690...445f29b (forced update), human work gone. The thing that actually preserves manual work is the BASE=origin/chore/sync reuse at L56. Keep the lease, but drop the claim.
• issues: write is unnecessary and its comment is wrong: gh applies --label via GraphQL updatePullRequest{labelIds} and comments via addComment(subjectId: <PullRequest>) — both under pull-requests. contents: write + pull-requests: write cover everything the job does.
• --no-edit is redundant with -m (L64). --set-upstream (L80) is pointless — the workspace is discarded each run and it has no bearing on the lease.
• A real lease race rejects with a bare (stale info) and a red job, no ::error::. Same for a transient gh pr list failure under set -e — the sync is silently skipped until the next push.
• PR_NUMBER is read at L48 and consumed at L90. If the PR merges in between (delete_branch_on_merge=true makes merge-then-push-to-main the normal sequence) the comment lands on a closed PR and no new PR is opened that run. Narrow, self-heals.
• The ::error:: says "Conflict" for any merge failure, including unrelated histories or a dirty tree.
• The base version's --jq '.[0].number' never emitted "null" in practice (run 33861349214's log shows the empty-PR path working), so // empty is hardening rather than a fix.

Checked and cleared

concurrency (GitHub keeps one pending run per group, but each run recomputes from origin/main's tip, so nothing is dropped) · set -euo pipefail interactions (if ! git merge and git rev-parse --quiet are in condition context) · git merge --abort || true after a non-conflict failure (exit 128, correctly swallowed; post-abort state clean, nothing pushed) · --force-with-lease on a brand-new remote branch with no tracking ref (pushes as a create — which is the repo's state right now, chore/sync is 404) · auto-pr label exists · fetch-depth: 0 cost (repo is 1.3 MB) · push permissions (no rulesets, chore/sync and experimental unprotected, token can push) · the new permissions: block only narrows the repo default · output plumbing (pr_number written before every early exit, passed via env: so no injection surface) · the current changed=false first-run case is correct (behind_by=0 today, so the next run legitimately no-ops).

Simplicity

Over-built by roughly ten lines, and the excess is where the bugs cluster — the reuse-existing-branch machinery accounts for most of the findings above. Keying "rebuild vs. keep" on whether the remote branch already contains origin/main's tip, rather than on whether a PR happens to be open, would remove the PR lookup, the two-ref loop and the cross-step PR_NUMBER hand-off at once. Dropping the sync branch entirely for gh pr create --base experimental --head main is not viable — conflict resolutions would have to land on main.

[decentraland-bot]

Re-review: fix: sync-main-to-experimental action correct base branch

Files changed: 2 — .github/workflows/sync-main-to-experimental.yml (+83 −6), .github/workflows/merge-sync-to-experimental.yml (new, +67)
CI: Pending (check_and_build, Validate compatibility — both in progress)
ADR-6: PR title (fix: …) and branch (fix/sync-main-to-experimental-action) follow semantic conventions ✅
Head: a364e95 (includes "mini refactor based on PR feedback")

---

What changed since the last review (5ece7fe → a364e95)

The latest commit addresses the two code-level P1s from @charly-bg's review:

charly-bg finding	Status
P1 — conflict can never open the PR (step-output gating + incomplete error message)	✅ Fixed — step-output gating removed; PR step always runs; ::error:: now includes full recovery instructions
P1 — no-op gate suppresses push, not just PR create (git diff --quiet gated both)	✅ Fixed — push gate now compares HEAD vs origin/chore/sync; PR-create gate uses merge-base --is-ancestor HEAD origin/experimental
P1 — repo is squash-only (precondition)	ℹ️ Precondition, not a code defect — PR body now includes explicit squash warning + recovery instructions

Additional cleanup: issues: write removed, --no-edit removed, --set-upstream removed, --force-with-lease claim dropped, branch-reuse decoupled from PR existence (just checks branch existence).

---

Analysis of current state

✅ Core merge logic

The fundamental fix is correct: branching from origin/experimental (or reusing origin/chore/sync) and merging origin/main into it preserves experimental-only content. The for-loop approach handles both fresh and reuse paths uniformly.

✅ Concurrency & race protection

• concurrency groups with cancel-in-progress: false serialize pushes to main correctly.
• --force-with-lease correctly rejects the push if a human pushed to chore/sync between fetch and push (verified: the tracking ref from the fetch is the lease comparand).

✅ Push & PR gate separation

• Push decision: HEAD vs REMOTE_HEAD — pushes whenever the recomputed branch differs from the remote. This fixes charly's scenario (main reverts → stale remote stays). ✅
• PR-create decision: merge-base --is-ancestor HEAD origin/experimental — skips PR creation when experimental already contains everything. ✅

✅ Security

• No injection vectors: All dynamic values are either hardcoded strings or integers from gh API. No untrusted input flows into shell commands.
• Permissions minimal: contents: write + pull-requests: write (sync), contents: write + pull-requests: read (merge). All justified.
• Triggers safe: push on main (requires repo write access) and workflow_dispatch (same). No pull_request_target or other external-triggerable events.
• No secrets exposure.

✅ Conflict handling

merge --abort || true + expanded ::error:: with four recovery steps + exit 1. Clean, nothing pushed on conflict. The recovery instructions correctly cover both the "branch exists" and "branch doesn't exist" cases.

---

Remaining findings (all P2)

[P2] Merge workflow no-op path skips cleanup

merge-sync-to-experimental.yml: when --is-ancestor origin/chore/sync origin/experimental is true (already merged), the workflow exits 0 without deleting chore/sync or affecting the PR. This is reachable after a partial failure (experimental push succeeds, branch delete fails in a prior run) or an external merge. Re-running the merge workflow always hits the same no-op exit.

Self-heals on next meaningful push to main (sync workflow recomputes and pushes). Manual fix is trivial (git push origin --delete chore/sync). Consider adding git push origin --delete chore/sync || true to the no-op path for robustness.

[P2] Redundant git config --replace-all in both workflows

With fetch-depth: 0, actions/checkout@v4 already sets remote.origin.fetch to the wildcard refspec via getRefSpecForAllHistory. The git config --replace-all line is dead code (as charly noted). The git fetch that follows is useful (refreshes refs), but the config line can be removed. Harmless, but noise.

[P2] Empty-diff PR lingers after net-zero main changes

If main adds then reverts a change while a sync PR is open, chore/sync accumulates merge commits not in experimental's graph even though the tree is identical. --is-ancestor (commit-graph-based) returns false, so the PR stays open with 0 files changed. Merging it creates an empty merge commit on experimental — harmless but noisy. A supplementary git diff --quiet origin/experimental HEAD check could catch this and skip PR maintenance.

---

Verdict: APPROVE ✅

No P0 or P1 issues. The latest commit substantively addresses both code-level P1s from the prior review. The push/PR gate separation is correct, conflict recovery is well-documented, and the two-workflow design (sync creates the branch, merge lands it as a real merge commit) is sound for working around the squash-only constraint. The P2 items above are worth addressing but do not block merge.

Note: charly's third P1 (squash-only repo setting) is a precondition — worth confirming merge commits are re-enabled or that the team is committed to using the merge workflow exclusively, but not a defect in this diff.

---

Reviewed by Jarvis 🤖 · Requested by Pravus (<@UDJQDQC0Z>) via Slack

[decentraland-bot]

Re-review (round 3): fix: sync-main-to-experimental action correct base branch

Head: d80fdb7 ("tackled review feedback")
CI: All checks passing ✅ (check_and_build, Validate compatibility, Deployment Notification)
ADR-6: ✅

---

Changes since last review (a364e95 → d80fdb7)

All three P2 findings from my prior review are addressed:

P2 finding	Status
Merge workflow no-op path skips cleanup	✅ Fixed — git push origin --delete chore/sync || true added to the --is-ancestor exit path
Redundant git config --replace-all in both workflows	✅ Fixed — removed from both files
Empty-diff PR lingers after net-zero main changes	✅ Fixed — PR-create guard now combines --is-ancestor with git diff --quiet origin/experimental HEAD

Verification of new logic

Merge workflow cleanup (L46-48):

if git merge-base --is-ancestor origin/chore/sync origin/experimental; then
    echo "experimental already contains chore/sync; nothing to merge."
    git push origin --delete chore/sync || true
    exit 0
fi

Correct. The || true handles the case where the branch is already gone. Recovers from partial failures (prior run pushed experimental but failed on branch delete).

Sync workflow tree check (L94-98):

if git merge-base --is-ancestor HEAD origin/experimental \
   || git diff --quiet origin/experimental HEAD; then
    echo "experimental already has everything chore/sync would propose."
    exit 0
fi

Correct. First condition catches commit-graph containment. Second condition catches tree-identical-but-different-history (the net-zero revert scenario). || means either skips PR creation/update. The push step still runs independently, so the remote chore/sync branch is always up to date.

git config --replace-all removal: Both workflows now rely on actions/checkout@v4 with fetch-depth: 0 to set the wildcard refspec, followed by git fetch --no-tags --force --prune origin to refresh refs. Correct — the explicit config was redundant.

Verdict: APPROVE ✅

No P0 or P1 issues. All prior P2s resolved. CI green. The workflow logic is sound:

• Push gate (HEAD vs remote chore/sync) ensures the remote branch is always current
• PR-create gate (commit-graph + tree-diff) avoids both empty PRs and gh pr create failures
• Merge workflow cleans up stale branches on both the happy path and the already-merged path
• Conflict recovery instructions are comprehensive
• Concurrency groups, --force-with-lease, and set -euo pipefail provide proper safety

---

Reviewed by Jarvis 🤖 · Requested by Pravus (<@UDJQDQC0Z>) via Slack

[decentraland-bot]

Re-review (round 4): fix: sync-main-to-experimental action correct base branch

Head: d80fdb7 ("tackled review feedback")
CI: All checks passing ✅ (check_and_build, Validate compatibility, Deployment Notification)
ADR-6: Title (fix: …) and branch (fix/sync-main-to-experimental-action) follow conventions ✅
Security: No issues found — minimal permissions, no injection vectors, no secrets exposure ✅

---

Changes since last review

No new commits since d80fdb7. Re-reviewing at Gabriel's request with fresh parallel sub-agent analysis (shell scripting correctness + architecture/edge-case review).

Core design assessment

The two-workflow design is sound:

• Sync workflow (push to main / workflow_dispatch) → builds chore/sync as experimental + main, opens a reviewable PR, triggers CI
• Merge workflow (workflow_dispatch only) → lands chore/sync into experimental as a real --no-ff merge commit, bypassing the squash-only PR button

This correctly solves the squash-only constraint: allow_merge_commit=false governs the GitHub UI button, not git push. Since experimental is unprotected, the merge workflow can push a two-parent merge commit directly.

Verified edge cases

Scenario	Behavior	OK?
First run, no existing PR	Branches from experimental, merges main, pushes, creates PR	✅
Existing chore/sync (sync in flight)	Reuses branch via BASE=origin/chore/sync, preserves manual conflict resolutions	✅
chore/sync exists but PR was closed	rev-parse finds branch → reuse; PR step finds no open PR → creates new one	✅
Nothing to sync (main ⊆ experimental)	--is-ancestor + git diff --quiet → skips PR step cleanly	✅
Merge conflict	merge --abort || true + ::error:: with recovery steps + exit 1	✅
Human pushes to chore/sync during sync run	--force-with-lease rejects; human work preserved	✅
Merge workflow re-run after partial failure	--is-ancestor detects already-merged, cleans up branch, exits 0	✅
Two rapid pushes to main	Concurrency group queues; latest run fetches all changes	✅
Squash button used accidentally	PR body warns prominently; recovery instructions included	✅ (process)

Shell scripting correctness

• set -euo pipefail interaction with conditionals is correct throughout — all git merge-base, git diff --quiet, git rev-parse --quiet calls are guarded by if or ||
• ${PR_NUMBER:+ (#$PR_NUMBER)} is safe under set -u (:+ does not trigger nounset)
• PR_BODY with backticks in YAML | block scalar is safe — double-quoted expansion does not re-evaluate for command substitution
• jq '.[0].number // empty' correctly produces empty string when no PR exists
• git fetch --force on fetch (not push) correctly handles non-fast-forward remote-tracking ref updates

Remaining findings (all P2 — non-blocking)

[P2] Missing || true on final branch deletion in merge workflow
merge-sync-to-experimental.yml last line: git push origin --delete chore/sync — if this fails after the critical git push origin experimental already succeeded, the step reports red even though the merge landed. The --is-ancestor early-exit path in the same file correctly uses || true. Adding it here too would match that pattern and avoid confusing operators on transient failures. Self-heals on re-run.

[P2] Cross-workflow race (sync + merge running concurrently)
The two workflows use separate concurrency groups and can overlap. If the merge workflow deletes chore/sync while the sync workflow is mid-run, the sync's --force-with-lease push fails. Self-heals on next push to main. Since the merge workflow is manual (workflow_dispatch), this is an operator-avoidable scenario.

[P2] Stale chore/sync branch on no-op sync
The push step runs before the PR step evaluates whether there's anything to propose. On a no-op (main already in experimental), chore/sync is pushed but no PR is created. Next sync treats it as "still in flight". Works correctly but wastes a reuse cycle. Moving the ancestry check before the push would avoid the stale branch.

[P2] Squash button not programmatically blocked (process concern)
Nothing prevents clicking the squash button, which breaks merge-base ancestry permanently. The PR body includes a prominent warning and recovery instructions — this is the best the code can do. The real fix is a repo-level config change (enable merge commits for experimental or add a branch ruleset). Not a defect in this diff.

---

Verdict: APPROVE ✅

No P0 or P1 issues found. The fix is well-designed and has been thoroughly iterated over four review rounds. All prior P1s (from charly-bg's review) and P2s (from my round 3 review) are resolved in d80fdb7. The P2 items above are improvements worth considering but do not block merge. CI is green, the design is sound, and the edge cases are handled correctly.

---

Reviewed by Jarvis 🤖 · Requested by Gabriel Díaz (<@U03MGHMAJL8>) via Slack

[charly-bg]

Re-checked at d80fdb7b. All three P1s are answered — the conflict path can now open the PR itself, the push and PR gates are properly split, and the squash problem has a real merge path. Two P2s left, both one-liners.

P2 — the push isn't gated on having anything to propose, so the first run creates a phantom branch. chore/sync is 404 and main...experimental is behind_by=0 right now, so both merges no-op, HEAD == origin/experimental, and L63 still pushes because REMOTE_HEAD=absent. Result: a chore/sync at experimental's tip with no PR — after which every run takes BASE=origin/chore/sync and the premise at L39-42 ("an existing chore/sync is a sync still in flight") is false from run one. Ran the two steps verbatim against a lab repo in today's shape to confirm.

if [ "$(git rev-parse HEAD)" = "$REMOTE_HEAD" ] || git merge-base --is-ancestor HEAD origin/experimental; then

P2 — the two workflows can interleave. Both mutate chore/sync but sit in different concurrency groups. If a push to main lands while the merge workflow is between its fetch and its --delete, it merges the older OID, pushes experimental, then deletes a branch carrying newer main commits — the PR head is no longer reachable, so it closes as Closed, not merged. Reverse order is milder but confirmed: --force-with-lease does not trip when the remote ref was deleted after the fetch, it just recreates the branch, undoing the cleanup. One shared group: in both files fixes it.

P3, minor: deleting the head branch also closes a PR, so if that delete is processed before GitHub's reachability check the sync PR reads Closed rather than Merged; delete_branch_on_merge doesn't cover a PR closed without merging (#473, #478), which leaves the branch as a permanent base; agreed on the || true at merge-sync-to-experimental.yml:68.

One larger point: 68 new lines, the PR-body warning and the git merge -s ours recovery all work around allow_merge_commit=false, which one repo setting removes — and the workaround is opt-in while the Squash button, the only button the repo offers, is the default and fails silently two syncs later. Re-enabling merge commits (or a ruleset on experimental) would let the whole second workflow go.

[pravusjif]

P2 — the two workflows can interleave. Both mutate chore/sync but sit in different concurrency groups. If a push to main lands while the merge workflow is between its fetch and its --delete, it merges the older OID, pushes experimental, then deletes a branch carrying newer main commits — the PR head is no longer reachable, so it closes as Closed, not merged. Reverse order is milder but confirmed: --force-with-lease does not trip when the remote ref was deleted after the fetch, it just recreates the branch, undoing the cleanup. One shared group: in both files fixes it.

Tested the scenario locally with Claude and and got the opposite:

! [rejected]  topic -> topic (stale info)
remote has topic again? no

It rejects when the run had fetched the branch. The silent-recreate you describe happens only when the sync run started with no chore/sync at all — no tracking ref, so the push is a create with no lease to violate. Both variants are real, both are bad, and both fixed at efb4ae6