Skip to content

[WIP, DRAFT] ci: add shared build infrastructure with S3 artifact caching #6693

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.

Sign up for GitHub

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

Closed
vyavdoshenko wants to merge 7 commits into from
Closed

[WIP, DRAFT] ci: add shared build infrastructure with S3 artifact caching #6693

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a6baa31
ci: workflow updated
vyavdoshenko Feb 20, 2026
b7ccfd0
ci: workflow updated
vyavdoshenko Feb 20, 2026
1e55c9a
ci: workflow updated
vyavdoshenko Feb 23, 2026
8013786
ci: workflow updated
vyavdoshenko Feb 23, 2026
2e39fbc
ci: workflow updated
vyavdoshenko Feb 23, 2026
4e6db44
ci: workflow updated
vyavdoshenko Feb 23, 2026
a11e6cc
Merge branch 'main' into bobik/update_ci_to_use_shared_artefacts
vyavdoshenko Feb 23, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 108 additions & 0 deletions .github/actions/ensure-build/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
name: Ensure Build
description: >
Comment on lines +1 to +2
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The PR description mentions that the S3 bucket requires a lifecycle rule with 5-day TTL on the ci-builds/ prefix, but this prerequisite is not documented in the workflow file or action. Consider adding a comment at the top of this file documenting the required S3 bucket configuration, including the lifecycle policy, so that future maintainers or anyone setting up this infrastructure in another repository knows what's required.

Copilot uses AI. Check for mistakes.
Check S3 for a pre-built artifact for the current SHA+config.
If missing, build from source and upload. If present, skip build.

inputs:
build-id:
required: true
type: string
description: "Unique ID for this build config (e.g., ubuntu20-gcc14-Debug)"
ninja-target:
required: true
type: string
description: "Ninja target: 'src/all' (full, for ctest) or 'dragonfly' (minimal, for pytest only)"
cmake-args:
required: true
type: string
description: "Full cmake args string passed through to cmake"
s3-bucket:
required: true
type: string
description: "S3 bucket name for storing build artifacts"

runs:
using: "composite"
steps:
- name: Install dependencies
shell: bash
run: |
apt-get update -qq && apt-get install -y -qq zstd > /dev/null
pip3 install -q awscli 2>/dev/null || pip3 install -q --break-system-packages awscli

- name: Check S3 for existing artifact
id: check-s3
shell: bash
run: |
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The S3 key uses ${GITHUB_SHA} at lines 37, 29 which is the commit SHA. According to the PR description, this is intentional for caching builds per commit. However, for pull requests, GITHUB_SHA is the merge commit SHA (the ephemeral merge of PR head with base), not the PR head SHA.

This means:

  1. Builds are cached per merge commit (different for each base update)
  2. Cannot reuse builds across different PRs even with same head SHA
  3. Artifacts are auto-cleaned by S3 lifecycle after 5 days (per PR description)

This design is acceptable for ephemeral PR builds but could be optimized by using the PR head SHA for better cache reuse. Document this tradeoff in the PR description or add a comment in the code explaining why merge commit SHA is used.

Suggested change
run: |
run: |
# NOTE: We intentionally use GITHUB_SHA here. For pull_request events this is the
# ephemeral merge commit SHA (PR head merged into base), so caches are per-merge
# rather than per PR head. This avoids any mismatch between the artifact and the
# exact code being tested. It does mean we cannot reuse artifacts across different
# PRs with the same head SHA or when the base branch moves, but that tradeoff is
# acceptable for these short-lived, auto-expiring PR build artifacts.

Copilot uses AI. Check for mistakes.
S3_KEY="ci-builds/${GITHUB_SHA}/${{ inputs.build-id }}/artifact.tar.zst"
echo "s3_key=${S3_KEY}" >> $GITHUB_OUTPUT

if aws s3api head-object --bucket "${{ inputs.s3-bucket }}" --key "${S3_KEY}" > /dev/null 2>&1; then
echo "found=true" >> $GITHUB_OUTPUT
echo "::notice::Artifact found for ${GITHUB_SHA}/${{ inputs.build-id }}, skipping build"
else
echo "found=false" >> $GITHUB_OUTPUT
echo "::notice::No artifact for ${GITHUB_SHA}/${{ inputs.build-id }}, building from source"
Comment on lines +37 to +45
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The build-id input is used directly in the S3 key path construction without validation. If build-id contains path traversal characters like '../', it could lead to artifacts being written to unexpected S3 locations. While the current workflow only passes hardcoded values like 'ubuntu20-gcc14-Debug', consider adding validation to ensure build-id contains only safe characters (alphanumeric, hyphens, underscores) to prevent potential misuse if this action is reused elsewhere.

Suggested change
S3_KEY="ci-builds/${GITHUB_SHA}/${{ inputs.build-id }}/artifact.tar.zst"
echo "s3_key=${S3_KEY}" >> $GITHUB_OUTPUT
if aws s3api head-object --bucket "${{ inputs.s3-bucket }}" --key "${S3_KEY}" > /dev/null 2>&1; then
echo "found=true" >> $GITHUB_OUTPUT
echo "::notice::Artifact found for ${GITHUB_SHA}/${{ inputs.build-id }}, skipping build"
else
echo "found=false" >> $GITHUB_OUTPUT
echo "::notice::No artifact for ${GITHUB_SHA}/${{ inputs.build-id }}, building from source"
BUILD_ID="${{ inputs.build-id }}"
if [[ ! "$BUILD_ID" =~ ^[A-Za-z0-9_-]+$ ]]; then
echo "::error::Invalid build-id '$BUILD_ID'. Allowed characters are: letters, digits, hyphen (-), underscore (_)."
exit 1
fi
S3_KEY="ci-builds/${GITHUB_SHA}/${BUILD_ID}/artifact.tar.zst"
echo "s3_key=${S3_KEY}" >> $GITHUB_OUTPUT
if aws s3api head-object --bucket "${{ inputs.s3-bucket }}" --key "${S3_KEY}" > /dev/null 2>&1; then
echo "found=true" >> $GITHUB_OUTPUT
echo "::notice::Artifact found for ${GITHUB_SHA}/${BUILD_ID}, skipping build"
else
echo "found=false" >> $GITHUB_OUTPUT
echo "::notice::No artifact for ${GITHUB_SHA}/${BUILD_ID}, building from source"

Copilot uses AI. Check for mistakes.
fi

- name: Configure CMake
if: steps.check-s3.outputs.found == 'false'
shell: bash
run: |
cmake -B /build ${{ inputs.cmake-args }} -L

- name: Build
if: steps.check-s3.outputs.found == 'false'
shell: bash
run: |
cd /build
ninja ${{ inputs.ninja-target }}
echo "=== Build complete ==="
df -h

- name: Package artifact
if: steps.check-s3.outputs.found == 'false'
shell: bash
run: |
cd /build

if [ "${{ inputs.ninja-target }}" = "dragonfly" ]; then
# Minimal package: just the dragonfly binary
tar --dereference -cf - dragonfly | zstd -3 -o /tmp/artifact.tar.zst
else
Comment on lines +69 to +72
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

The tar invocation is missing a -- end-of-options delimiter. If an attacker can cause a file in /build to begin with - (e.g. via a crafted target/output name), tar may interpret it as an option (including dangerous ones like checkpoint actions). Use tar ... -- dragonfly (and consider a fixed allowlist/list-file approach) to avoid option-injection.

Copilot uses AI. Check for mistakes.
# Full package: dragonfly + test binaries + CTestTestfile tree + runfiles
BINARIES="dragonfly"

# Collect test binaries (top-level *_test files)
TEST_BINS=$(find . -maxdepth 1 -name '*_test' -type f | sed 's|^\./||')
Comment on lines +76 to +77
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The packaging logic at lines 76-80 uses find . -maxdepth 1 -name '*_test' -type f to collect test binaries, but doesn't check if the files are actually executable. If CMake produces a non-executable file matching *_test (e.g., a data file or script), it would be included but not made executable later.

While the extract step at lines 43-44 in fetch-build/action.yml does chmod +x on these files, it would be more robust to filter by executable bit during collection:

TEST_BINS=$(find . -maxdepth 1 -name '*_test' -type f -executable | sed 's|^\./||')

This ensures only executable test binaries are packaged.

Suggested change
# Collect test binaries (top-level *_test files)
TEST_BINS=$(find . -maxdepth 1 -name '*_test' -type f | sed 's|^\./||')
# Collect test binaries (top-level executable *_test files)
TEST_BINS=$(find . -maxdepth 1 -name '*_test' -type f -executable | sed 's|^\./||')

Copilot uses AI. Check for mistakes.
if [ -n "$TEST_BINS" ]; then
BINARIES="$BINARIES $TEST_BINS"
fi

# Collect CTestTestfile.cmake files (needed for ctest -L DFLY)
CTEST_FILES=$(find . -name 'CTestTestfile.cmake' | sed 's|^\./||')

# Collect runfiles directories (test data symlinks resolved by --dereference)
RUNFILES=$(find . -name '*.runfiles' -type d | sed 's|^\./||')

# Build the file list
FILE_LIST=""
for f in $BINARIES $CTEST_FILES $RUNFILES; do
FILE_LIST="$FILE_LIST $f"
done

echo "=== Packaging $(echo $FILE_LIST | wc -w) items ==="
echo "$FILE_LIST" | tr ' ' '\n' | sort
tar --dereference -cf - $FILE_LIST | zstd -3 -o /tmp/artifact.tar.zst
Comment on lines +88 to +96
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

tar --dereference -cf - $FILE_LIST expands an unquoted, whitespace-delimited string and also lacks a -- end-of-options delimiter. This is both fragile (breaks if any path contains whitespace/newlines) and can allow option-injection if any packaged path starts with -. Prefer generating a file list and passing it via tar --null -T - --files-from=- (or similar) and include -- to terminate options.

Suggested change
# Build the file list
FILE_LIST=""
for f in $BINARIES $CTEST_FILES $RUNFILES; do
FILE_LIST="$FILE_LIST $f"
done
echo "=== Packaging $(echo $FILE_LIST | wc -w) items ==="
echo "$FILE_LIST" | tr ' ' '\n' | sort
tar --dereference -cf - $FILE_LIST | zstd -3 -o /tmp/artifact.tar.zst
# Build the file list and package it safely
ITEM_COUNT=0
for f in $BINARIES $CTEST_FILES $RUNFILES; do
ITEM_COUNT=$((ITEM_COUNT + 1))
done
echo "=== Packaging ${ITEM_COUNT} items ==="
for f in $BINARIES $CTEST_FILES $RUNFILES; do
echo "$f"
done | sort
# Use a NUL-delimited file list to avoid word-splitting and option-injection
{
for f in $BINARIES $CTEST_FILES $RUNFILES; do
printf '%s\0' "$f"
done
} | tar --dereference -cf - --null --files-from=- | zstd -3 -o /tmp/artifact.tar.zst

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

@augmentcode augmentcode Bot Feb 20, 2026

Choose a reason for hiding this comment

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

Using tar --dereference here can end up archiving symlink targets outside the build tree (if the build generates such links), which could unintentionally upload host/workspace files into the S3 artifact.

Severity: medium

Other Locations
  • .github/actions/ensure-build/action.yml:65

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Comment on lines +88 to +96
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The variables BINARIES, CTEST_FILES, and RUNFILES are not quoted when used in the for loop and tar command. If any filenames contain spaces or special characters, this will cause the script to fail or behave incorrectly. Wrap variable expansions in quotes to handle filenames with spaces safely.

Suggested change
# Build the file list
FILE_LIST=""
for f in $BINARIES $CTEST_FILES $RUNFILES; do
FILE_LIST="$FILE_LIST $f"
done
echo "=== Packaging $(echo $FILE_LIST | wc -w) items ==="
echo "$FILE_LIST" | tr ' ' '\n' | sort
tar --dereference -cf - $FILE_LIST | zstd -3 -o /tmp/artifact.tar.zst
# Build the file list as newline-separated entries to safely handle spaces
FILE_LIST="/tmp/artifact_file_list.txt"
: > "$FILE_LIST"
# Always include the main binary
printf '%s\n' "$BINARIES" >> "$FILE_LIST"
# Append test binaries, CTest files, and runfiles if present
if [ -n "$TEST_BINS" ]; then
printf '%s\n' "$TEST_BINS" >> "$FILE_LIST"
fi
if [ -n "$CTEST_FILES" ]; then
printf '%s\n' "$CTEST_FILES" >> "$FILE_LIST"
fi
if [ -n "$RUNFILES" ]; then
printf '%s\n' "$RUNFILES" >> "$FILE_LIST"
fi
ITEM_COUNT=$(wc -l < "$FILE_LIST")
echo "=== Packaging $ITEM_COUNT items ==="
sort "$FILE_LIST"
tar --dereference -cf - --files-from="$FILE_LIST" | zstd -3 -o /tmp/artifact.tar.zst

Copilot uses AI. Check for mistakes.
Comment on lines +63 to +96
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The artifact packaging uses zstd -3 at lines 71, 96, which is a low compression level. While this is faster than higher levels, it results in larger artifacts and longer S3 upload/download times.

For CI artifacts that are downloaded multiple times (one upload, multiple downloads by test jobs), using a higher compression level like zstd -10 could reduce total network transfer time despite slower compression. The trade-off depends on artifact size and network speed.

Current approach prioritizes build speed over network efficiency. For a DRAFT/WIP workflow, this is acceptable. Consider benchmarking with higher compression levels in production to optimize total CI time.

Copilot uses AI. Check for mistakes.
Comment on lines +89 to +96
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The tar command uses unquoted variable expansion for $FILE_LIST, which could fail if any file names contain spaces or special characters. Although CTestTestfile.cmake and .runfiles paths are unlikely to have spaces, this is a maintainability risk.

Consider using an array and proper quoting:

FILE_ARRAY=()
for f in $BINARIES $CTEST_FILES $RUNFILES; do
  FILE_ARRAY+=("$f")
done
tar --dereference -cf - "${FILE_ARRAY[@]}" | zstd -3 -o /tmp/artifact.tar.zst
Suggested change
FILE_LIST=""
for f in $BINARIES $CTEST_FILES $RUNFILES; do
FILE_LIST="$FILE_LIST $f"
done
echo "=== Packaging $(echo $FILE_LIST | wc -w) items ==="
echo "$FILE_LIST" | tr ' ' '\n' | sort
tar --dereference -cf - $FILE_LIST | zstd -3 -o /tmp/artifact.tar.zst
FILE_ARRAY=()
for f in $BINARIES $CTEST_FILES $RUNFILES; do
if [ -n "$f" ]; then
FILE_ARRAY+=("$f")
fi
done
echo "=== Packaging ${#FILE_ARRAY[@]} items ==="
printf '%s\n' "${FILE_ARRAY[@]}" | sort
tar --dereference -cf - "${FILE_ARRAY[@]}" | zstd -3 -o /tmp/artifact.tar.zst

Copilot uses AI. Check for mistakes.
fi

ls -lh /tmp/artifact.tar.zst

- name: Upload to S3
if: steps.check-s3.outputs.found == 'false'
shell: bash
run: |
export AWS_MAX_ATTEMPTS=3
aws s3 cp --no-progress /tmp/artifact.tar.zst \
"s3://${{ inputs.s3-bucket }}/${{ steps.check-s3.outputs.s3_key }}"
echo "::notice::Artifact uploaded to s3://${{ inputs.s3-bucket }}/${{ steps.check-s3.outputs.s3_key }}"
48 changes: 48 additions & 0 deletions .github/actions/fetch-build/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
name: Fetch Build
description: >
Download and extract a pre-built artifact from S3.
Expects the artifact to have been uploaded by the ensure-build action.

inputs:
build-id:
required: true
type: string
description: "Same build-id used in ensure-build (e.g., ubuntu20-gcc14-Debug)"
s3-bucket:
required: true
type: string
description: "S3 bucket name for storing build artifacts"

runs:
using: "composite"
steps:
- name: Install dependencies
shell: bash
run: |
apt-get update -qq && apt-get install -y -qq zstd > /dev/null
pip3 install -q awscli 2>/dev/null || pip3 install -q --break-system-packages awscli
Comment on lines +19 to +23
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The apt-get update and apt-get install commands at lines 22, 30 redirect output to suppress noise, but don't check for failures. If the package installation fails (e.g., network issues, repository problems), subsequent steps will fail with cryptic "command not found" errors for zstd or aws.

Consider checking exit codes:

apt-get update -qq && apt-get install -y -qq zstd > /dev/null || {
  echo "::error::Failed to install dependencies"
  exit 1
}

This is particularly important for zstd which is critical for artifact extraction.

Copilot uses AI. Check for mistakes.

- name: Download artifact from S3
shell: bash
run: |
export AWS_MAX_ATTEMPTS=3
S3_KEY="ci-builds/${GITHUB_SHA}/${{ inputs.build-id }}/artifact.tar.zst"
echo "Downloading s3://${{ inputs.s3-bucket }}/${S3_KEY}"
aws s3 cp --no-progress "s3://${{ inputs.s3-bucket }}/${S3_KEY}" /tmp/artifact.tar.zst
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

If the S3 download fails (network issue, incorrect bucket, missing artifact), the script will continue to the extraction step which will fail with a confusing error. Consider adding explicit error handling after the aws s3 cp command to check if the download succeeded and provide a clear error message if the artifact is missing or the download failed.

Suggested change
aws s3 cp --no-progress "s3://${{ inputs.s3-bucket }}/${S3_KEY}" /tmp/artifact.tar.zst
if ! aws s3 cp --no-progress "s3://${{ inputs.s3-bucket }}/${S3_KEY}" /tmp/artifact.tar.zst; then
echo "ERROR: Failed to download artifact from s3://${{ inputs.s3-bucket }}/${S3_KEY}. Check the bucket name, key, and AWS credentials." >&2
exit 1
fi
if [ ! -s /tmp/artifact.tar.zst ]; then
echo "ERROR: Artifact file /tmp/artifact.tar.zst is missing or empty after download." >&2
exit 1
fi

Copilot uses AI. Check for mistakes.
ls -lh /tmp/artifact.tar.zst
Comment on lines +25 to +32
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The S3 download step has no error handling if the artifact doesn't exist. While this might be intentional (expecting the build job to have completed successfully), it will produce a cryptic error message if the artifact is missing due to build job failure or race condition.

Consider adding explicit error handling:

if ! aws s3 cp --no-progress "s3://${{ inputs.s3-bucket }}/${S3_KEY}" /tmp/artifact.tar.zst; then
  echo "::error::Failed to download artifact from S3. Build may have failed or artifact is missing."
  exit 1
fi

Copilot uses AI. Check for mistakes.

- name: Extract artifact
shell: bash
run: |
mkdir -p /build
cd /build
zstd -d /tmp/artifact.tar.zst --stdout | tar xf -
Copy link
Copy Markdown

@augmentcode augmentcode Bot Feb 20, 2026

Choose a reason for hiding this comment

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

Extracting a tarball from S3 with tar xf - has the usual risk of path traversal/overwrites if the artifact contents are ever corrupted or attacker-controlled; do we want a guardrail to ensure extraction stays within /build?

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

rm -f /tmp/artifact.tar.zst
Comment on lines +34 to +40
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The tar extraction uses tar xf - without verification flags. If the downloaded artifact is corrupted or incomplete (network issue, S3 issue), this could silently extract a partial/corrupt build leading to confusing test failures.

Consider adding verification:

if ! zstd -d /tmp/artifact.tar.zst --stdout | tar xf - --warning=failed-read; then
  echo "::error::Failed to extract artifact. The archive may be corrupted."
  exit 1
fi

Copilot uses AI. Check for mistakes.

# Ensure binaries are executable
chmod +x /build/dragonfly 2>/dev/null || true
find /build -maxdepth 1 -name '*_test' -type f -exec chmod +x {} \;

echo "=== Artifact extracted to /build ==="
ls -lh /build/dragonfly
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

If the dragonfly binary does not exist after extraction, the chmod command will fail silently with '2>/dev/null || true', but ls -lh will then fail with a non-zero exit code and terminate the action. Consider checking for the file's existence before attempting to list it, or handle the error case explicitly.

Suggested change
ls -lh /build/dragonfly
if [ -f /build/dragonfly ]; then
ls -lh /build/dragonfly
else
echo "dragonfly binary not found at /build/dragonfly"
fi

Copilot uses AI. Check for mistakes.
echo "Test binaries: $(find /build -maxdepth 1 -name '*_test' -type f | wc -l)"
23 changes: 19 additions & 4 deletions .github/actions/regression-tests/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,10 @@ inputs:
epoll:
required: false
type: string
dragonfly-path:
required: false
type: string
description: "Absolute path to dragonfly binary. If set, overrides build-folder-name/dfly-executable."
Comment on lines +36 to +39
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The new dragonfly-path input at lines 36-39 is marked as required: false with type: string, but the implementation at lines 94-98, 111-115, 202-206 checks if [ -n "${{ inputs.dragonfly-path }}" ] which will always be true even if the input is an empty string.

In GitHub Actions, when an optional input is not provided, it evaluates to an empty string, not null. The current check works correctly, but it's worth noting that explicitly passing an empty string (dragonfly-path: "") would be treated differently than omitting the parameter.

This is not a bug in practice (users won't explicitly pass empty strings), but for robustness, consider documenting that the parameter should be omitted rather than set to empty string when not needed.

Copilot uses AI. Check for mistakes.
df-arg:
default: ""
required: false
Expand Down Expand Up @@ -87,7 +91,11 @@ runs:
echo "=== Running S3 snapshot tests with local MinIO ==="
cd ${GITHUB_WORKSPACE}/tests

export DRAGONFLY_PATH="${GITHUB_WORKSPACE}/${{inputs.build-folder-name}}/${{inputs.dfly-executable}}"
if [ -n "${{ inputs.dragonfly-path }}" ]; then
export DRAGONFLY_PATH="${{ inputs.dragonfly-path }}"
else
export DRAGONFLY_PATH="${GITHUB_WORKSPACE}/${{inputs.build-folder-name}}/${{inputs.dfly-executable}}"
fi

# MinIO binary is downloaded and started by conftest.py when MINIO_S3_ENDPOINT is set
MINIO_S3_ENDPOINT=http://localhost:9000 timeout 10m pytest -k "s3" --timeout=300 --color=yes dragonfly/snapshot_test.py --log-cli-level=INFO -v
Expand All @@ -100,7 +108,11 @@ runs:
cd ${GITHUB_WORKSPACE}/tests
echo "Current commit is ${{github.sha}}"
# used by PyTests
export DRAGONFLY_PATH="${GITHUB_WORKSPACE}/${{inputs.build-folder-name}}/${{inputs.dfly-executable}}"
if [ -n "${{ inputs.dragonfly-path }}" ]; then
export DRAGONFLY_PATH="${{ inputs.dragonfly-path }}"
else
export DRAGONFLY_PATH="${GITHUB_WORKSPACE}/${{inputs.build-folder-name}}/${{inputs.dfly-executable}}"
fi
export ROOT_DIR="${GITHUB_WORKSPACE}/tests/dragonfly/valkey_search"
export UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=1 # to crash on errors

Expand Down Expand Up @@ -186,6 +198,9 @@ runs:
if: failure() && contains(runner.labels, 'self-hosted')
shell: bash
run: |
cd ${GITHUB_WORKSPACE}/build
timestamp=$(date +%Y-%m-%d_%H:%M:%S)
mv ./dragonfly /var/crash/dragonfly_${timestamp}
if [ -n "${{ inputs.dragonfly-path }}" ]; then
mv "${{ inputs.dragonfly-path }}" /var/crash/dragonfly_${timestamp}
else
mv "${GITHUB_WORKSPACE}/${{ inputs.build-folder-name }}/dragonfly" /var/crash/dragonfly_${timestamp}
Copy link

Copilot AI Feb 20, 2026

Choose a reason for hiding this comment

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

When dragonfly-path is not provided, this fallback path hard-codes dragonfly and ignores the dfly-executable input. If callers set dfly-executable to a different binary name, this mv will fail and the debugging artifact won’t be preserved. Use ${{ inputs.dfly-executable }} in the fallback path for consistency with the action’s inputs.

Suggested change
mv "${GITHUB_WORKSPACE}/${{ inputs.build-folder-name }}/dragonfly" /var/crash/dragonfly_${timestamp}
mv "${GITHUB_WORKSPACE}/${{ inputs.build-folder-name }}/${{ inputs.dfly-executable }}" /var/crash/dragonfly_${timestamp}

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

@augmentcode augmentcode Bot Feb 20, 2026

Choose a reason for hiding this comment

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

Copy binary on a self hosted runner hard-codes /dragonfly instead of using the dfly-executable input, so it may fail to collect the crashing binary when a non-default executable name is used.

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

fi
Loading
Loading