feat: add support for extra artifacts in release process - #85
Conversation
📝 WalkthroughWalkthroughThis PR extends the existing build-makefile-dylib workflow and composite action by introducing an ChangesExtra Artifacts Parameter
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
📄 README may need an updateThis PR introduces changes that might not be reflected in Reason: The README does not document the new public
|
📚 Skills documentation may need an updateThis PR introduces changes that might not be reflected in the skills documentation. Reason:
|
There was a problem hiding this comment.
Pull request overview
This PR extends the reusable “Makefile Dylib Release” GitHub workflow and its underlying composite action to optionally bundle additional build outputs into the uploaded artifact that gets attached to the GitHub release.
Changes:
- Added a new
extra-artifactsworkflow input to allow passing a newline-separated list of additional files to include in the release artifact. - Updated the
build-makefile-dylibcomposite action to copy the provided extra artifact paths intoartifacts/alongside the built.dylib.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| .github/workflows/makefile-dylib-release.yml | Adds the extra-artifacts input and forwards it into the build composite action. |
| .github/blocks/build-makefile-dylib/action.yml | Copies additional user-specified files into the artifacts/ directory before upload. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/blocks/build-makefile-dylib/action.yml (1)
56-59: ⚡ Quick winConsider validating that paths remain within the repository.
While the GitHub Actions workspace is isolated, it's a good defensive practice to validate that extra artifact paths are relative and don't escape the repository using
..sequences. This prevents confusion and potential issues if the script is adapted for other uses.🔒 Example path validation
if [ ! -f "$extra" ]; then echo "::error::extra artifact '$extra' not found — build failure" exit 1 fi + # Ensure path doesn't escape repository + resolved=$(realpath --relative-to=. "$extra" 2>/dev/null || echo "$extra") + if [[ "$resolved" == ../* ]] || [[ "$resolved" == /* ]]; then + echo "::error::extra artifact '$extra' escapes repository root" + exit 1 + fi + echo "Including extra artifact: $extra"🤖 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/blocks/build-makefile-dylib/action.yml around lines 56 - 59, Validate the artifact path in the existing if-block that checks "$extra" to ensure it doesn't escape the repo: reject absolute paths (leading '/'), any '..' segments, and canonicalize the path (using realpath or readlink -f) and verify the resolved path begins with "$GITHUB_WORKSPACE" before accepting; update the error branch for "$extra" to perform these checks and fail with a clear message if the path is invalid or outside the workspace.
🤖 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/blocks/build-makefile-dylib/action.yml:
- Around line 61-63: The current copy step cp "$extra" artifacts/ flattens files
and allows silent overwrites; update the script around the echo/file/cp lines to
either (A) detect duplicate basenames before copying by computing
base="$(basename "$extra")" and failing with a clear error if artifacts/"$base"
already exists, or (B) preserve directory structure when copying by creating
target dirs and using a path-preserving copy (e.g., mkdir -p
"artifacts/$(dirname "$extra")" and cp "$extra" "artifacts/$(dirname
"$extra")/") so files from different source dirs don’t collide; implement one
strategy and ensure the error path logs the conflicting basename and source
paths for diagnosis.
---
Nitpick comments:
In @.github/blocks/build-makefile-dylib/action.yml:
- Around line 56-59: Validate the artifact path in the existing if-block that
checks "$extra" to ensure it doesn't escape the repo: reject absolute paths
(leading '/'), any '..' segments, and canonicalize the path (using realpath or
readlink -f) and verify the resolved path begins with "$GITHUB_WORKSPACE" before
accepting; update the error branch for "$extra" to perform these checks and fail
with a clear message if the path is invalid or outside the workspace.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 53a0dc7e-04c9-4578-a6b1-c2306991ec06
📒 Files selected for processing (2)
.github/blocks/build-makefile-dylib/action.yml.github/workflows/makefile-dylib-release.yml
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: check-skills / check-skills
- GitHub Check: check-readme / check-readme
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2026-04-05T00:07:12.194Z
Learnt from: qwerzl
Repo: photon-hq/buildspace PR: 62
File: .github/workflows/update-docs.yaml:164-167
Timestamp: 2026-04-05T00:07:12.194Z
Learning: In this repo (photon-hq/buildspace), reusable workflow blocks referenced under photon-hq/buildspace (e.g., `photon-hq/buildspace/.github/blocks/<block>main`) are intentionally pinned to `main` because the blocks and workflows ship together and are kept in sync. During review, do not flag these references as version-pinning/supply-chain issues just because they use `main`; only require pinning to a release tag if there’s a separate reason (e.g., referencing an external repo or a non-controlled branch).
Applied to files:
.github/workflows/makefile-dylib-release.yml
📚 Learning: 2026-04-27T01:30:22.893Z
Learnt from: yanxue06
Repo: photon-hq/buildspace PR: 73
File: .github/workflows/check-readme.yaml:18-18
Timestamp: 2026-04-27T01:30:22.893Z
Learning: When reviewing this repo’s GitHub Actions workflows, treat Blacksmith runner labels like `blacksmith-4vcpu-ubuntu-2404` and other `blacksmith-*vcpu-ubuntu-*` values as valid/intentional third-party runner labels (Blacksmith: blacksmith.sh). Do not flag them as unknown or non-standard runner labels—these are an intentional drop-in replacement for GitHub-hosted runners.
Applied to files:
.github/workflows/makefile-dylib-release.yml
📚 Learning: 2026-04-27T01:30:22.893Z
Learnt from: yanxue06
Repo: photon-hq/buildspace PR: 73
File: .github/workflows/check-readme.yaml:18-18
Timestamp: 2026-04-27T01:30:22.893Z
Learning: In photon-hq/buildspace GitHub workflow YAML files, runner labels that match `blacksmith-*vcpu-ubuntu-*` (e.g., `blacksmith-4vcpu-ubuntu-2404`) are intentionally managed by the Blacksmith CI service (blacksmith.sh) and serve as drop-in replacements for GitHub-hosted ephemeral microVM runners. Do not flag these labels as unknown/non-standard, supply-chain concerns, or “unrecognized runner” issues; any linter/actionlint warnings about these specific labels are expected false positives.
Applied to files:
.github/workflows/makefile-dylib-release.yml
🔇 Additional comments (5)
.github/blocks/build-makefile-dylib/action.yml (3)
12-15: LGTM!
39-40: LGTM!
56-56:extra-artifactstreats directories as missing (files-only)
if [ ! -f "$extra" ]; thenrejects directories (-fis false for a directory), andcp "$extra" artifacts/(without-r) is consistent with copying only regular files. If the action is intended to support directory paths, update the check to something like-eand copy withcp -r; otherwise the current behavior is correct..github/workflows/makefile-dylib-release.yml (2)
14-18: LGTM!
68-68: LGTM!
This commit removes the `extra-artifacts` input and related logic from the build-makefile-dylib action and the makefile-dylib-release workflow. The changes streamline the release process by eliminating unnecessary complexity associated with handling additional build outputs.
Need help on this PR? Tag
@codesmithwith what you need. Autofix is disabled.Summary by CodeRabbit