Skip to content

feat: add branch-specific PR preview environments#87

Merged
sudip-mondal-2002 merged 5 commits into
sudip-mondal-2002:mainfrom
ionfwsrijan:feature/pr-preview-pages
May 16, 2026
Merged

feat: add branch-specific PR preview environments#87
sudip-mondal-2002 merged 5 commits into
sudip-mondal-2002:mainfrom
ionfwsrijan:feature/pr-preview-pages

Conversation

@ionfwsrijan

@ionfwsrijan ionfwsrijan commented May 15, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Adds branch-specific GitHub Pages preview environments for open pull requests.

This introduces a two-stage preview flow:

  • .github/workflows/preview.yml builds the Emscripten web target for PRs using read-only permissions.
  • .github/workflows/preview-pages.yml deploys the generated preview artifact to gh-pages under pr-previews/pr-<number>/, comments the live preview URL on the PR, and removes the preview directory when the PR is closed.
  • DEPLOYMENT.md documents the new preview workflow and URL format.

The workflow is split so PR code is built without repository write permissions, while the deploy workflow only publishes the generated artifact.

Related Issue

Closes #82

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • ♻️ Refactor / code cleanup
  • 📝 Documentation
  • ✅ Tests
  • 🔧 Build / CI / Configuration

How Was This Tested?

  • Platform(s) tested on: Windows local workflow syntax review
  • Test command run: YAML parsed successfully with Python
  • Additional check: git diff --cached --check
  • Manual test steps: Reviewed against existing CI Emscripten build steps and GitHub Pages deployment pattern.

Checklist

  • Code compiles and builds successfully on my platform
  • All existing tests pass (./amplitron-tests)
  • New tests added for new functionality (if applicable)
  • Documentation updated (README, code comments) if applicable
  • No blocking calls on the audio thread
  • Tested on: Windows / GitHub Actions YAML validation

Screenshots / Demo

Not applicable. This PR adds GitHub Actions preview deployment infrastructure.

Summary by CodeRabbit

  • New Features

    • Pull requests now generate automatic web previews with preview URLs posted as PR comments for easy access.
  • Documentation

    • Documented new PR preview workflow including setup and URL behavior.
    • Updated GitHub Pages site URL to https://amplitron.sudipmondal.co.in/.
  • Chores

    • Enhanced CI workflow reliability for external dependency downloads with improved error handling.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

This PR hardens CI dependency downloads, implements automated per-PR preview environments on GitHub Pages, automates cleanup on PR closure, and documents the preview workflow architecture.

Changes

PR Preview Environments and CI Infrastructure

Layer / File(s) Summary
CI Workflow Download Robustness
.github/workflows/ci.yml
External-dependency download steps across six build scenarios and iOS SDL2 build switch from curl -sL to curl -fsSL (fail fast on HTTP errors) and consolidate directory creation to mkdir -p external external/kiss_fft at block start.
Preview Deployment Workflow Structure and Deploy Job
.github/workflows/deploy-preview.yml
New workflow defines workflow_run (on CI success) and pull_request_target (on close) triggers with concurrency control. The deploy-preview job downloads the CI artifact, extracts PR metadata from context, stages preview files into pr-previews/pr-<number>/, validates required files, checks PR state via GitHub API, publishes to gh-pages, and creates/updates PR comments with preview URL and commit SHA using HTML markers.
Preview Cleanup Job and Removal Workflow
.github/workflows/deploy-preview.yml
The cleanup-preview job runs on PR close: checks out gh-pages, removes the pr-previews/pr-<PR_NUMBER> directory, commits and pushes the removal, and updates PR comments to indicate preview removal.
Preview Deployment Documentation
DEPLOYMENT.md
Documents the two-job preview workflow architecture, artifact reuse from CI, static-file deployment to GitHub Pages under PR-specific paths, PR comment updates, cleanup behavior, and the constraint that previews begin only after the workflow file reaches main. Also updates the documented site URL to https://amplitron.sudipmondal.co.in/.

🐰 Branch-specific preview paths now shine,
CI downloads fail fast by design,
Each PR gets its own live view,
Cleanup when the branch is through—
No more "clone and build" to review! 🎉

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat: add branch-specific PR preview environments' directly matches the main change—adding GitHub Actions workflows and documentation for per-PR preview environments on GitHub Pages.
Linked Issues check ✅ Passed The PR implements all coding requirements from issue #82: automatic web target builds for PRs via GitHub Actions, deployment to gh-pages under pr-previews/pr-/, PR comment posting with preview URLs, and cleanup on PR closure.
Out of Scope Changes check ✅ Passed Changes are within scope: new deploy-preview.yml workflow, CI workflow improvements to download robustness, and DEPLOYMENT.md documentation updates all directly support the PR preview feature objective.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

Review ran into problems

🔥 Problems

Git: Failed to clone repository. Please run the @coderabbitai full review command to re-trigger a full review. If the issue persists, set path_filters to include or exclude specific files.

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/preview-pages.yml:
- Around line 3-16: The workflow currently runs on workflow_run and
pull_request_target and may publish artifacts from untrusted fork PRs into the
repository Pages site using unvalidated metadata (e.g., preview_path →
destination_dir), so restrict deployments to trusted contexts or isolate
previews: update the triggers to avoid pull_request_target or require specific
trusted branches/authors (e.g., only run when workflow_run.workflow == "PR
Preview Build" and originating branch is in an allowlist), validate or sanitize
preview_path before using it to construct destination_dir (reject path-traversal
characters like ../ and enforce a whitelist or normalized basename), or change
publishing to a separate, dedicated Pages site or external domain to enforce
origin isolation.
- Around line 13-16: The workflow's explicit permissions block is missing
actions: read so GITHUB_TOKEN cannot download cross-run artifacts; update the
permissions mapping in the top-level permissions block (the section containing
keys like contents and pull-requests) to include actions: read, and also add the
same actions: read permission to any other permissions blocks used by jobs that
call actions/download-artifact (the other permissions sections referenced around
the later job blocks), ensuring the token has the required scope for cross-run
artifact downloads.

In @.github/workflows/preview.yml:
- Around line 64-75: In the "Setup External Dependencies" step, create the
external directory before launching any backgrounded fetches and make curl fail
on HTTP errors: move or add a mkdir -p external (and keep mkdir -p
external/kiss_fft) before the git clone and curl ... & lines, and replace all
occurrences of curl -sL with curl -fsSL so broken downloads fail the job; keep
the backgrounding (&) and final wait if you want parallel fetches.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 01ba34ae-9b9b-41ec-b2a3-4138abd28d0a

📥 Commits

Reviewing files that changed from the base of the PR and between f1e7b74 and 115e347.

📒 Files selected for processing (3)
  • .github/workflows/preview-pages.yml
  • .github/workflows/preview.yml
  • DEPLOYMENT.md

Comment thread .github/workflows/deploy-preview.yml
Comment thread .github/workflows/deploy-preview.yml
Comment thread .github/workflows/build-preview.yml Outdated
Comment on lines +64 to +75
- name: Setup External Dependencies
run: |
git clone --depth 1 --branch v1.90.1 https://github.com/ocornut/imgui.git external/imgui &
curl -sL -o external/nanosvg.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvg.h &
curl -sL -o external/nanosvgrast.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvgrast.h &
curl -sL -o external/dr_wav.h https://raw.githubusercontent.com/mackron/dr_libs/master/dr_wav.h &
mkdir -p external/kiss_fft
curl -sL -o external/kiss_fft/kiss_fft.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.h &
curl -sL -o external/kiss_fft/kiss_fft.c https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.c &
curl -sL -o external/kiss_fft/_kiss_fft_guts.h https://raw.githubusercontent.com/mborgerding/kissfft/master/_kiss_fft_guts.h &
curl -sL -o external/kiss_fft/kiss_fft_log.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft_log.h &
wait

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

# First, find and examine the preview workflow file
find . -name "preview.yml" -type f

Repository: sudip-mondal-2002/Amplitron

Length of output: 103


🏁 Script executed:

# Read the relevant section of the workflow file
cat -n .github/workflows/preview.yml | sed -n '60,80p'

Repository: sudip-mondal-2002/Amplitron

Length of output: 1609


🌐 Web query:

curl -sL vs -fsSL flags difference error handling HTTP

💡 Result:

The primary difference between the curl flag combinations -sL and -fsSL lies in how they handle HTTP errors and error reporting [1][2]. -sL (Silent, Location) - -s (--silent): Disables the progress meter and error messages [3][4]. - -L (--location): Tells curl to follow HTTP 3xx redirects [5][6]. - Error Handling: Because -s is used without -S, curl will suppress error messages if the request fails (e.g., DNS issues, connection refused, or SSL errors) [1]. This can make debugging difficult in scripts, as the command may fail silently without providing feedback [1]. -fsSL (Fail, Silent, Show-error, Location) - -f (--fail): Tells curl to fail silently (no output) on server errors (HTTP 4xx or 5xx) [7][6]. Without this flag, curl treats HTTP error responses as successful transfers and will output the error page content [2]. - -s (--silent): Disables the progress meter [3][4]. - -S (--show-error): When used with -s, this ensures that if curl fails, it will still output the error message to stderr [1][3]. - -L (--location): Follows HTTP 3xx redirects [5][6]. - Error Handling: This combination is generally preferred for scripts [1]. It ensures that HTTP server errors cause the command to exit with a non-zero status (error 22) [7][2], while still providing diagnostic error messages if a network or connection failure occurs [1]. Summary of Key Flags - -f (--fail): Prevents curl from outputting the body of an HTTP error response and forces a non-zero exit code [7][2]. - -s (--silent): Suppresses progress bars and other output [3][4]. - -S (--show-error): Ensures error messages are printed even when -s is active [1][3]. - -L (--location): Enables automatic following of redirects [5][6]. For robust scripting, -fsSL is recommended over -sL because it provides better error visibility and forces the command to fail explicitly on HTTP errors [1][2].

Citations:


Create external/ before starting background operations and use -fsSL for robust curl error handling.

Lines 66-69 are backgrounded before the external/ directory is created on line 70, so this step has a race condition that can fail intermittently with "No such file or directory" on git clone and curl operations. Additionally, curl -sL silently outputs HTTP error pages (404/500) as file content instead of failing the build. Switch to -fsSL to fail explicitly on upstream errors.

Suggested fix
       - name: Setup External Dependencies
         run: |
+          mkdir -p external external/kiss_fft
           git clone --depth 1 --branch v1.90.1 https://github.com/ocornut/imgui.git external/imgui &
-          curl -sL -o external/nanosvg.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvg.h &
-          curl -sL -o external/nanosvgrast.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvgrast.h &
-          curl -sL -o external/dr_wav.h https://raw.githubusercontent.com/mackron/dr_libs/master/dr_wav.h &
-          mkdir -p external/kiss_fft
-          curl -sL -o external/kiss_fft/kiss_fft.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.h &
-          curl -sL -o external/kiss_fft/kiss_fft.c https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.c &
-          curl -sL -o external/kiss_fft/_kiss_fft_guts.h https://raw.githubusercontent.com/mborgerding/kissfft/master/_kiss_fft_guts.h &
-          curl -sL -o external/kiss_fft/kiss_fft_log.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft_log.h &
+          curl -fsSL -o external/nanosvg.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvg.h &
+          curl -fsSL -o external/nanosvgrast.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvgrast.h &
+          curl -fsSL -o external/dr_wav.h https://raw.githubusercontent.com/mackron/dr_libs/master/dr_wav.h &
+          curl -fsSL -o external/kiss_fft/kiss_fft.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.h &
+          curl -fsSL -o external/kiss_fft/kiss_fft.c https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.c &
+          curl -fsSL -o external/kiss_fft/_kiss_fft_guts.h https://raw.githubusercontent.com/mborgerding/kissfft/master/_kiss_fft_guts.h &
+          curl -fsSL -o external/kiss_fft/kiss_fft_log.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft_log.h &
           wait
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- name: Setup External Dependencies
run: |
git clone --depth 1 --branch v1.90.1 https://github.com/ocornut/imgui.git external/imgui &
curl -sL -o external/nanosvg.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvg.h &
curl -sL -o external/nanosvgrast.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvgrast.h &
curl -sL -o external/dr_wav.h https://raw.githubusercontent.com/mackron/dr_libs/master/dr_wav.h &
mkdir -p external/kiss_fft
curl -sL -o external/kiss_fft/kiss_fft.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.h &
curl -sL -o external/kiss_fft/kiss_fft.c https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.c &
curl -sL -o external/kiss_fft/_kiss_fft_guts.h https://raw.githubusercontent.com/mborgerding/kissfft/master/_kiss_fft_guts.h &
curl -sL -o external/kiss_fft/kiss_fft_log.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft_log.h &
wait
- name: Setup External Dependencies
run: |
mkdir -p external external/kiss_fft
git clone --depth 1 --branch v1.90.1 https://github.com/ocornut/imgui.git external/imgui &
curl -fsSL -o external/nanosvg.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvg.h &
curl -fsSL -o external/nanosvgrast.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvgrast.h &
curl -fsSL -o external/dr_wav.h https://raw.githubusercontent.com/mackron/dr_libs/master/dr_wav.h &
curl -fsSL -o external/kiss_fft/kiss_fft.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.h &
curl -fsSL -o external/kiss_fft/kiss_fft.c https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.c &
curl -fsSL -o external/kiss_fft/_kiss_fft_guts.h https://raw.githubusercontent.com/mborgerding/kissfft/master/_kiss_fft_guts.h &
curl -fsSL -o external/kiss_fft/kiss_fft_log.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft_log.h &
wait
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/preview.yml around lines 64 - 75, In the "Setup External
Dependencies" step, create the external directory before launching any
backgrounded fetches and make curl fail on HTTP errors: move or add a mkdir -p
external (and keep mkdir -p external/kiss_fft) before the git clone and curl ...
& lines, and replace all occurrences of curl -sL with curl -fsSL so broken
downloads fail the job; keep the backgrounding (&) and final wait if you want
parallel fetches.

@sudip-mondal-2002 sudip-mondal-2002 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please use file names like
build-preview.yml/ deploy preview.yml

Similar with workflow names

Otherwise it's hard to understand

Comment thread DEPLOYMENT.md
Comment on lines +54 to +58

```text
https://sudip-mondal-2002.github.io/Amplitron/pr-previews/pr-<number>/
```

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Please use https://amplitron.sudipmondal.co.in/

@sudip-mondal-2002 sudip-mondal-2002 added ci High Impact Significant user-facing improvement gssoc:approved GSSoC 2026 contribution level:advanced Advanced task · 45 GSSoC points type:feature New feature or request type:devops and removed High Impact Significant user-facing improvement labels May 15, 2026
@sudip-mondal-2002

Copy link
Copy Markdown
Owner

@ionfwsrijan

Copy link
Copy Markdown
Contributor Author

@sudip-mondal-2002 I've implemented all the changes you asked for. Kindly review and merge.

@sudip-mondal-2002 sudip-mondal-2002 left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Can we re use the build from CI workflow?

And the page is not working, please test your changes before flagging for review

http://amplitron.sudipmondal.co.in/pr-previews/pr-87/

image

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/workflows/deploy-preview.yml (1)

210-218: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Create a cleanup comment when no prior marker comment exists.

Line 210 only handles update-on-existing-comment. If the marker comment is missing, the workflow posts nothing on PR close. Add a createComment fallback.

Suggested fix
             if (previous) {
               await github.rest.issues.updateComment({
                 owner,
                 repo,
                 comment_id: previous.id,
                 body,
               });
+            } else {
+              await github.rest.issues.createComment({
+                owner,
+                repo,
+                issue_number,
+                body,
+              });
             }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/deploy-preview.yml around lines 210 - 218, The current
block only calls github.rest.issues.updateComment when a previous marker comment
exists, so add an else branch to call github.rest.issues.createComment with the
same owner, repo and body when previous is falsy; use previous?.id to detect
existence and ensure you pass the correct parameters (owner, repo, body) to
createComment so a cleanup marker is created when no prior comment exists.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In @.github/workflows/ci.yml:
- Around line 42-51: Replace each bare use of wait after launching background
download processes with code that captures each background PID (e.g., pid=$! or
pids+=("$!")) and then loops over those PIDs to run wait "$pid" and check its
exit status; update the CI workflow steps that start background downloads (the
blocks using backgrounded curl/wget/download commands followed by wait) to
collect PIDs and fail the step if any individual wait returns non-zero so a
single failed download causes the job to fail.

---

Outside diff comments:
In @.github/workflows/deploy-preview.yml:
- Around line 210-218: The current block only calls
github.rest.issues.updateComment when a previous marker comment exists, so add
an else branch to call github.rest.issues.createComment with the same owner,
repo and body when previous is falsy; use previous?.id to detect existence and
ensure you pass the correct parameters (owner, repo, body) to createComment so a
cleanup marker is created when no prior comment exists.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4a58b8f3-28fa-432e-9c36-15df3cafcb80

📥 Commits

Reviewing files that changed from the base of the PR and between 115e347 and 42f72aa.

📒 Files selected for processing (3)
  • .github/workflows/ci.yml
  • .github/workflows/deploy-preview.yml
  • DEPLOYMENT.md
🚧 Files skipped from review as they are similar to previous changes (1)
  • DEPLOYMENT.md

Comment thread .github/workflows/ci.yml
Comment on lines +42 to 51
mkdir -p external external/kiss_fft
git clone --depth 1 --branch v1.90.1 https://github.com/ocornut/imgui.git external/imgui &
curl -sL -o external/nanosvg.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvg.h &
curl -sL -o external/nanosvgrast.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvgrast.h &
curl -sL -o external/dr_wav.h https://raw.githubusercontent.com/mackron/dr_libs/master/dr_wav.h &
mkdir -p external/kiss_fft
curl -sL -o external/kiss_fft/kiss_fft.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.h &
curl -sL -o external/kiss_fft/kiss_fft.c https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.c &
curl -sL -o external/kiss_fft/_kiss_fft_guts.h https://raw.githubusercontent.com/mborgerding/kissfft/master/_kiss_fft_guts.h &
curl -sL -o external/kiss_fft/kiss_fft_log.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft_log.h &
curl -fsSL -o external/nanosvg.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvg.h &
curl -fsSL -o external/nanosvgrast.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvgrast.h &
curl -fsSL -o external/dr_wav.h https://raw.githubusercontent.com/mackron/dr_libs/master/dr_wav.h &
curl -fsSL -o external/kiss_fft/kiss_fft.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.h &
curl -fsSL -o external/kiss_fft/kiss_fft.c https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.c &
curl -fsSL -o external/kiss_fft/_kiss_fft_guts.h https://raw.githubusercontent.com/mborgerding/kissfft/master/_kiss_fft_guts.h &
curl -fsSL -o external/kiss_fft/kiss_fft_log.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft_log.h &
wait

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "1) Confirm workflow uses background jobs followed by bare wait:"
rg -n -C2 'curl -fsSL.*&|git clone .*&|^\s*wait\s*$' .github/workflows/ci.yml

echo
echo "2) Demonstrate why bare wait is risky with parallel jobs:"
(
  set +e
  (sleep 0.1; false) &
  (sleep 0.2; true) &
  wait
  rc=$?
  echo "bare wait exit code: $rc (can be 0 even when one job failed)"
)

echo
echo "3) Robust pattern check: waiting each PID surfaces any failure."
(
  set +e
  pids=()
  (sleep 0.1; false) & pids+=($!)
  (sleep 0.2; true) & pids+=($!)
  overall=0
  for pid in "${pids[@]}"; do
    wait "$pid" || overall=$?
  done
  echo "per-pid wait overall exit code: $overall (non-zero when any job fails)"
)

Repository: sudip-mondal-2002/Amplitron

Length of output: 7092


Fix bare wait to properly fail when any background download job fails

In Lines 42-51, 89-98, 235-245, 346-355, 427-437, and 493-503, background downloads are followed by bare wait, which can return 0 even when one job fails. This allows failed downloads to go undetected. Capture PIDs and wait on each one individually:

Suggested fix
-        git clone --depth 1 --branch v1.90.1 https://github.com/ocornut/imgui.git external/imgui &
-        curl -fsSL -o external/nanosvg.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvg.h &
-        curl -fsSL -o external/nanosvgrast.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvgrast.h &
-        curl -fsSL -o external/dr_wav.h https://raw.githubusercontent.com/mackron/dr_libs/master/dr_wav.h &
-        curl -fsSL -o external/kiss_fft/kiss_fft.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.h &
-        curl -fsSL -o external/kiss_fft/kiss_fft.c https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.c &
-        curl -fsSL -o external/kiss_fft/_kiss_fft_guts.h https://raw.githubusercontent.com/mborgerding/kissfft/master/_kiss_fft_guts.h &
-        curl -fsSL -o external/kiss_fft/kiss_fft_log.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft_log.h &
-        wait
+        pids=()
+        git clone --depth 1 --branch v1.90.1 https://github.com/ocornut/imgui.git external/imgui & pids+=($!)
+        curl -fsSL -o external/nanosvg.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvg.h & pids+=($!)
+        curl -fsSL -o external/nanosvgrast.h https://raw.githubusercontent.com/memononen/nanosvg/master/src/nanosvgrast.h & pids+=($!)
+        curl -fsSL -o external/dr_wav.h https://raw.githubusercontent.com/mackron/dr_libs/master/dr_wav.h & pids+=($!)
+        curl -fsSL -o external/kiss_fft/kiss_fft.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.h & pids+=($!)
+        curl -fsSL -o external/kiss_fft/kiss_fft.c https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft.c & pids+=($!)
+        curl -fsSL -o external/kiss_fft/_kiss_fft_guts.h https://raw.githubusercontent.com/mborgerding/kissfft/master/_kiss_fft_guts.h & pids+=($!)
+        curl -fsSL -o external/kiss_fft/kiss_fft_log.h https://raw.githubusercontent.com/mborgerding/kissfft/master/kiss_fft_log.h & pids+=($!)
+        for pid in "${pids[@]}"; do
+          wait "$pid"
+        done
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In @.github/workflows/ci.yml around lines 42 - 51, Replace each bare use of wait
after launching background download processes with code that captures each
background PID (e.g., pid=$! or pids+=("$!")) and then loops over those PIDs to
run wait "$pid" and check its exit status; update the CI workflow steps that
start background downloads (the blocks using backgrounded curl/wget/download
commands followed by wait) to collect PIDs and fail the step if any individual
wait returns non-zero so a single failed download causes the job to fail.

@ionfwsrijan

Copy link
Copy Markdown
Contributor Author

@sudip-mondal-2002 I pushed another follow-up commit addressing the review notes.

What was wrong with the 404: the CI web-build artifact did complete successfully for this PR, but the preview deploy workflow itself is introduced by this PR. GitHub only runs workflow_run workflows when that workflow file already exists on the default branch, so this PR cannot publish its own preview URL from the newly added deploy workflow. Once the workflow is merged into main, future PRs will be able to reuse the CI web-build artifact and publish to https://amplitron.sudipmondal.co.in/pr-previews/pr-<number>/.

I also hardened the CI dependency setup CodeRabbit pointed out:

  • create external/ and external/kiss_fft/ before starting background fetches
  • use curl -fsSL so failed upstream downloads fail the job instead of saving an error page
  • documented the workflow_run default-branch limitation in DEPLOYMENT.md

Validated locally with YAML parsing and git diff --check.

@sudip-mondal-2002 sudip-mondal-2002 merged commit 88e0ff6 into sudip-mondal-2002:main May 16, 2026
8 checks passed
@sudip-mondal-2002 sudip-mondal-2002 added type:performance Performance optimization or latency improvement type:testing Unit tests, integration tests, or test coverage type:accessibility type:design type:refactor labels May 19, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci gssoc:approved GSSoC 2026 contribution level:advanced Advanced task · 45 GSSoC points mentor:sudip-mondal-2002 type:accessibility type:design type:devops type:feature New feature or request type:performance Performance optimization or latency improvement type:refactor type:testing Unit tests, integration tests, or test coverage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Branch-specific Preview Environments via GitHub Pages for Each PR

2 participants