build(action): action.yml now compiles from source if not release

[rh-hemartin]

Summary

Now the action.yml tries to find a version that matches, if it does not it clones the repository at the ref and then compiles the binary.

[github-actions]

Site preview

Preview: https://cc1630d5-site.fullsend-ai.workers.dev

Commit: 3ffbdb5b32fd39f0c4982b6bab53d848ec3efa2d

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 7:33 AM UTC · Completed 7:48 AM UTC
Commit: ba204cb · View workflow run →

[fullsend-ai-review]

Review

Findings

Medium

• [error handling gap] action.yml:121 — 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 rate-limit, 503 service unavailable), curl succeeds on the first attempt, retry_curl does not retry, and the step falls into the else branch which exits with an error. While the else branch correctly surfaces the unexpected status, the lack of retry for transient API errors is a resilience gap.
  Remediation: Either (a) add -f so that non-2xx responses cause a non-zero curl exit code and trigger retry_curl retries, then parse the status separately afterward, or (b) add an inner retry loop that retries on 429/5xx status codes before reaching the 200/404/else branch.

• [scope-mismatch] action.yml — The PR title uses the build commit type, but this change introduces new user-facing behavior (source compilation fallback). Per CONTRIBUTING.md, build: commits are excluded from release notes entirely. The correct type is feat: (new functionality). This is not cosmetic — GoReleaser parses commit prefixes to build release notes, and wrong prefixes produce incorrect changelogs.
  Remediation: Change the PR title from build(action): to feat(action):.

Low

• [edge case] action.yml:186 — When shallow fetch fails for a commit SHA, the fallback performs a full git clone of the entire repository. If SOURCE_REF is an invalid ref (e.g., a typo), the full clone succeeds but git checkout fails, wasting time and runner resources on a doomed build.
  Remediation: After the full clone, validate that SOURCE_REF exists before proceeding (e.g., git rev-parse --verify) and fail fast. Consider using git clone --filter=blob:none for the fallback.

• [command-injection] action.yml — SOURCE_REF (derived from inputs.version) is passed to git fetch --depth 1 origin "${SOURCE_REF}" and git checkout "${SOURCE_REF}" without validation that it doesn't start with -. If a caller passes a version like --upload-pack=malicious-command, git fetch would interpret it as a flag. Exploitability is limited since inputs.version is controlled by workflow callers, not external users.
  Remediation: Validate that SOURCE_REF does not start with -: [[ "${SOURCE_REF}" != -* ]] || { echo '::error::Invalid ref'; exit 1; }.

• [missing validation] action.yml:148 — If the detect step somehow completes without setting install-method (no current code path does this, but future edits could), all three install steps are silently skipped and fullsend --version fails with a confusing 'command not found'. Defense-in-depth concern.

• [edge case] action.yml:207 — The source build runs make go-build, which uses git describe --tags --always --dirty. In a shallow clone (depth 1), tags are not fetched, so the version string falls back to a short commit SHA or 'dev'. The resulting binary reports an inaccurate version.

• [secret-exposure] action.yml — The source build step embeds GH_TOKEN directly in the git remote URL (https://x-access-token:${GH_TOKEN}@...). While ::add-mask:: is called, embedding tokens in URLs is a known anti-pattern — git error messages, verbose output, and .git/config can expose the URL.
  Remediation: Consider using `git -c http.extraHeader="Authorization: Bearer ${GH... instead of embedding the token in the URL.

• [architectural-coherence] action.yml — The new source-build path allows arbitrary git refs to be built from source when no release exists, which changes the distribution model from ADR-0031's release-pinning paradigm. Without a linked issue or design documentation, the intent (authorized evolution vs. unplanned expansion) is unclear. See also: [composite-action-behavior] finding on ADR-0031.
  Remediation: If intended: document the design rationale and consider amending ADR-0031. If unintended: limit the source-build path to recognized ref patterns.

• [Variable naming] action.yml — Output variables use kebab-case (install-method, vendored-path) while some existing composite actions use snake_case for GITHUB_OUTPUT keys (pr_number, file_summary). The codebase uses both conventions, but aligning to one would improve consistency.

• [composite-action-behavior] docs/ADRs/0031-reusable-workflows-for-action-installed-distribution.md:80 — ADR 0031 describes composite actions but does not document the tri-modal install strategy (vendored → release → source). See also: [architectural-coherence] finding at action.yml.

• [version-input-semantics] docs/ADRs/0033-per-repo-installation-mode.md:125 — ADR 0033 does not document that the version input now has expanded semantics (release tag, branch, or SHA with source build fallback).

Info

• [injection] action.yml — Multiple echo statements interpolate VERSION-derived values without sanitizing :: sequences. Exploitability is negligible since inputs.version is set by workflow callers and the remaining injectable commands (::error::, ::warning::) only affect log annotations.

• [version-input-semantics] docs/ADRs/0035-layered-content-resolution.md:62 — ADR 0035 does not clarify how fullsend_version (with source build fallback) interacts with fullsend_ai_ref. These are orthogonal concerns but users may expect them to be synchronized.

• [Code organization] action.yml — Splitting the monolithic install step into conditional steps is a readability improvement.

Previous run

Review

Findings

Medium

• [error handling gap] action.yml:97 — 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. A valid released version with a temporarily unavailable API triggers an expensive, likely-failing source build instead of surfacing the real error.
  Remediation: 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.

• [error handling gap] action.yml:68 — 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) for the same API call. The retry_curl function is only defined in the "Download release binary" step and is unavailable in the detect step. This is a resilience regression — transient GitHub API failures now immediately fail the workflow. See also: [error handling consistency] finding at this location.
  Remediation: Either define retry_curl in the detect step as well, or extract it into a shared function defined before both steps.

• [edge case] action.yml:186 — 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 (requires uploadpack.allowReachableSHA1InWant). Branch names and tags work fine, but SHA-based refs will fail with a confusing "unadvertised object" error.
  Remediation: Document that SOURCE_REF must be a branch name or tag, not a bare SHA. Alternatively, fall back to a full clone or use the GitHub archive API for SHA-based refs.

• [missing-authorization] action.yml — This PR introduces a new feature (source compilation fallback for non-release versions) without a linked issue. Non-trivial structural changes benefit from explicit authorization via a linked issue documenting the problem, design tradeoffs, and maintainer sign-off.
  Remediation: Create a GitHub issue documenting the problem being solved and link it to the PR.

• [scope-mismatch] action.yml — The PR title uses the build commit type, but this change introduces new user-facing behavior (compile from source when a branch/ref doesn't match a release tag). This is a feat (new functionality), not a build change. The commit type will cause the feature to be excluded from release notes.
  Remediation: Change the PR title to feat(action): compile fullsend from source when version matches no release.

• [composite-action-behavior] docs/ADRs/0031-reusable-workflows-for-action-installed-distribution.md:80 — ADR 0031 describes composite actions but does not document that the composite action now supports a tri-modal install (vendored → release → source build). Adding a brief note would keep the ADR current.
  Remediation: Add a note documenting the three installation modes.

Low

• [injection] action.yml — The VERSION input undergoes whitespace stripping but does not strip :: sequences before being echoed in log messages. A crafted VERSION containing ::error:: or ::warning:: sequences could emit workflow commands. Exploitability is limited since inputs.version is set by workflow callers, not arbitrary external users.

• [secret-exposure] action.yml — The source build step embeds GH_TOKEN directly in the git remote URL (https://x-access-token:${GH_TOKEN}@...). If git fetch fails, git may log the full URL including the token. While github.token is auto-masked, custom tokens may not be.

• [architectural-coherence] action.yml — The new source-build path allows arbitrary git refs to be built from source, which expands the trust surface beyond the release-pinning model described in ADR-0031. Consider documenting that source builds are intended for development/testing only.

• [design-coherence] docs/guides/dev/testing-workflows.md:19 — The documentation adds fullsend_version: <YOUR_BRANCH> but lacks guidance on when it's safe to diverge uses:, fullsend_ai_ref, and fullsend_version, or a note that production should use released tags only.

• [missing validation] action.yml:65 — No explicit validation that the detect step produced a valid install-method output. If an unexpected code path is reached, all conditional steps are silently skipped. The set -euo pipefail and downstream fullsend --version check mitigate this, but an explicit guard would improve debuggability.

• [Variable naming] action.yml:13 — Output variables use kebab-case (install-method, version-url) while other composite actions in the repo use snake_case or single words for GITHUB_OUTPUT keys.

• [Informational logging] action.yml:20 — Log messages use future tense and semicolons ("will download", "will build") where existing codebase uses terse present-tense style.

• [Error handling - message format] action.yml:34 — The "No release found" fallback message is a plain echo for normal behavior; consider ::notice:: for informational fallback messages.

• [version-input-semantics] docs/ADRs/0033-per-repo-installation-mode.md:125 — ADR 0033 shows version examples but does not reflect the expanded version input semantics.

• [version-input-semantics] docs/ADRs/0035-layered-content-resolution.md:62 — ADR 0035 does not document how source builds interact with the layering model.

Info

• [Code organization] action.yml:43 — Splitting the monolithic install step into conditional steps is a readability improvement, though it departs from other composite actions' single-step pattern.

• [testing-workflow-reference] docs/architecture.md:614 — The architecture document's @<version> references describe standard GitHub Actions ref syntax, not the version input specifically.

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-review]

[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.

[fullsend-ai-review]

[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.

[ralphbean]

+1 — I'm battling transient API flakes in e2e right now and retries everywhere help. This one's worth fixing before we merge.

[fullsend-ai-review]

[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.

[ralphbean]

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.

[ralphbean]

The three-tier fallback makes sense. A few things to sort out — mostly around making the detect step as resilient as the download step, and getting the version stamp right for source builds. See inline comments and the threads on the bot's review.

[ralphbean]

[important] The Makefile already handles version stamping — make go-build passes -ldflags with git describe --tags --always --dirty. Could we use make go-build here and then copy bin/fullsend into place? That keeps the version logic in one place, and fullsend --version would show the actual commit ref instead of dev.

Without ldflags, internal/cli.version defaults to "dev", and the CLI uses that string in binary.ResolveForRun and binary.ResolveForVendor — so it's not just cosmetic.

[ralphbean]

[non-blocking] If git fetch fails, the token embedded in the remote URL could end up in stderr. The default GITHUB_TOKEN gets auto-masked by Actions, but a custom PAT might not. An ::add-mask::${GH_TOKEN} before the git commands would cover that edge.

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 1:25 PM UTC · Completed 1:39 PM UTC
Commit: 4ed6da4 · View workflow run →

[fullsend-ai-review]

See the review comment for full details.

[fullsend-ai-review]

[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.

[fullsend-ai-review]

[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.

[fullsend-ai-review]

[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.

[fullsend-ai-review]

[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.

[ralphbean]

All four items from my previous review are addressed in the rebase. LGTM.

[fullsend-ai-retro]

🤖 Finished Retro · ✅ Success · Started 6:56 AM UTC · Completed 7:01 AM UTC
Commit: 3ffbdb5 · View workflow run →

[fullsend-ai-retro]

Retro: PR #2053 — build(action): action.yml now compiles from source if not release

Workflow: Human-authored PR (rh-hemartin + Claude Sonnet) → 2 review agent runs → 1 human review (ralphbean) → fixes → human approval → merge. Two-iteration cycle, reasonable turnaround.

What went well:

• The review agent caught valid resilience issues (retry gaps, SHA shallow-fetch) that the human reviewer confirmed and endorsed.
• Agent and human reviews were complementary — the agent found error-handling gaps while the human found build-system architectural issues.
• The author addressed all feedback in a single rebase, and the human approved on second pass.

What could go better:

• The agent rated the version stamping problem as [low: edge case] ("shallow clone doesn't fetch tags"), but missed the real issue: the PR used raw go build instead of the existing make go-build target that already handles -ldflags version stamping. The human correctly identified this as [important] because fullsend --version reporting "dev" affects runtime binary resolution logic. This is a correctness bug, not an edge case.
• One proposal filed below for build-system awareness during review. Several other potential proposals (duplicate findings across runs, severity calibration, incremental re-review) are already tracked by existing open issues: Review agent should deduplicate findings across iterations on the same PR #1013, Severity calibration: silent-failure correctness bugs should not be rated [low] #1144, Review agent should not re-request changes for unchanged findings #1500, Pass prior review findings to follow-up reviews for targeted verification #1552, Review agent should catch all findings in the first pass to reduce rework cycles #1582.

Proposals filed

• Review agent should consult Makefile/build targets when reviewing build-related changes (in fullsend-ai/fullsend)