Skip to content

@W-23331697 | Preserve target-only manifest fields on auto-promote #136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jbisaSF wants to merge 1 commit into from
Closed

@W-23331697 | Preserve target-only manifest fields on auto-promote #136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 59 additions & 18 deletions .github/scripts/promotion-utils.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,23 @@

# Helpers for forward-integration (auto-promotion) of app artifacts across
# release branches. Promotion is forward-only along the chain
# release/<oldest> -> ... -> release/<newest> -> main
# release/<oldest> -> ... -> release/<newest>
# The newest release branch is the end of the chain; auto-promote never opens
# a PR into main.
# Each function is pure (reads args / files, writes stdout) so it can be unit
# tested by test-promotion-utils.sh without a live checkout.

# Prints the next branch in the forward-integration chain for a given ref, or
# nothing when the ref is the end of the chain (main) or not a release branch.
# nothing when the ref is the end of the chain or not a release branch.
#
# next_release_branch <current_ref> <newline_separated_branch_list>
#
# Rules:
# - "main" -> chain terminates, prints nothing.
# - "release/X.Y" -> the smallest release/X'.Y' strictly greater than the
# current version; if none exists, "main".
# current version; if none exists, prints nothing
# (newest release is the end of the chain — never hops
# to main).
# - anything else -> prints nothing (feature branches are not promoted).
#
# Version comparison is numeric on (major, minor); no dependency on `sort -V`
Expand Down Expand Up @@ -50,8 +54,6 @@ next_release_branch() {

if [[ -n "$best" ]]; then
echo "$best"
else
echo "main"
fi
}

Expand Down Expand Up @@ -109,14 +111,50 @@ merge_catalog_json() {
' "$catalog_path"
}

# INIT catalog.json template used when promoting a brand-new app onto a target
# that has no catalog.json yet. Version history is then populated by the
# target branch's own update-catalog-pr job (CONTRIBUTING.md contract).
INIT_CATALOG_JSON='{
"latest": {
"version": "INIT",
"tag": "INIT"
},
"versions": []
}'

# Seeds catalog.json with the INIT template iff the file does not already
# exist. Prints "seeded" when a new file is written, or "skipped" when an
# existing catalog.json is left untouched. Version bumps of pre-existing apps
# must not rewrite catalog.json in the auto-promo PR — that file is owned by
# the target branch's catalog CI job.
#
# seed_init_catalog_if_absent <catalog_path>
seed_init_catalog_if_absent() {
local catalog_path="${1:?catalog path is required}"
if [[ -f "$catalog_path" ]]; then
echo "skipped"
return 0
fi
mkdir -p "$(dirname "$catalog_path")"
printf '%s\n' "$INIT_CATALOG_JSON" > "$catalog_path"
echo "seeded"
}

# Upserts a manifest entry into a category array of the target branch's
# manifest and prints the merged JSON. The manifest holds exactly one entry per
# app, pinned to its latest version, keyed by `.id` (e.g. loqate carries nine
# catalog versions but a single manifest entry). So the upsert:
# - matches the existing entry by `.id` (app identity), not by zip filename;
# - is monotonic on version: it replaces the entry only when the incoming
# version is >= the existing pinned version, so promoting an OLDER artifact
# forward (a back-port hop) never regresses the target's pinned version;
# - is monotonic on version: it overlays the incoming entry onto the existing
# one only when the incoming version is >= the existing pinned version, so
# promoting an OLDER artifact forward (a back-port hop) never regresses the
# target's pinned version;
# - overlays rather than replaces: keys present only on the target (e.g.
# workspace-carousel fields added on a newer release branch) are preserved
# even when the source entry at the same or newer version lacks them.
# Overlapping keys still take the incoming value, so version/zip/sha256
# advance and an intentional source-side metadata edit still promotes;
# - updates in place (no remove-and-append), so category order is stable;
# - appends when the app is not yet present on the target;
# - creates the category array when the target manifest lacks it.
# A release ranks above a pre-release of the same major.minor.patch.
Expand Down Expand Up @@ -146,15 +184,15 @@ merge_manifest_entry() {
.[$cat] = ($arr + [$entry])
elif ($existing == $entry) then
# Byte-for-byte identical already -> true no-op. Without this branch,
# the "replace" branch below would still fire (equal version compares
# >= 0) and re-append the entry at the end of the array, reordering
# every other promoted entry for zero effect and producing a
# spurious diff on an otherwise fully-idempotent re-promotion.
# jq would still rewrite the whole file (indent/key order) for zero
# effect on an otherwise fully-idempotent re-promotion.
.
elif (semver_cmp($entry.version; ($existing.version // "0.0.0")) >= 0) then
# Incoming version is newer, or equal but with changed metadata ->
# replace the pinned entry.
.[$cat] = ((($arr | map(select((.id? // "") != $id)))) + [$entry])
# Incoming version is newer, or equal with changed/additional fields.
# Overlay source keys onto the existing entry IN PLACE so target-only
# keys (isFeatured, badge, featured*, companyName, …) are never dropped
# just because the older branch lacks them.
.[$cat] = ($arr | map(if (.id? // "") == $id then . + $entry else . end))
else
# Target already pins a newer version -> leave it untouched (monotonic).
.
Expand All @@ -163,12 +201,15 @@ merge_manifest_entry() {
}

# Merges every entry across every category of a source manifest.json into a
# target manifest.json, applying the same per-id monotonic upsert as
# target manifest.json, applying the same per-id monotonic overlay as
# merge_manifest_entry (above) to each entry individually. This is what lets a
# manual, hand-edited manifest.json change (not tied to any ZIP push) promote
# forward: the edited entry is MERGED into the target - respecting the
# monotonic, id-keyed guard - rather than the whole file being overwritten,
# so it can never regress an entry the target already pins to a newer version.
# monotonic, id-keyed guard - rather than the whole file being overwritten.
# Equal-or-newer source entries overlay onto the target (source wins on
# overlapping keys; target-only keys are preserved), so a newer-branch field
# such as isFeatured cannot be wiped by an older-branch promote that simply
# lacks the key. An older source version never regresses the target's pin.
#
# A single malformed entry (e.g. a non-semver version string) must not abort
# the whole merge under the caller's `set -e` - that would silently discard
Expand Down
58 changes: 51 additions & 7 deletions .github/scripts/test-promotion-utils.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ echo "--- next_release_branch ---"

assert_eq "26.8 -> 26.9" "release/26.9" next_release_branch "release/26.8" "$BRANCHES"
assert_eq "26.9 -> 27.0" "release/27.0" next_release_branch "release/26.9" "$BRANCHES"
assert_eq "highest release -> main" "main" next_release_branch "release/27.0" "$BRANCHES"
assert_eq "highest release terminates" "" next_release_branch "release/27.0" "$BRANCHES"
assert_eq "refs/heads/ prefix accepted" "release/26.9" next_release_branch "refs/heads/release/26.8" "$BRANCHES"
assert_eq "major rollover picks minimal" "release/27.0" next_release_branch "release/26.9" \
"$(printf '%s\n' refs/heads/release/27.5 refs/heads/release/27.0 refs/heads/release/28.0)"
Expand Down Expand Up @@ -183,6 +183,29 @@ assert_json_eq "empty catalog seeds first entry" \

echo ""

# ---------------------------------------------------------------------------
# seed_init_catalog_if_absent (auto-promo: INIT only for brand-new apps)
# ---------------------------------------------------------------------------
echo "--- seed_init_catalog_if_absent ---"

# Brand-new app: no catalog.json on the target -> write INIT, do not invent a version.
new_cat="$TMPDIR_ROOT/new-app/catalog.json"
seeded_result="$(seed_init_catalog_if_absent "$new_cat")"
assert_eq "absent catalog is seeded" "seeded" printf '%s' "$seeded_result"
assert_json_eq "seeded catalog is INIT template" \
'{"latest":{"version":"INIT","tag":"INIT"},"versions":[]}' \
cat "$new_cat"

# Version bump: catalog.json already exists -> leave it untouched (no merge, no INIT overwrite).
existing_cat="$(mkfile '{"latest":{"version":"1.0.0","tag":"app-v1.0.0"},"versions":[{"version":"1.0.0","tag":"app-v1.0.0"}]}')"
skipped_result="$(seed_init_catalog_if_absent "$existing_cat")"
assert_eq "existing catalog is skipped" "skipped" printf '%s' "$skipped_result"
assert_json_eq "existing catalog content is untouched" \
'{"latest":{"version":"1.0.0","tag":"app-v1.0.0"},"versions":[{"version":"1.0.0","tag":"app-v1.0.0"}]}' \
cat "$existing_cat"

echo ""

# ---------------------------------------------------------------------------
# merge_manifest_entry
# ---------------------------------------------------------------------------
Expand All @@ -194,7 +217,7 @@ assert_json_eq "appends new app to category" \
'{"shipping":[{"id":"a","zip":"a-v1.0.0.zip","version":"1.0.0"},{"id":"b","zip":"b-v1.0.0.zip","version":"1.0.0"}]}' \
merge_manifest_entry "$m1" '{"id":"b","zip":"b-v1.0.0.zip","version":"1.0.0"}' "shipping"

# Same app id, newer version -> replace the single pinned entry (no dup, zip advances).
# Same app id, newer version -> overlay onto the single pinned entry (no dup, zip advances).
m2="$(mkfile '{"shipping":[{"id":"a","zip":"a-v1.0.0.zip","version":"1.0.0"}]}')"
assert_json_eq "replaces pinned entry with newer version" \
'{"shipping":[{"id":"a","zip":"a-v1.1.0.zip","version":"1.1.0"}]}' \
Expand All @@ -206,6 +229,20 @@ assert_json_eq "idempotent upsert at equal version" \
'{"shipping":[{"id":"a","zip":"a-v1.0.0.zip","version":"1.0.0","sha256":"new"}]}' \
merge_manifest_entry "$m2b" '{"id":"a","zip":"a-v1.0.0.zip","version":"1.0.0","sha256":"new"}' "shipping"

# Same app id, same version, target has extra keys the source lacks (e.g. 26.9
# carousel fields, 26.8 entry without them) -> extras are preserved, not wiped.
m2d="$(mkfile '{"shipping":[{"id":"a","zip":"a-v1.0.0.zip","version":"1.0.0","sha256":"old","isFeatured":true,"badge":"popular"}]}')"
assert_json_eq "equal-version overlay preserves target-only keys" \
'{"shipping":[{"id":"a","zip":"a-v1.0.0.zip","version":"1.0.0","sha256":"new","isFeatured":true,"badge":"popular"}]}' \
merge_manifest_entry "$m2d" '{"id":"a","zip":"a-v1.0.0.zip","version":"1.0.0","sha256":"new"}' "shipping"

# Same app id, NEWER version, target has extra keys -> version/zip advance and
# extras are still preserved. Also stays in its original array position.
m2e="$(mkfile '{"shipping":[{"id":"a","zip":"a-v1.0.0.zip","version":"1.0.0","isFeatured":true},{"id":"b","zip":"b-v1.0.0.zip","version":"1.0.0"}]}')"
assert_json_eq "newer-version overlay preserves target-only keys and position" \
'{"shipping":[{"id":"a","zip":"a-v1.1.0.zip","version":"1.1.0","isFeatured":true},{"id":"b","zip":"b-v1.0.0.zip","version":"1.0.0"}]}' \
merge_manifest_entry "$m2e" '{"id":"a","zip":"a-v1.1.0.zip","version":"1.1.0"}' "shipping"

# Same app id, OLDER version (back-port hop) -> target's newer pin is untouched.
m2c="$(mkfile '{"shipping":[{"id":"a","zip":"a-v2.0.0.zip","version":"2.0.0"}]}')"
assert_json_eq "older version does not regress pinned entry" \
Expand All @@ -227,10 +264,9 @@ m4_indent="$(merge_manifest_entry "$m4" '{"id":"b","zip":"b-v1.0.0.zip","version
| grep -m1 '"analytics"' | sed -E 's/[^ ].*//' | tr -d '\n' | wc -c | tr -d ' ')"
assert_eq "upsert emits 4-space indentation" "4" printf '%s' "$m4_indent"

# Byte-identical re-upsert must be a true no-op: it must NOT move the entry to
# the end of its category array (which would happen if the equal-version
# "replace" branch fired), or every other entry's promotion would be reordered
# for zero effect on a fully-idempotent re-promotion.
# Byte-identical re-upsert must be a true no-op: it must NOT rewrite or move
# the entry (which would reorder every other entry for zero effect on a
# fully-idempotent re-promotion).
m5="$(mkfile '{"shipping":[{"id":"a","zip":"a-v1.0.0.zip","version":"1.0.0"},{"id":"b","zip":"b-v1.0.0.zip","version":"1.0.0"}]}')"
assert_json_eq "byte-identical re-upsert does not reorder" \
'{"shipping":[{"id":"a","zip":"a-v1.0.0.zip","version":"1.0.0"},{"id":"b","zip":"b-v1.0.0.zip","version":"1.0.0"}]}' \
Expand Down Expand Up @@ -299,7 +335,7 @@ echo ""
# ---------------------------------------------------------------------------
echo "--- merge_manifest_file ---"

# Source has a newer entry for an app the target already pins -> replaces it,
# Source has a newer entry for an app the target already pins -> overlays it,
# same monotonic guard as merge_manifest_entry, applied across every category.
mf1_target="$(mkfile '{"shipping":[{"id":"a","zip":"a-v1.0.0.zip","version":"1.0.0"}]}')"
mf1_source="$(mkfile '{"shipping":[{"id":"a","zip":"a-v1.1.0.zip","version":"1.1.0"}]}')"
Expand Down Expand Up @@ -328,6 +364,14 @@ assert_json_eq "idempotent re-merge (no dup)" \
'{"shipping":[{"id":"a","zip":"a-v1.1.0.zip","version":"1.1.0"}]}' \
merge_manifest_file "$mf4_target" "$mf4_source"

# Whole-file promote of an equal-version source that lacks target-only keys
# must not wipe them (the #113 26.8→26.9 carousel-field regression).
mf4b_target="$(mkfile '{"tax":[{"id":"avalara-tax","zip":"avalara-tax-v1.0.0.zip","version":"1.0.0","isFeatured":true,"badge":"popular","featuredLearnMoreUrl":"https://example.com"}]}')"
mf4b_source="$(mkfile '{"tax":[{"id":"avalara-tax","zip":"avalara-tax-v1.0.0.zip","version":"1.0.0"}]}')"
assert_json_eq "whole-file equal-version merge preserves target-only keys" \
'{"tax":[{"id":"avalara-tax","zip":"avalara-tax-v1.0.0.zip","version":"1.0.0","isFeatured":true,"badge":"popular","featuredLearnMoreUrl":"https://example.com"}]}' \
merge_manifest_file "$mf4b_target" "$mf4b_source"

# A scalar top-level field (e.g. defaultLocale) has no per-entry version to
# compare, so it must still carry forward verbatim rather than being silently
# dropped by a merge that only knows how to walk category arrays.
Expand Down
40 changes: 21 additions & 19 deletions .github/workflows/update-catalog.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,16 +207,20 @@ jobs:
#
# When a ZIP lands on a release branch, open a self-contained promotion PR
# into the next branch in the chain
# release/<older> -> release/<newer> -> main
# carrying the ZIP, its manifest entry, the merged catalog.json, and icons.
# release/<older> -> release/<newer>
# carrying the ZIP, its manifest entry, an INIT catalog.json only for
# brand-new apps, and icons. Pre-existing apps' catalog.json is left for the
# target branch's update-catalog-pr job.
#
# Because the PR carries a ZIP, merging it re-fires THIS workflow on the next
# branch, which advances the chain one hop further. The chain is therefore
# self-propagating with one gated (human/App-reviewed) merge per hop and no
# cross-workflow wiring. The `update-catalog-pr` job on the next branch sees
# no diff (catalog merge is idempotent, tag is immutable) and is a no-op.
# the new ZIP and writes version history into catalog.json (and is a no-op
# when no ZIP changed).
#
# Runs only on release branches; `main` is the end of the chain.
# Runs only on release branches. The newest release branch is the end of the
# chain; auto-promote never opens a PR into main.
# ---------------------------------------------------------------------------
promote-forward:
runs-on: ubuntu-latest
Expand Down Expand Up @@ -299,7 +303,7 @@ jobs:

# 2) Determine the next hop from the live remote branch list. Fail loud
# if the remote returned nothing: an empty list would make
# next_release_branch fall through to "main" and skip release hops.
# next_release_branch print nothing and skip every hop.
branch_list="$(git ls-remote --heads origin | awk '{print $2}')"
if [[ -z "$branch_list" ]]; then
echo "::error::git ls-remote returned no branches; cannot determine next hop"
Expand Down Expand Up @@ -391,22 +395,19 @@ jobs:
mv "$tmp" "$manifest_path"
git add "$manifest_path"

# 5c) catalog.json next to the ZIP (union versions, monotonic latest).
# A brand-new app on the target has no catalog.json yet - seed
# it with the INIT template and skip merge_catalog_json so the
# target branch's own post-merge update-catalog run is what
# first populates version history (the CONTRIBUTING.md
# contract for new apps), instead of the promotion writing a
# concrete version here and duplicating that work.
# 5c) catalog.json next to the ZIP. Brand-new apps (no catalog.json
# on the target) get the INIT template so the target branch's
# post-merge update-catalog-pr job can populate version history
# (CONTRIBUTING.md contract). Version bumps of apps that already
# have catalog.json are left untouched — that file must not
# appear in the auto-promo PR.
catalog_path="$(dirname "$zip_path")/catalog.json"
if [[ -f "$catalog_path" ]]; then
tmp="$(mktemp)"
merge_catalog_json "$catalog_path" "$version" "$tag_name" > "$tmp"
mv "$tmp" "$catalog_path"
catalog_result="$(seed_init_catalog_if_absent "$catalog_path")"
if [[ "$catalog_result" == "seeded" ]]; then
git add "$catalog_path"
else
printf '{\n "latest": {\n "version": "INIT",\n "tag": "INIT"\n },\n "versions": []\n}\n' > "$catalog_path"
echo "Existing catalog.json at $catalog_path; leaving it for the target branch catalog CI job."
fi
git add "$catalog_path"

# 5d) Icons: copy from the ZIP into the shared icons dir.
isv_name="$(echo "$zip_path" | cut -d'/' -f2)"
Expand Down Expand Up @@ -516,7 +517,8 @@ jobs:

- Any app ZIP(s)
- The matching `commerce-apps-manifest/manifest.json` entry/entries (monotonic upsert keyed by app id)
- The merged `catalog.json` (union `versions`, monotonic `latest`)
- An INIT `catalog.json` only when promoting a brand-new app (existing
apps' catalogs are left to the target branch's catalog CI job)
- Any app icons under `commerce-apps-manifest/icons/`
- Any `commerce-apps-manifest/translations/*.json` changes (additive per-key merge, keyed by app id)
- Any other non-CAP file changes (docs, skills, etc. - excluding `.github/workflows/**`, which
Expand Down
Loading