-
Notifications
You must be signed in to change notification settings - Fork 1
Fix Xvfb startup failures, split OpenSCAD workflows, add persistent logging and animated GIF generation #48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: dorkmo <[email protected]>
Co-authored-by: dorkmo <[email protected]>
Co-authored-by: dorkmo <[email protected]>
|
@copilot lets split out the structural anlysis tests from the other github actions such as the png rendering etc |
Co-authored-by: dorkmo <[email protected]>
Done! I've split the workflows into two separate files: OpenSCAD Render and Export - Handles all PNG rendering, animations, and CNC output files Both workflows run independently on the same triggers. The structural analysis captures the ECHO output from your parametric design calculations and generates a separate report artifact. Commit: c7ff56b |
Co-authored-by: dorkmo <[email protected]>
|
@copilot perhaps we should add a log file in the mechanical design folder with descriptions of the structural analysis failures for future review and improvement. |
Co-authored-by: dorkmo <[email protected]>
Added! The structural analysis workflow now creates
The log file is automatically committed to the repository for easy tracking and future review. Commit: 379a04d |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR fixes Xvfb startup failures in GitHub Actions workflows by starting a persistent Xvfb instance instead of spawning multiple instances via xvfb-run. It also splits the monolithic workflow into two separate workflows for better organization and parallel execution.
Key Changes
- Xvfb Fix: Replaced
xvfb-runwrappers with a single persistent Xvfb instance on display:99, usingxdpyinfofor startup verification and proper graceful cleanup - Workflow Separation: Split validation and rendering into two independent workflows (
openscad-render.ymlfor visual outputs,openscad-structural-analysis.ymlfor validation) - Persistent Logging: Added automated
STRUCTURAL_ANALYSIS_LOG.mdgeneration and commit with detailed failure tracking and historical records
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
.github/workflows/openscad-render.yml |
Updated to use persistent Xvfb instance, removed validation steps (moved to structural analysis workflow), added cleanup step and cross-reference to structural analysis workflow |
.github/workflows/openscad-structural-analysis.yml |
New workflow for syntax validation and structural analysis with persistent Xvfb, generates and commits detailed analysis logs with failure tracking |
LifeTrac-v25/mechanical_design/STRUCTURAL_ANALYSIS_LOG.md |
New automatically-generated log file tracking structural analysis test results with timestamps, commit info, and failure details |
LifeTrac-v25/mechanical_design/assembly.png |
Binary PNG file generated by the render workflow (visual output artifact) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| sed -n '/COMPLETE STRUCTURAL ANALYSIS SUMMARY/,/========================================/{p;/========================================/q}' structural_analysis.log | tail -n +2 >> structural_report.md || \ | ||
| sed -n '/COMPLETE STRUCTURAL ANALYSIS SUMMARY/,$p' structural_analysis.log | tail -n +2 | head -n 20 >> structural_report.md |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sed command uses a fallback with || operator, but the logic is slightly confusing. The first sed command exits with code 0 if it finds the pattern, even if the closing delimiter ======================================== isn't found. The fallback will only execute if the first command exits non-zero (which is rare for sed).
Consider checking if the output is empty or adding explicit error handling to ensure the fallback works as intended.
| sed -n '/COMPLETE STRUCTURAL ANALYSIS SUMMARY/,/========================================/{p;/========================================/q}' structural_analysis.log | tail -n +2 >> structural_report.md || \ | |
| sed -n '/COMPLETE STRUCTURAL ANALYSIS SUMMARY/,$p' structural_analysis.log | tail -n +2 | head -n 20 >> structural_report.md | |
| summary=$(sed -n '/COMPLETE STRUCTURAL ANALYSIS SUMMARY/,/========================================/{p;/========================================/q}' structural_analysis.log | tail -n +2) | |
| if [ -z "$summary" ]; then | |
| summary=$(sed -n '/COMPLETE STRUCTURAL ANALYSIS SUMMARY/,$p' structural_analysis.log | tail -n +2 | head -n 20) | |
| fi | |
| echo "$summary" >> structural_report.md |
| - name: Commit structural analysis log | ||
| run: | | ||
| cd LifeTrac-v25/mechanical_design | ||
| git config --global user.name 'github-actions[bot]' | ||
| git config --global user.email 'github-actions[bot]@users.noreply.github.com' | ||
|
|
||
| # Add the log file | ||
| git add STRUCTURAL_ANALYSIS_LOG.md | ||
|
|
||
| # Check if there are changes | ||
| if ! git diff --staged --quiet; then | ||
| git commit -m "Update structural analysis log [skip ci]" | ||
| git push --force-with-lease origin HEAD || { | ||
| echo "INFO: git push failed. This is expected on PRs from forks." | ||
| echo "The log file is available as a workflow artifact." | ||
| } | ||
| else | ||
| echo "No changes to structural analysis log" | ||
| fi |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The workflow performs git operations (commit and push) but doesn't explicitly declare the required permissions. While GITHUB_TOKEN is provided, newer GitHub Actions security practices require explicit permission declarations.
Consider adding a permissions: block at the job or workflow level with contents: write to make the required permissions explicit and follow security best practices.
| cd LifeTrac-v25/mechanical_design | ||
|
|
||
| # Run OpenSCAD with echo output to capture structural analysis | ||
| openscad -o /dev/null --export-format echo openscad/lifetrac_v25.scad 2>&1 | tee structural_analysis.log |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The openscad command's exit code is not checked after running the structural analysis. If OpenSCAD fails, the pipeline will continue and subsequent steps may produce misleading results (e.g., "No structural analysis summary found" error instead of indicating the actual OpenSCAD failure).
Consider adding error checking similar to the validation step above, or use set -e and set -o pipefail at the beginning of the script to ensure the step fails on command errors.
| openscad -o /dev/null --export-format echo openscad/lifetrac_v25.scad 2>&1 | tee structural_analysis.log | |
| openscad -o /dev/null --export-format echo openscad/lifetrac_v25.scad 2>&1 | tee structural_analysis.log | |
| OPENSCAD_EXIT=${PIPESTATUS[0]} | |
| if [ $OPENSCAD_EXIT -ne 0 ]; then | |
| echo "✗ OpenSCAD structural analysis failed (exit code $OPENSCAD_EXIT)" | |
| exit $OPENSCAD_EXIT | |
| fi |
| # Check if there are changes | ||
| if ! git diff --staged --quiet; then | ||
| git commit -m "Update structural analysis log [skip ci]" | ||
| git push --force-with-lease origin HEAD || { |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using --force-with-lease may cause issues when committing back to the same branch that triggered the workflow. On push events, the workflow checks out the exact commit SHA, not the branch HEAD. If another commit is pushed before the workflow completes, --force-with-lease will fail.
Consider using a regular git push instead, or add logic to handle concurrent pushes more gracefully. The fallback error handler is good for fork PRs, but may also silently mask legitimate push failures on the main repository.
| git push --force-with-lease origin HEAD || { | |
| git push origin HEAD || { |
| echo "" >> STRUCTURAL_ANALYSIS_LOG.md | ||
|
|
||
| # Extract rated capacity | ||
| CAPACITY=$(grep "RATED LIFT CAPACITY:" structural_analysis.log | head -1 | sed 's/ECHO: "RATED LIFT CAPACITY:", //') |
Copilot
AI
Dec 7, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The sed command to extract the CAPACITY may fail silently if the pattern doesn't match exactly. The variable $CAPACITY could be empty or contain unexpected content if the log format varies.
Consider adding validation to check if $CAPACITY is non-empty before using it, or provide a default value like "Unknown" to avoid rendering an incomplete markdown line.
| CAPACITY=$(grep "RATED LIFT CAPACITY:" structural_analysis.log | head -1 | sed 's/ECHO: "RATED LIFT CAPACITY:", //') | |
| CAPACITY=$(grep "RATED LIFT CAPACITY:" structural_analysis.log | head -1 | sed 's/ECHO: "RATED LIFT CAPACITY:", //') | |
| if [ -z "$CAPACITY" ]; then | |
| CAPACITY="Unknown" | |
| fi |
|
@copilot would it be possible to generate an animated gif of the arms moving in the openscad animation? |
Co-authored-by: dorkmo <[email protected]>
Yes! The workflow already generates 36 animation frames and now creates an animated GIF (lifetrac_v25_animation.gif) showing the arms moving through their full range of motion. The GIF is automatically committed to the repository along with the other rendered outputs for easy access. The animation uses the parametric $t variable to animate the arm movement from ground position to fully raised. Commit: eda6ae4 |
The workflow was failing with
xvfb-run: error: Xvfb failed to startafter rendering multiple OpenSCAD files. Eachxvfb-runinvocation attempted to spawn a new Xvfb instance, eventually exhausting display resources.Changes
Xvfb Fix
:99with robust startup verification usingxdpyinfo(from x11-utils package)xvfb-run openscadcommands with directopenscadinvocations usingDISPLAY=:99Workflow Separation
openscad-render.yml- Generates visual outputs (PNG renders, animations, CNC files)openscad-structural-analysis.yml- Validates syntax and runs structural analysis tests (stress ratios, lift capacity, design metrics)Persistent Structural Analysis Logging
STRUCTURAL_ANALYSIS_LOG.mdinLifeTrac-v25/mechanical_design/[skip ci]for easy access and tracking of design improvements over timeSTRUCTURAL_ANALYSIS.mdAnimated GIF Generation
lifetrac_v25_animation.gif) showing the loader arms moving through their full range of motion (36 frames)Both workflows use the same Xvfb fix and run independently on the same triggers (push, PR, manual dispatch).
Before:
After:
Benefits
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.