[Tooling] Upload CDN builds as Internal first, publish as External later#2724
[Tooling] Upload CDN builds as Internal first, publish as External later#2724
Conversation
During finalize_release, all CDN builds are now uploaded with visibility: internal. The CDN post IDs are embedded in the draft GitHub release body. In publish_release, those IDs are read back and update_apps_cdn_build_metadata flips visibility to external before the GitHub release is published. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
fastlane/Fastfile
Outdated
| release_name = "v#{version}" | ||
| cdn_post_ids = extract_cdn_post_ids_from_draft_release(release_name: release_name) | ||
|
|
||
| UI.user_error!("No CDN post IDs found in draft release #{release_name}. Cannot publish without updating CDN visibility.") if cdn_post_ids.empty? |
There was a problem hiding this comment.
I wonder if there is a (curl?) command we could run from developers' machines to ask the AppsCDN site for those post IDs if they are missing?
- The goal would be that if we hit that case where the GitHub Release doesn't have the CDN post IDs, at least the release manager could run that command from their terminal to get the list of post IDs for the given version, then edit the GitHub Release draft to copy/paste that list of post IDs into the release draft's description, then retry the
publish_releaselane from Releases V2, allowing them to recover from such a state even if a bit manually? - This would only be doable from a developer's machine (or one that is AutoProxxy'd), since the only way to have the Apps CDN would expose the list of builds with
visibility:internalin the API response is if that API call was made by an Automattician user (i.e. ifis_automatticianreturnstrue)—which is why you couldn't do that from CI
If we can find such a command, then maybe it would then be worth to include it in this UI.user_error! message as a suggestion for the developer to run it as a way to recover?
There was a problem hiding this comment.
I've tried a few things and added a message (hard coding the visibility code for internal) in 38e6abe.
fastlane/Fastfile
Outdated
|
|
||
| UI.user_error!("No CDN post IDs found in draft release #{release_name}. Cannot publish without updating CDN visibility.") if cdn_post_ids.empty? | ||
|
|
||
| post_ids = cdn_post_ids.values.map(&:to_i) |
There was a problem hiding this comment.
Given the only usage we have for those post IDs is to get the list of the IDs (i.e. we don't care about the keys of that JSON in the first place), what's the point of storing the post IDs in the GitHub Release draft as a JSON object, as opposed to a simple comma-separated list of IDs?
<!-- CDN_POST_IDS:123,456,789 -->This would:
- Make this magic comment smaller and taking less visual space (i.e. less verbose if e.g. someone wanted to edit the draft to amend the release description and got this large JSON being a bit in the way, also avoiding risk of introduce a syntax error if accidentally making changes to that comment…)
- Make it way easier to parse in
extract_cdn_post_ids_from_draft_release(no need to parse the JSON, just have that helper returnmatch[1].split(',').map(&:to_i)as anArray<Integer>instead of aHash) - Make it easier to add that comment manually in the unlikely case that the Release Managers would hit this case and would want to recover from it manually by adding the list of post IDs themselves to unblock the process
There was a problem hiding this comment.
👍 Good idea, it simplifies things a lot. Updated on 3739247.
fastlane/Fastfile
Outdated
| # Embed CDN post IDs so publish_release can update their visibility later | ||
| cdn_post_ids = builds.each_with_object({}) do |(key, build), hash| | ||
| hash[key] = build[:post_id] if build[:post_id]&.positive? | ||
| end | ||
| body += "\n<!-- CDN_POST_IDS:#{cdn_post_ids.to_json} -->" unless cdn_post_ids.empty? |
There was a problem hiding this comment.
Based on my previous suggestion comment about storing them as a simple comma-separate list instead:
| # Embed CDN post IDs so publish_release can update their visibility later | |
| cdn_post_ids = builds.each_with_object({}) do |(key, build), hash| | |
| hash[key] = build[:post_id] if build[:post_id]&.positive? | |
| end | |
| body += "\n<!-- CDN_POST_IDS:#{cdn_post_ids.to_json} -->" unless cdn_post_ids.empty? | |
| # Embed CDN post IDs so publish_release can update their visibility later | |
| cdn_post_ids = builds.values.map(&:post_id).select(&:positive?) | |
| body += "\n<!-- CDN_POST_IDS:#{cdn_post_ids.join(',')} -->" unless cdn_post_ids.empty? |
fastlane/Fastfile
Outdated
| match = body.match(/<!-- CDN_POST_IDS:(\{.*?\}) -->/) | ||
| unless match | ||
| UI.important("Draft release #{release_name} found but no CDN post IDs embedded in the body.") | ||
| return {} | ||
| end | ||
|
|
||
| begin | ||
| parsed = JSON.parse(match[1]) | ||
| rescue JSON::ParserError => e | ||
| UI.error("Failed to parse CDN post IDs from draft release #{release_name}: #{e.message}") | ||
| return {} | ||
| end | ||
|
|
||
| parsed.each do |key, value| | ||
| UI.user_error!("Invalid CDN post ID for '#{key}' in draft release #{release_name}: #{value.inspect} (expected positive integer)") unless value.is_a?(Integer) && value.positive? | ||
| end | ||
|
|
||
| parsed |
There was a problem hiding this comment.
Based on my previous suggestion comment about storing them as a simple comma-separate list instead:
| match = body.match(/<!-- CDN_POST_IDS:(\{.*?\}) -->/) | |
| unless match | |
| UI.important("Draft release #{release_name} found but no CDN post IDs embedded in the body.") | |
| return {} | |
| end | |
| begin | |
| parsed = JSON.parse(match[1]) | |
| rescue JSON::ParserError => e | |
| UI.error("Failed to parse CDN post IDs from draft release #{release_name}: #{e.message}") | |
| return {} | |
| end | |
| parsed.each do |key, value| | |
| UI.user_error!("Invalid CDN post ID for '#{key}' in draft release #{release_name}: #{value.inspect} (expected positive integer)") unless value.is_a?(Integer) && value.positive? | |
| end | |
| parsed | |
| match = body.match(/<!-- CDN_POST_IDS:([0-9,]*) -->/) | |
| unless match | |
| UI.important("Draft release #{release_name} found but no CDN post IDs embedded in the body.") | |
| return [] | |
| end | |
| match[1]).split(',').map(&:to_i) |
AliSoftware
left a comment
There was a problem hiding this comment.
Code changes in Fastlane looks good so approving to unblock
But 🎗️ reminder you'll need to first merge wordpress-mobile/release-toolkit#701 then releasing a new version then do a bundle update fastlane-plugin-wpmreleasetoolkit in this PR before it's actually ready to be merged.
Related issues
update_apps_cdn_build_metadataaction with batchpost_idssupport)How AI was used in this PR
Claude Code was used to implement the Fastfile changes based on the approach designed in #2689 and refined through PR discussion in #2695. All generated code was reviewed.
Proposed Changes
visibility: :internal(Automatticians-only) duringfinalize_release/distribute_release_build, instead ofexternal<!-- CDN_POST_IDS:{...} -->) in the draft GitHub release body so they can be retrieved laterpublish_release, read the draft release body, extract post IDs, and callupdate_apps_cdn_build_metadatato flip visibility toexternalbefore publishing the GitHub releaseWhy visibility instead of draft status?
An earlier approach (#2695) used
post_status: 'draft'instead ofvisibility: :internal. This was abandoned because:finalize_releaseran multiple times)Testing Instructions
SearchControlmargin bottom styles to ensure compatibility with the new component version #701)Pre-merge Checklist
🤖 Generated with Claude Code