From f9ef06d9c2edab42834559f51e991420c12c4fc5 Mon Sep 17 00:00:00 2001 From: Dmytro Sydorov Date: Tue, 21 Jul 2026 21:28:43 +0200 Subject: [PATCH] fix(oss-commit-sync): align-tree must align the full tree including excluded paths rehearsal in the experiments org caught this: with the assertion made exclude-aware, the migration run (align-tree=true, seeds, excluded producer workflow as the only drift) became a green no-op. it deleted nothing, appended no alignment commit, and seeded no Monorepo-Commit trailer, so every subsequent push-triggered export would fail asking for seeds. align-tree now branches on strict tree inequality: it is the explicit operator escape hatch and aligns everything, while the failure path and the guard stay exclude-aware. regression-tested with the exact migration fixture. --- .github/actions/oss-commit-sync/README.md | 9 +++-- .github/actions/oss-commit-sync/export.sh | 15 +++++--- .../actions/oss-commit-sync/test/export.bats | 37 +++++++++++++++++++ 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/.github/actions/oss-commit-sync/README.md b/.github/actions/oss-commit-sync/README.md index 629bcb1..76d4ab3 100644 --- a/.github/actions/oss-commit-sync/README.md +++ b/.github/actions/oss-commit-sync/README.md @@ -38,10 +38,11 @@ Safety mechanisms, in order: applied (snapshot projection would rewrite the whole tree). 3. **Convergence assertion** — after replay, the OSS tip tree must equal the monorepo staging tree (ignoring `exclude-paths`, which are never - mirrored), or the run fails without pushing. `align-tree: true` - instead appends one bot-authored snapshot commit that sets the OSS tree to - the staging tree: the append-only escape hatch (used at migration to drop - OSS-only producer workflows, or after manual reconciliation). + mirrored), or the run fails without pushing. `align-tree: true` instead + appends one bot-authored snapshot commit that sets the OSS tree to the + staging tree — on ANY difference, excluded paths included: it is the + explicit operator escape hatch, and at migration it is what deletes the + OSS-only producer workflows and seeds the first trailer. New release lines: when the branch does not exist on OSS, it is created from the OSS commit corresponding to the monorepo branch point (found via trailers diff --git a/.github/actions/oss-commit-sync/export.sh b/.github/actions/oss-commit-sync/export.sh index c6d8e43..1f4adb8 100755 --- a/.github/actions/oss-commit-sync/export.sh +++ b/.github/actions/oss-commit-sync/export.sh @@ -228,12 +228,15 @@ NEW_TIP="$(git -C "$WT" rev-parse HEAD)" # --- convergence assertion --------------------------------------------------- -# Excluded paths are ignored: they are never mirrored, so an external commit -# touching only them may legitimately leave the OSS tree differing there. +# The assertion ignores excluded paths: they are never mirrored, so an +# external commit touching only them may legitimately leave the OSS tree +# differing there. ALIGN_TREE=true instead aligns on ANY difference, +# including excluded paths: it is the explicit operator escape hatch, and at +# migration this is what deletes the OSS-only producer workflows and seeds +# the first Monorepo-Commit trailer. STAGING_TREE="$(git rev-parse "HEAD:${SUBTREE_PREFIX}")" OSS_TREE="$(git -C "$WT" rev-parse "HEAD^{tree}")" -if [ "$STAGING_TREE" != "$OSS_TREE" ] \ - && ! git diff --quiet "$OSS_TREE" "$STAGING_TREE" -- . ${excludes[@]+"${excludes[@]}"}; then +if [ "$STAGING_TREE" != "$OSS_TREE" ]; then if [ "$ALIGN_TREE" = "true" ]; then msgfile="$(mktemp)" { @@ -248,11 +251,13 @@ if [ "$STAGING_TREE" != "$OSS_TREE" ] \ rm -f "$msgfile" count=$((count + 1)) echo "Appended alignment commit ${NEW_TIP}" - else + elif ! git diff --quiet "$OSS_TREE" "$STAGING_TREE" -- . ${excludes[@]+"${excludes[@]}"}; then echo "::error::OSS tree does not match the monorepo staging tree after replay:" git --no-pager diff --stat "$OSS_TREE" "$STAGING_TREE" -- . ${excludes[@]+"${excludes[@]}"} || true echo "::error::Re-run with align-tree=true to append a snapshot alignment commit." exit 1 + else + echo "OSS tree differs from staging only in excluded paths; leaving them as-is" fi fi diff --git a/.github/actions/oss-commit-sync/test/export.bats b/.github/actions/oss-commit-sync/test/export.bats index de16991..732a4fe 100644 --- a/.github/actions/oss-commit-sync/test/export.bats +++ b/.github/actions/oss-commit-sync/test/export.bats @@ -263,3 +263,40 @@ Monorepo-Commit: $M0" [ "$status" -ne 0 ] [ "$(output_value diverged)" = "true" ] } + +@test "migration: align-tree removes excluded-path leftovers and seeds the trailer" { + # Pre-migration OSS carries a producer workflow that staging does not have, + # the exclude list covers it, and OSS has no trailers yet. Reproduces the + # rehearsal finding: with an exclude-aware align, the migration run was a + # green no-op that seeded nothing. + rm -rf "$OSS_REMOTE" "$ROOT/ossseed" + git init -q --bare "$OSS_REMOTE" + git init -q "$ROOT/ossseed" + ( + cd "$ROOT/ossseed" + git checkout -q -b main + mkdir -p pkg .github/workflows + printf 'l1\nl2\nl3\n' > pkg/app.go + echo "producer" > .github/workflows/release.yaml + git add . && git commit -qm "pre-migration oss" + git push -q "$OSS_REMOTE" main + ) + seed_oss=$(oss_tip) + + SEED_MONOREPO_COMMIT="$M0" SEED_OSS_COMMIT="$seed_oss" \ + EXCLUDE_PATHS=".github/workflows/release.yaml" ALIGN_TREE=true run bash "$EXPORT" + [ "$status" -eq 0 ] + [ "$(output_value pushed)" = "true" ] + # the alignment commit deleted the excluded leftover and seeded the trailer + run oss_file .github/workflows/release.yaml + [ "$status" -ne 0 ] + [ -n "$(git -C "$OSS_REMOTE" log -1 --format='%(trailers:key=Monorepo-Commit,valueonly)' main)" ] + [ "$(git -C "$OSS_REMOTE" rev-parse 'main^{tree}')" = "$(git -C "$MONO" rev-parse "HEAD:$PFX")" ] + + # and the seeded trailer makes the next run self-sufficient (no seeds) + company_commit pkg/app.go "post-migration" "feat: first post-migration change" >/dev/null + EXCLUDE_PATHS=".github/workflows/release.yaml" run bash "$EXPORT" + [ "$status" -eq 0 ] + [ "$(output_value exported-count)" = "1" ] + [ "$(oss_file pkg/app.go)" = "post-migration" ] +}