Skip to content

ci: add GitHub Actions workflow for GitHub Pages deployment#6314

Closed
Anexus5919 wants to merge 1 commit intosugarlabs:masterfrom
Anexus5919:ci/github-pages-deployment
Closed

ci: add GitHub Actions workflow for GitHub Pages deployment#6314
Anexus5919 wants to merge 1 commit intosugarlabs:masterfrom
Anexus5919:ci/github-pages-deployment

Conversation

@Anexus5919
Copy link
Copy Markdown
Contributor

@Anexus5919 Anexus5919 commented Mar 19, 2026

Summary

Adds a CI/CD pipeline that automatically deploys Music Blocks to GitHub Pages on every push to master, with Jest test gating to prevent broken deployments.

Closes #4284

PR Category

  • Feature
  • Bug Fix
  • Performance
  • Tests
  • Documentation

Approach

Uses the modern GitHub Actions Pages deployment (actions/deploy-pages) instead of the legacy gh-pages branch approach. This is cleaner, avoids branch management complexity, and provides proper deployment environments with history and rollback in the GitHub UI.

Workflow structure

Job Purpose
test Runs Jest tests — deployment is blocked if tests fail
deploy Uploads the repository as a static artifact and deploys to GitHub Pages

Key design decisions

  • Triggers: push to master + workflow_dispatch (manual) — PRs do not trigger deployment
  • Fork guard: Deploy job only runs on sugarlabs/musicblocks, preventing forks from wasting runner minutes
  • No build step: The project serves static files directly, so no npm ci is needed in the deploy job — keeps the artifact lean
  • Concurrency control: Only one deployment runs at a time; queued runs wait rather than cancelling in-progress deploys
  • Consistent tooling: Uses Node 22, actions/checkout@v4, npm ci — matching all existing workflows

One-time admin prerequisite

After merging, a repo admin must change the Pages source:
Settings → Pages → Build and deployment → Source → "GitHub Actions"

This is a one-time, 10-second change. The workflow includes a comment documenting this requirement.

Prior work

This issue has had three previous PRs, all closed by the stale bot due to inactivity:

This PR incorporates the lessons from all three: correct branch (master), proper token handling (GITHUB_TOKEN via permissions), test gating, and a modern deployment approach that avoids the gh-pages branch issues entirely.

Test plan

Screenshots:
image
image

@github-actions
Copy link
Copy Markdown
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@github-actions github-actions bot added feature Adds new functionality size/M Medium: 50-249 lines changed area/ci-cd Changes to CI/CD workflows labels Mar 19, 2026
@Anexus5919
Copy link
Copy Markdown
Contributor Author

Hi @walterbender @pikurasa @FirePheonix , this PR adds a CI/CD pipeline for automated GitHub Pages deployment, addressing #4284.

I've reviewed the previous attempts (#4285, #4290, #4472) and their discussions, and this PR takes a different approach using the modern actions/deploy-pages workflow instead of the legacy gh-pages branch method. This avoids the token issues and branch management complexity that affected the earlier PRs.

What it does:

Runs Jest tests on every push to master, deployment is blocked if tests fail
Deploys to GitHub Pages using official GitHub Actions (no custom PATs, no gh-pages branch)
Supports manual deployment via workflow_dispatch
Fork-safe - deploy job only runs on sugarlabs/musicblocks
One prerequisite after merge: A repo admin needs to change the Pages source to "GitHub Actions" under Settings -> Pages (one-time, 10-second change).

I've tested this on my fork and all jobs passed successfully. Would appreciate a review when you get a chance!

@mahesh-09-12
Copy link
Copy Markdown
Contributor

@Anexus5919 nice improvement to modernize the Pages deployment workflow, using actions/deploy-pages with test gating is a solid approach. one question around deployment strategy: this switches from a more controlled or manual deploy to automatic deployment on every push to master. given the repo is actively developed, do maintainers prefer this level of automation, or would a manual trigger (or restricted deploy step) be safer to avoid unintended production updates? Just checking alignment with the current deployment policy.

@Anexus5919
Copy link
Copy Markdown
Contributor Author

Thanks for the review @mahesh-09-12! Good question.

The original issue (#4284) specifically requests automatic deployment on push to master, that's the intended behavior. Looking at PR #4290's discussion, @walterbender asked whether the workflow would auto-update sugarlabs.github.io/musicblocks, and the contributor confirmed that was the intent - there was no objection raised to the automatic approach.

That said, the workflow already has safety layers to prevent unintended deployments:

  1. Test gating - the deploy job only runs if all Jest tests pass
  2. Fork guard - if: github.repository == 'sugarlabs/musicblocks' prevents forks from triggering deployments
  3. Concurrency control - only one deployment runs at a time, queued runs wait
  4. workflow_dispatch - manual trigger is also supported if maintainers ever want to deploy on-demand

Since only maintainers can merge to master, every deployment is implicitly approved through the merge review process. But if the maintainers prefer a more restrictive approach (e.g., release-tag-only deploys), I'm happy to adjust the trigger strategy.

@mahesh-09-12
Copy link
Copy Markdown
Contributor

@Anexus5919 thanks for the detailed context, that helps. Looking back at #4290, it seems there were some challenges around deployment behavior and configuration, and maintainers were still evaluating the approach at that time. given that history, it might be worth confirming that the current setup avoids those earlier pitfalls (especially around permissions and deployment consistency), otherwise, the overall approach here looks solid.

@Anexus5919
Copy link
Copy Markdown
Contributor Author

Good point @mahesh-09-12. Here's how this PR specifically avoids the pitfalls from the earlier attempts:

Permissions (the #4285/#4290 PAT issue):
The previous PRs used secrets.GH_TOKEN, a custom Personal Access Token that only existed in the contributor's fork, so it failed on the upstream repo. This PR uses zero custom secrets. It relies entirely on the built-in GITHUB_TOKEN via the permissions block (pages: write, id-token: write), which is automatically available in every GitHub Actions run. No admin configuration of secrets is needed.

Deployment consistency (the #4290 branch/scope issues):
Targets master (the actual default branch), not main
Uses actions/upload-pages-artifact + actions/deploy-pages, the official GitHub-recommended approach - instead of manually pushing to a gh-pages branch
The only post-merge admin step is a one-time Pages source toggle (Settings → Pages → Source → "GitHub Actions")
Both of these were intentional design decisions based on reviewing the earlier PR discussions.

That said, the workflow is also flexible, if maintainers prefer manual-only deployments, the push trigger can simply be removed, leaving just workflow_dispatch for on-demand deploys.
Happy to adjust based on maintainer feedback.

@Anexus5919
Copy link
Copy Markdown
Contributor Author

Anexus5919 commented Mar 21, 2026

@walterbender @pikurasa @omsuneri Just wanted to flag an alternative deployment strategy for your consideration.

Currently the workflow deploys automatically on every push to master, but only after all Jest tests pass, so no broken code can reach production. This aligns with what #4284 requests, and since only maintainers can merge to master, every deployment is implicitly approved through the merge review process.

However, if you'd prefer more explicit control, I can switch the trigger to release-tag-only deploys meaning the site only updates when a tag like v1.0.0 is created:

on:
  push:
    tags:
      - 'v*'
  workflow_dispatch:  # manual trigger still available

This way, PRs merge to master as usual without triggering a deploy, and you'd deploy only when ready by creating a release tag. It gives you the ability to batch changes and deploy intentionally.

That said, given Music Blocks serves static files and Jest tests already gate the deploy, auto-deploy on push to master should be safe for this project.
But happy to adjust if you prefer the tag-based approach or if you have any other suggestions - do let me know!

@zealot-zew
Copy link
Copy Markdown
Contributor

we have a GSOC project on this same feature.

@Anexus5919
Copy link
Copy Markdown
Contributor Author

we have a GSOC project on this same feature.

Oh yeah! This PR addresses the same idea that is now listed under GSoC in Sugar Labs. I had opened it way before but forgot to close it. Since I’ve already submitted a GSoC proposal for this idea, I’ll close this PR.

@Anexus5919 Anexus5919 closed this Apr 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ci-cd Changes to CI/CD workflows feature Adds new functionality size/M Medium: 50-249 lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CI/CD pipeline for Deployment

3 participants