Skip to content

fix(ci): repair pages workflow so the site actually deploys#53

Merged
tstapler merged 3 commits intomainfrom
stelekit-web-demo
Apr 28, 2026
Merged

fix(ci): repair pages workflow so the site actually deploys#53
tstapler merged 3 commits intomainfrom
stelekit-web-demo

Conversation

@tstapler
Copy link
Copy Markdown
Owner

Summary

  • build-demo was always skipped because github.event.head_commit.modified is empty for squash-merged PR commits — confirmed by checking the feat(editor): Cmd+K commit, which touched 6 kmp/ files but still skipped the job. Replaced with a check-changes job that uses git diff HEAD~1 HEAD (reliable for all push types).
  • deploy was always skipped due to a known GitHub Actions quirk: when any job in the transitive needs chain was skipped (build-demo), downstream jobs using the default success() check are also skipped, even if the intermediate job (build-site) ran and succeeded. Fixed with if: always() && needs.build-site.result == 'success'.
  • DEMO_AVAILABLE was always false when the artifact came from the fallback download step. Fixed to check both download outcomes. Also fixed the fallback script to exit 1 when no previous run exists (was silently exiting 0, incorrectly indicating success).

The site at stelekit.stapler.dev has status: null in the Pages API — it has never been deployed. These three bugs are the reason.

Test plan

  • Merge to main triggers pages workflow
  • check-changes detects the workflow file change (no kmp change) → kmp=false
  • build-demo skipped (no kmp change)
  • build-site runs, deploys docs-only site with DEMO_AVAILABLE=false
  • deploy runs and deploys to stelekit.stapler.dev
  • Site is accessible at stelekit.stapler.dev
  • On next commit that touches kmp/, build-demo runs and builds the WASM
  • After that, workflow_dispatch or next kmp commit makes demo available

🤖 Generated with Claude Code

Three bugs prevented stelekit.stapler.dev from ever being deployed:

1. `build-demo` always skipped — github.event.head_commit.modified is
   empty for squash-merged PR commits, so the kmp/ path filter never
   fired. Replaced with a `check-changes` job that uses git diff
   HEAD~1 HEAD, which works reliably for all push types.

2. `deploy` always skipped — GitHub Actions propagates the skipped
   state of `build-demo` transitively, causing `deploy` (which only
   depends on `build-site`) to also be skipped even though build-site
   ran and succeeded. Fixed by adding
   `if: always() && needs.build-site.result == 'success'`.

3. `DEMO_AVAILABLE` ignored the fallback download — if the artifact
   came from a previous run via the fallback step, DEMO_AVAILABLE was
   still false. Now checks both download steps. Also fixed the fallback
   script to exit 1 when no previous run exists so the outcome
   correctly reflects availability.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 28, 2026 01:45
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 28, 2026

Android Load Benchmark

Instrumented benchmark on an API 30 x86_64 emulator measuring load performance for the Android app.

Comparing 3b0b91b (this PR) vs 16a1b40 (baseline)
Device: API 30 x86_64 emulator — 25 pages

Metric This PR Baseline Delta
Phase 1 TTI ↓ 76ms 80ms -4ms (-5%) ✅
Phase 3 index ↓ 94ms 99ms -5ms (-5%) ✅
Write p95 (baseline) ↓ 4ms 2ms +2ms (+100%) ⚠️
Write p95 (during phase 3) ↓ 3ms 2ms +1ms (+50%) ⚠️
Jank factor ↓ 0.75x 1x -0.25x (-25%) ✅
Concurrent writes ↑ 1 1 0 (0%)
↓ lower is better · ↑ higher is better

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 28, 2026

JVM Load Benchmark (Desktop)

Synthetic in-memory benchmark measuring load performance for the desktop (JVM) app.
Comparing 3b0b91b (this PR) vs 16a1b40 (baseline)
Graph config: xlarge — 230 pages

Metric This PR Baseline Delta
Phase 1 TTI ↓ 9ms 9ms 0 (0%)
Phase 2 background ↓ 3ms 3ms 0 (0%)
Phase 3 index ↓ 15ms 13ms +2ms (+15%) ⚠️
Total ↓ 26ms 25ms +1ms (+4%) ⚠️
Write p95 (baseline) ↓ 31ms 31ms 0 (0%)
Write p95 (under load) ↓ n/a n/a
Jank factor ↓ n/a n/a
↓ lower is better
Flamegraphs (this PR) **Allocation** — object allocation pressure (JDBC/SQLite churn)

Alloc flamegraph not available

CPU — method-level hotspots by on-CPU time

CPU flamegraph not available

Top SQL queries by total time (this PR) | table:operation | calls | p50 | p99 | max | total | |-----------------|-------|-----|-----|-----|-------| | `pages:select` | 2 | 1ms | 1ms | 1ms | 1ms |
Top allocation hotspots (this PR) `67.9%` byte[]_[k] `4.1%` java.lang.String_[k] `2.6%` jdk.internal.org.objectweb.asm.SymbolTable$Entry[]_[k] `2.6%` java.lang.StringBuilder_[k] `2.1%` int[]_[k]
Top CPU hotspots (this PR) `99%` /usr/lib/x86_64-linux-gnu/libc.so.6 `0.1%` pread `0.1%` inflate `0.1%` app/cash/sqldelight/driver/jdbc/JdbcPreparedStatement.executeQuery_[0] `0.1%` SymbolTable::new_symbol

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Repairs the GitHub Pages deployment workflow so docs and the optional WASM demo artifact are built reliably and the deploy job runs even when upstream jobs are skipped.

Changes:

  • Adds a check-changes job to detect kmp//Gradle-related changes and gate build-demo.
  • Fixes deploy job gating so it runs when build-site succeeds, even if upstream jobs were skipped.
  • Improves demo artifact handling (fallback download behavior and DEMO_AVAILABLE computation).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .github/workflows/pages.yml Outdated
Comment on lines +26 to +35
fetch-depth: 2
- name: Detect kmp/gradle changes
id: filter
# git diff HEAD~1 HEAD is reliable for squash merges and direct commits;
# github.event.head_commit.modified is often empty for PR-merged commits.
run: |
if git diff --name-only HEAD~1 HEAD | grep -qE '^(kmp/|gradle|settings\.gradle)'; then
echo "kmp=true" >> "$GITHUB_OUTPUT"
else
echo "kmp=false" >> "$GITHUB_OUTPUT"
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

git diff --name-only HEAD~1 HEAD only compares the last commit. On a push containing multiple commits, changes to kmp/ (or build files) earlier in the push can be missed and build-demo will be skipped incorrectly. Consider diffing the full push range instead (e.g., ${{ github.event.before }}..${{ github.sha }}) and adjusting fetch-depth accordingly.

Suggested change
fetch-depth: 2
- name: Detect kmp/gradle changes
id: filter
# git diff HEAD~1 HEAD is reliable for squash merges and direct commits;
# github.event.head_commit.modified is often empty for PR-merged commits.
run: |
if git diff --name-only HEAD~1 HEAD | grep -qE '^(kmp/|gradle|settings\.gradle)'; then
echo "kmp=true" >> "$GITHUB_OUTPUT"
else
echo "kmp=false" >> "$GITHUB_OUTPUT"
fetch-depth: 0
- name: Detect kmp/gradle changes
id: filter
# Compare the full pushed range so multi-commit pushes don't miss earlier changes.
# github.event.head_commit.modified is often empty for PR-merged commits.
run: |
if [ "${{ github.event_name }}" != "push" ]; then
echo "kmp=false" >> "$GITHUB_OUTPUT"
else
BEFORE="${{ github.event.before }}"
AFTER="${{ github.sha }}"
if [ "$BEFORE" = "0000000000000000000000000000000000000000" ]; then
CHANGED_FILES="$(git ls-tree -r --name-only "$AFTER")"
else
CHANGED_FILES="$(git diff --name-only "$BEFORE" "$AFTER")"
fi
if printf '%s\n' "$CHANGED_FILES" | grep -qE '^(kmp/|gradle|settings\.gradle)'; then
echo "kmp=true" >> "$GITHUB_OUTPUT"
else
echo "kmp=false" >> "$GITHUB_OUTPUT"
fi

Copilot uses AI. Check for mistakes.
Comment thread .github/workflows/pages.yml Outdated
# git diff HEAD~1 HEAD is reliable for squash merges and direct commits;
# github.event.head_commit.modified is often empty for PR-merged commits.
run: |
if git diff --name-only HEAD~1 HEAD | grep -qE '^(kmp/|gradle|settings\.gradle)'; then
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

The change detector regex doesn't include root Gradle build scripts like build.gradle.kts (present in this repo) or buildSrc/, so edits to those files could still skip build-demo even though they affect the WASM build. Expand the match to include root build scripts (and any other build inputs you consider relevant).

Suggested change
if git diff --name-only HEAD~1 HEAD | grep -qE '^(kmp/|gradle|settings\.gradle)'; then
if git diff --name-only HEAD~1 HEAD | grep -qE '^(kmp/|buildSrc/|gradle/|gradlew(\.bat)?$|settings\.gradle(\.kts)?$|build\.gradle(\.kts)?$|gradle\.properties$)'; then

Copilot uses AI. Check for mistakes.
Comment on lines +108 to +109
# True when demo files are present from either current run or fallback
DEMO_AVAILABLE: ${{ steps.download-demo.outcome == 'success' || steps.download-demo-fallback.outcome == 'success' }}
Copy link

Copilot AI Apr 28, 2026

Choose a reason for hiding this comment

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

DEMO_AVAILABLE is being passed as an env var, but the site reads it via import.meta.env.DEMO_AVAILABLE (Astro/Vite env). By default, Vite only exposes env vars with the configured prefix (commonly PUBLIC_/VITE_), so this may remain undefined and evaluate to false even when set here. Align the variable name/prefix (or configure Vite envPrefix / switch the site to read process.env at build time) so the flag actually reaches the build.

Suggested change
# True when demo files are present from either current run or fallback
DEMO_AVAILABLE: ${{ steps.download-demo.outcome == 'success' || steps.download-demo-fallback.outcome == 'success' }}
# True when demo files are present from either current run or fallback.
# Use a public prefix so Astro/Vite exposes the value via import.meta.env.
PUBLIC_DEMO_AVAILABLE: ${{ steps.download-demo.outcome == 'success' || steps.download-demo-fallback.outcome == 'success' }}

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

No change needed here. import.meta.env.DEMO_AVAILABLE is in Astro frontmatter (the --- block in demo.astro), which executes at build time on the server, not in the browser. In Astro, all env vars are accessible in frontmatter without any prefix; PUBLIC_ is only required for variables that need to be embedded into client-side JavaScript.

tstapler and others added 2 commits April 27, 2026 19:24
Validates that the compiled Kotlin/WASM binary actually runs in a
browser, not just that the build directory is non-empty.

Three assertions in sequence:
1. canvas#ComposeTarget is attached to the DOM (HTML loaded)
2. Canvas width grows past 300 px (Compose sized the canvas, meaning
   the WASM main() ran and CanvasBasedWindow initialized)
3. Centre WebGL pixel has non-zero alpha (Compose committed a paint
   frame, meaning the UI actually rendered)

The test server (e2e/server.mjs) sets COOP/COEP headers directly, so
coi-serviceworker.min.js sees crossOriginIsolated=true on the first
navigation and skips its reload—no flaky redirect to handle.

CI: pages.yml build-demo job now installs Playwright/Chromium after
the Gradle build and runs the smoke test before uploading the artifact.
The playwright-report artifact is preserved on failure for debugging.
Changes to e2e/ also trigger build-demo (path filter updated).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…patterns

Addresses two review comments on PR #53:

1. Use github.event.before instead of HEAD~1 so multi-commit pushes
   don't miss kmp/ changes that landed in earlier commits of the same
   push. Switches to fetch-depth: 0 so the before SHA is always
   reachable. Handles the all-zeros SHA (initial push / no before)
   by defaulting to kmp=true.

2. Expand the path regex to include build.gradle.kts (root build
   script) and buildSrc/ (both present in this repo), which affect
   the WASM build but were not previously triggering build-demo.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@tstapler tstapler merged commit 782448b into main Apr 28, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants