-
Notifications
You must be signed in to change notification settings - Fork 94
build(action): action.yml now compiles from source if not release #2053
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,12 +43,11 @@ inputs: | |
| runs: | ||
| using: composite | ||
| steps: | ||
| - name: Install fullsend CLI | ||
| - name: Detect install method | ||
| id: detect | ||
| shell: bash | ||
| env: | ||
| VERSION: ${{ inputs.version }} | ||
| RUNNER_OS: ${{ runner.os }} | ||
| RUNNER_ARCH: ${{ runner.arch }} | ||
| GH_TOKEN: ${{ inputs.github_token }} | ||
| run: | | ||
| set -euo pipefail | ||
|
|
@@ -64,10 +63,10 @@ runs: | |
| return 0 | ||
| fi | ||
| if (( attempt >= max_attempts )); then | ||
| echo "::error::Download failed after ${max_attempts} attempts" | ||
| echo "::error::API request failed after ${max_attempts} attempts" | ||
| return 1 | ||
| fi | ||
| echo "::warning::Download attempt ${attempt}/${max_attempts} failed, retrying in ${delay}s..." | ||
| echo "::warning::API attempt ${attempt}/${max_attempts} failed, retrying in ${delay}s..." | ||
| sleep "${delay}" | ||
| (( attempt++ )) | ||
| (( delay *= 3 )) | ||
|
|
@@ -85,29 +84,16 @@ runs: | |
| VENDORED="${GITHUB_WORKSPACE}/bin/fullsend" | ||
| fi | ||
| if [[ -n "${VENDORED}" ]]; then | ||
| echo "Using vendored fullsend binary from ${VENDORED#"${GITHUB_WORKSPACE}/"}" | ||
| mkdir -p "${RUNNER_TEMP}/fullsend" | ||
| cp "${VENDORED}" "${RUNNER_TEMP}/fullsend/fullsend" | ||
| chmod +x "${RUNNER_TEMP}/fullsend/fullsend" | ||
| echo "${RUNNER_TEMP}/fullsend" >> "${GITHUB_PATH}" | ||
| echo "Using vendored binary: ${VENDORED#"${GITHUB_WORKSPACE}/"}" | ||
| echo "install-method=vendored" >> "${GITHUB_OUTPUT}" | ||
| echo "vendored-path=${VENDORED}" >> "${GITHUB_OUTPUT}" | ||
| exit 0 | ||
| fi | ||
|
|
||
| VERSION="$(printf '%s' "${VERSION:-latest}" | tr -d '[:space:]')" | ||
| VERSION="${VERSION:-latest}" | ||
|
|
||
| os="$(echo "${RUNNER_OS}" | tr '[:upper:]' '[:lower:]')" | ||
| case "${os}" in | ||
| macos) os=darwin ;; | ||
| esac | ||
|
|
||
| arch="$(echo "${RUNNER_ARCH}" | tr '[:upper:]' '[:lower:]')" | ||
| case "${arch}" in | ||
| x64) arch=amd64 ;; | ||
| x86) arch=386 ;; | ||
| arm64|aarch64) arch=arm64 ;; | ||
| esac | ||
|
|
||
| # Resolve 'latest' to the actual tag before checking the release API. | ||
| if [[ "${VERSION}" == "latest" ]]; then | ||
| TAG="$(retry_curl -fsSL \ | ||
| -H "Accept: application/vnd.github+json" \ | ||
|
|
@@ -117,34 +103,145 @@ runs: | |
| echo "::error::Could not resolve latest release tag" | ||
| exit 1 | ||
| fi | ||
| if [[ "${TAG}" == v* ]]; then | ||
| VERSION_URL="${TAG}" | ||
| VERSION_ASSET="${TAG#v}" | ||
| else | ||
| VERSION_URL="v${TAG}" | ||
| VERSION_ASSET="${TAG}" | ||
| fi | ||
| BASE_URL="https://github.com/fullsend-ai/fullsend/releases/download/${VERSION_URL}" | ||
| VERSION="${TAG}" | ||
| fi | ||
|
|
||
| if [[ "${VERSION}" == v* ]]; then | ||
| VERSION_URL="${VERSION}" | ||
| VERSION_ASSET="${VERSION#v}" | ||
| else | ||
| if [[ "${VERSION}" == v* ]]; then | ||
| VERSION_URL="${VERSION}" | ||
| VERSION_ASSET="${VERSION#v}" | ||
| else | ||
| VERSION_URL="v${VERSION}" | ||
| VERSION_ASSET="${VERSION}" | ||
| fi | ||
| BASE_URL="https://github.com/fullsend-ai/fullsend/releases/download/${VERSION_URL}" | ||
| VERSION_URL="v${VERSION}" | ||
| VERSION_ASSET="${VERSION}" | ||
| fi | ||
|
|
||
| HTTP_STATUS="$(retry_curl -sL -o /dev/null -w "%{http_code}" \ | ||
| -H "Accept: application/vnd.github+json" \ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] error handling gap The release existence check treats ANY non-200 HTTP status (including 403 rate-limit, 500 server error, or network timeout returning 000) as 'release not found' and falls through to a source build instead of surfacing the real error. Suggested fix: Distinguish between 404 (genuinely no release) and other HTTP status codes. For 403/429/5xx/000, either retry or fail with a clear error message. |
||
| -H "Authorization: Bearer ${GH_TOKEN}" \ | ||
| "https://api.github.com/repos/fullsend-ai/fullsend/releases/tags/${VERSION_URL}")" | ||
|
|
||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] error handling gap The release-existence check uses retry_curl -sL -o /dev/null -w "%{http_code}" without -f. Because curl returns exit code 0 for any completed HTTP response when -f is absent, retry_curl will never retry this call regardless of the HTTP status. If the GitHub API returns a transient error (429, 503), retry_curl does not retry, and the step falls into the else branch which exits with an error. Suggested fix: Either add -f so that non-2xx responses trigger retry_curl retries, or add an inner retry loop for 429/5xx status codes. |
||
| if [[ "${HTTP_STATUS}" == "200" ]]; then | ||
| echo "Found release ${VERSION_URL}; downloading pre-built binary" | ||
| echo "install-method=release" >> "${GITHUB_OUTPUT}" | ||
| echo "version-url=${VERSION_URL}" >> "${GITHUB_OUTPUT}" | ||
| echo "version-asset=${VERSION_ASSET}" >> "${GITHUB_OUTPUT}" | ||
| elif [[ "${HTTP_STATUS}" == "404" ]]; then | ||
| echo "No release found for ${VERSION_URL}; building from source at ref: ${VERSION}" | ||
| echo "install-method=source" >> "${GITHUB_OUTPUT}" | ||
| echo "source-ref=${VERSION}" >> "${GITHUB_OUTPUT}" | ||
| else | ||
| echo "::error::Unexpected HTTP ${HTTP_STATUS} checking release ${VERSION_URL}; cannot proceed" | ||
| exit 1 | ||
| fi | ||
|
|
||
| - name: Install vendored binary | ||
| if: steps.detect.outputs.install-method == 'vendored' | ||
| shell: bash | ||
| env: | ||
| VENDORED: ${{ steps.detect.outputs.vendored-path }} | ||
| run: | | ||
| set -euo pipefail | ||
| mkdir -p "${RUNNER_TEMP}/fullsend" | ||
| cp "${VENDORED}" "${RUNNER_TEMP}/fullsend/fullsend" | ||
| chmod +x "${RUNNER_TEMP}/fullsend/fullsend" | ||
| echo "${RUNNER_TEMP}/fullsend" >> "${GITHUB_PATH}" | ||
|
|
||
| - name: Download release binary | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] missing validation If the detect step completes without setting install-method, all install steps are silently skipped. Defense-in-depth concern, not a current bug. |
||
| if: steps.detect.outputs.install-method == 'release' | ||
| shell: bash | ||
| env: | ||
| VERSION_URL: ${{ steps.detect.outputs.version-url }} | ||
| VERSION_ASSET: ${{ steps.detect.outputs.version-asset }} | ||
| RUNNER_OS: ${{ runner.os }} | ||
| RUNNER_ARCH: ${{ runner.arch }} | ||
| GH_TOKEN: ${{ inputs.github_token }} | ||
| run: | | ||
| set -euo pipefail | ||
|
|
||
| # retry_curl: retry a curl command up to MAX_ATTEMPTS times with exponential backoff. | ||
| # Usage: retry_curl [curl args...] | ||
| retry_curl() { | ||
| local max_attempts=3 | ||
| local attempt=1 | ||
| local delay=5 | ||
| while true; do | ||
| if curl "$@"; then | ||
| return 0 | ||
| fi | ||
| if (( attempt >= max_attempts )); then | ||
| echo "::error::Download failed after ${max_attempts} attempts" | ||
| return 1 | ||
| fi | ||
| echo "::warning::Download attempt ${attempt}/${max_attempts} failed, retrying in ${delay}s..." | ||
| sleep "${delay}" | ||
| (( attempt++ )) | ||
| (( delay *= 3 )) | ||
| done | ||
| } | ||
|
|
||
| os="$(echo "${RUNNER_OS}" | tr '[:upper:]' '[:lower:]')" | ||
| case "${os}" in | ||
| macos) os=darwin ;; | ||
| esac | ||
|
|
||
| arch="$(echo "${RUNNER_ARCH}" | tr '[:upper:]' '[:lower:]')" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] edge case When shallow fetch fails, the fallback performs a full git clone. If SOURCE_REF is invalid, the clone succeeds but git checkout fails, wasting time and resources. Suggested fix: After the full clone, validate SOURCE_REF exists before proceeding. |
||
| case "${arch}" in | ||
| x64) arch=amd64 ;; | ||
| x86) arch=386 ;; | ||
| arm64|aarch64) arch=arm64 ;; | ||
| esac | ||
|
|
||
| ASSET_NAME="fullsend_${VERSION_ASSET}_${os}_${arch}.tar.gz" | ||
| URL="${BASE_URL}/${ASSET_NAME}" | ||
| URL="https://github.com/fullsend-ai/fullsend/releases/download/${VERSION_URL}/${ASSET_NAME}" | ||
|
|
||
| echo "Downloading from: ${URL}" | ||
| retry_curl -fsSL "$URL" -o "/tmp/${ASSET_NAME}" | ||
| retry_curl -fsSL "${URL}" -o "/tmp/${ASSET_NAME}" | ||
| mkdir -p "${RUNNER_TEMP}/fullsend" | ||
| tar -xzf "/tmp/${ASSET_NAME}" -C "${RUNNER_TEMP}/fullsend" | ||
| echo "${RUNNER_TEMP}/fullsend" >> "${GITHUB_PATH}" | ||
|
|
||
| - name: Clone fullsend at ref for source build | ||
| if: steps.detect.outputs.install-method == 'source' | ||
| shell: bash | ||
| env: | ||
| SOURCE_REF: ${{ steps.detect.outputs.source-ref }} | ||
| GH_TOKEN: ${{ inputs.github_token }} | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low] edge case Source build uses make go-build which relies on git describe --tags. In a shallow clone, tags are not fetched, so the binary reports an inaccurate version string. |
||
| run: | | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [non-blocking] If |
||
| set -euo pipefail | ||
| echo "::add-mask::${GH_TOKEN}" | ||
| SRC="${RUNNER_TEMP}/fullsend-src" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [medium] edge case The source build path uses 'git fetch --depth 1 origin "${SOURCE_REF}"'. If SOURCE_REF is a full commit SHA, this will fail because GitHub does not allow fetching arbitrary commit SHAs via shallow fetch. Branch names and tags work, but SHAs fail with a confusing 'unadvertised object' error. Suggested fix: Document that SOURCE_REF must be a branch name or tag. Alternatively, fall back to a full clone or use the GitHub archive API for SHA-based refs.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we'll need SHA support here — for e2e and CI, pinning to a specific commit is the main use case for source builds. |
||
| REMOTE="https://x-access-token:${GH_TOKEN}@github.com/fullsend-ai/fullsend.git" | ||
| echo "Cloning fullsend at ref: ${SOURCE_REF}" | ||
| git init "${SRC}" | ||
| git -C "${SRC}" remote add origin "${REMOTE}" | ||
| # Shallow fetch works for branch/tag refs but not bare commit SHAs (GitHub disallows | ||
| # unadvertised SHA fetches). Fall back to a full clone when shallow fetch fails. | ||
| if ! git -C "${SRC}" fetch --depth 1 origin "${SOURCE_REF}"; then | ||
| echo "Shallow fetch failed (ref may be a commit SHA); falling back to full clone" | ||
| rm -rf "${SRC}" | ||
| git clone "${REMOTE}" "${SRC}" | ||
| git -C "${SRC}" checkout "${SOURCE_REF}" | ||
| else | ||
| git -C "${SRC}" checkout FETCH_HEAD | ||
| fi | ||
|
|
||
| - name: Set up Go for source build | ||
| if: steps.detect.outputs.install-method == 'source' | ||
| uses: actions/setup-go@v6 | ||
| with: | ||
| go-version-file: ${{ runner.temp }}/fullsend-src/go.mod | ||
| cache-dependency-path: ${{ runner.temp }}/fullsend-src/go.sum | ||
|
|
||
| - name: Build fullsend from source | ||
| if: steps.detect.outputs.install-method == 'source' | ||
| shell: bash | ||
| run: | | ||
| set -euo pipefail | ||
| mkdir -p "${RUNNER_TEMP}/fullsend" | ||
| cd "${RUNNER_TEMP}/fullsend-src" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [important] The Makefile already handles version stamping — Without ldflags, |
||
| make go-build | ||
| cp bin/fullsend "${RUNNER_TEMP}/fullsend/fullsend" | ||
| echo "${RUNNER_TEMP}/fullsend" >> "${GITHUB_PATH}" | ||
|
|
||
| - name: Print fullsend version | ||
| shell: bash | ||
| run: fullsend --version | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,51 +1,69 @@ | ||
| # Testing workflow changes | ||
|
|
||
| This guide explains how to test changes to Fullsend's GitHub Actions workflows. | ||
| This guide explains how to test changes to Fullsend's GitHub Actions workflows, composite actions, and the CLI itself. | ||
|
|
||
| ## References | ||
|
|
||
| There are independent version reference inputs that control different parts of the system: | ||
|
|
||
| | Input | Controls | Where set | | ||
| |-------|----------|-----------| | ||
| | `@<ref>` on `uses:` | Which reusable workflow YAML runs | The `uses:` line in the caller workflow | | ||
| | `fullsend_ai_ref` | Which ref composite actions (`action.yml`) and defaults are loaded from at runtime | Passed as a `with:` input | | ||
| | `fullsend_version` | Which fullsend CLI binary is installed | Passed as a `with:` input | | ||
|
|
||
| If `uses:`, `fullsend_ai_ref` and `fullsend_version` diverge, the workflows, agents and harnesses, and | ||
| CLI diverge, potentially causing mismatch in behavior and failures. | ||
|
|
||
| ## Per-repo mode | ||
|
|
||
| In your repository modify the dispatch job at `.github/workflows/fullsend.yaml` to | ||
| use the ref you want to test. Change the reference `uses` use and | ||
| `fullsend_ai_ref` to the same value. | ||
| use the ref you want to test: | ||
|
|
||
| ```yaml | ||
| # .github/workflows/fullsend.yaml | ||
| # [...] | ||
| jobs: | ||
| dispatch: | ||
| # [...] | ||
| uses: fullsend-ai/fullsend/.github/workflows/reusable-dispatch.yml@<YOUR_VERSION> | ||
| uses: fullsend-ai/fullsend/.github/workflows/reusable-dispatch.yml@<YOUR_BRANCH> | ||
| with: | ||
| # [...] | ||
| fullsend_ai_ref: <YOUR_VERSION> | ||
| fullsend_ai_ref: <YOUR_BRANCH> | ||
| fullsend_version: <YOUR_BRANCH> | ||
| # [...] | ||
| ``` | ||
|
|
||
| Then push this change and trigger a Fullsend action: `/fs-triage`, `/fs-code`, ... When the ref is | ||
| deleted from fullsend-ai/fullsend (branch deleted or commit amended), revert this back to the | ||
| desired reference. | ||
|
|
||
| **Note**: for forks, change the `fullsend-ai/fullsend` portion to point to your fork. | ||
|
|
||
| ## Per-org mode | ||
|
|
||
| **WARNING**: this impacts all repositories, so proceed with care. You can install your test repository | ||
| using the repository install mode to avoid this problem. | ||
|
|
||
| In your `.fullsend` repository modify the desired stage workflow file (triage in the example below). | ||
| Change the reference on `uses` for the `reusable-<stage>.yml` and the `fullsend_ai_ref` passed to it: | ||
| In your `.fullsend` repository change the references for the `reusable-<stage>.yml` you want to | ||
| test (triage in the example below): | ||
|
|
||
| ```yaml | ||
| # .github/workflows/triage.yml | ||
| # [...] | ||
| jobs: | ||
| triage: | ||
| # [...] | ||
| uses: fullsend-ai/fullsend/.github/workflows/reusable-triage.yml@<YOUR_VERSION> | ||
| uses: fullsend-ai/fullsend/.github/workflows/reusable-triage.yml@<YOUR_BRANCH> | ||
| with: | ||
| # [...] | ||
| fullsend_ai_ref: <YOUR_VERSION> | ||
| fullsend_ai_ref: <YOUR_BRANCH> | ||
| fullsend_version: <YOUR_BRANCH> | ||
| # [...] | ||
| ``` | ||
|
|
||
| Then push this change and trigger a Fullsend action on your test repository: `/fs-triage`, `/fs-code`, ... | ||
| When the ref is deleted from fullsend-ai/fullsend (branch deleted or commit amended), revert this back | ||
| to the desired reference. | ||
|
|
||
| **Note**: for forks, change the `fullsend-ai/fullsend` portion to point to your fork. |
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.
[medium] error handling gap
The 'Detect install method' step resolves the 'latest' tag and checks release existence using plain curl without retries. The original code used retry_curl (3 attempts with exponential backoff). The retry_curl function is only defined in the 'Download release binary' step. This is a resilience regression.
Suggested fix: Either define retry_curl in the detect step as well, or extract it into a shared function defined before both steps.
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.
+1 — I'm battling transient API flakes in e2e right now and retries everywhere help. This one's worth fixing before we merge.