Skip to content

[Tooling] Upload CDN builds as Internal first, publish as External later#2724

Open
iangmaia wants to merge 4 commits intotrunkfrom
ainfra-2102-cdn-builds-internal-visibility
Open

[Tooling] Upload CDN builds as Internal first, publish as External later#2724
iangmaia wants to merge 4 commits intotrunkfrom
ainfra-2102-cdn-builds-internal-visibility

Conversation

@iangmaia
Copy link
Contributor

@iangmaia iangmaia commented Mar 6, 2026

Related issues

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

  • Upload all CDN build artifacts with visibility: :internal (Automatticians-only) during finalize_release / distribute_release_build, instead of external
  • Embed CDN post IDs as an HTML comment (<!-- CDN_POST_IDS:{...} -->) in the draft GitHub release body so they can be retrieved later
  • In publish_release, read the draft release body, extract post IDs, and call update_apps_cdn_build_metadata to flip visibility to external before publishing the GitHub release
  • This gives the team a window for internal testing between finalize and publish

Why visibility instead of draft status?

An earlier approach (#2695) used post_status: 'draft' instead of visibility: :internal. This was abandoned because:

  • Draft posts on the CDN may not be easily downloadable/testable by the Release Manager
  • Internal visibility keeps builds accessible to Automatticians for smoke-testing
  • Storing post IDs in the GitHub release body is more deterministic than querying the CDN API for drafts (only publishes builds from the last finalization, even if finalize_release ran multiple times)

Testing Instructions

Pre-merge Checklist

  • Have you checked for TypeScript, React or other console errors?

🤖 Generated with Claude Code

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>
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?
Copy link
Contributor

@AliSoftware AliSoftware Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried a few things and added a message (hard coding the visibility code for internal) in 38e6abe.


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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 return match[1].split(',').map(&:to_i) as an Array<Integer> instead of a Hash)
  • 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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Good idea, it simplifies things a lot. Updated on 3739247.

Comment on lines +772 to +776
# 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?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on my previous suggestion comment about storing them as a simple comma-separate list instead:

Suggested change
# 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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated on 3739247.

Comment on lines +839 to +856
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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on my previous suggestion comment about storing them as a simple comma-separate list instead:

Suggested change
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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated on 3739247

Copy link
Contributor

@AliSoftware AliSoftware left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants