fix(cli): address PR review findings for binary vendoring

Validate --fullsend-binary requires --vendor-fullsend-binary, update flag
help text, improve download size-limit errors, restore run arch hint and
120s OIDC timeout, and add ResolveForRun fallback tests.

Signed-off-by: Barak Korren <bkorren@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

Author: ifireball

docs/guides/dev/cli-internals.md

@@ -243,7 +243,7 @@ Linux binary resolution for `fullsend run` and vendoring lives in `internal/bina
 
 | Function | Policy |
 |----------|--------|
-| `ResolveForRun` | Release download (if released CLI) → cross-compile → latest release |
+| `ResolveForRun` | Release download (released CLI only) → cross-compile → latest release |
 | `ResolveForVendor` | Cross-compile → matching release (released CLI only) → fail (no latest) |
 | `ResolveExplicit` | Validate linux/{arch} ELF for `--fullsend-binary` |
 

internal/binary/download.go

@@ -18,11 +18,18 @@ import (
 )
 
 // ReleaseBaseURL is the GitHub releases download base URL. Tests may override.
+// Not safe for concurrent test mutation.
 var ReleaseBaseURL = "https://github.com/fullsend-ai/fullsend/releases/download"
 
 // HTTPClient is used for release downloads. Tests may override.
+// Not safe for concurrent test mutation.
 var HTTPClient = &http.Client{Timeout: 120 * time.Second}
 
+const defaultMaxDownloadSize = 200 * 1024 * 1024 // 200 MB compressed
+
+// maxDownloadSize caps release asset downloads. Tests may lower temporarily.
+var maxDownloadSize = defaultMaxDownloadSize
+
 const maxBinarySize = 500 * 1024 * 1024 // 500 MB — reasonable upper bound for a Go binary
 
 // DownloadRelease downloads the fullsend binary for linux/{arch} from the
@@ -48,11 +55,14 @@ func DownloadRelease(ver, arch, destPath string) error {
 		return fmt.Errorf("GET %s returned %d", url, resp.StatusCode)
 	}
 
-	const maxDownloadSize = 200 * 1024 * 1024 // 200 MB compressed
+	maxSize := int64(maxDownloadSize)
 	var buf bytes.Buffer
-	if _, err := io.Copy(&buf, io.LimitReader(resp.Body, maxDownloadSize)); err != nil {
+	if _, err := io.Copy(&buf, io.LimitReader(resp.Body, maxSize+1)); err != nil {
 		return fmt.Errorf("reading %s: %w", assetName, err)
 	}
+	if int64(buf.Len()) > maxSize {
+		return fmt.Errorf("download of %s exceeds maximum size (%d bytes)", assetName, maxSize)
+	}
 
 	h := sha256.Sum256(buf.Bytes())
 	actualHash := hex.EncodeToString(h[:])

internal/binary/download_test.go

@@ -16,11 +16,42 @@ import (
 	"strings"
 	"sync/atomic"
 	"testing"
+	"time"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
+type redirectTransport struct {
+	srvURL string
+	base   http.RoundTripper
+}
+
+func (t redirectTransport) RoundTrip(req *http.Request) (*http.Response, error) {
+	clone := req.Clone(req.Context())
+	clone.URL.Scheme = "http"
+	clone.URL.Host = strings.TrimPrefix(strings.TrimPrefix(t.srvURL, "https://"), "http://")
+	if t.base == nil {
+		t.base = http.DefaultTransport
+	}
+	return t.base.RoundTrip(clone)
+}
+
+func withTestReleaseServer(t *testing.T, srv *httptest.Server) {
+	t.Helper()
+	origClient := HTTPClient
+	origBaseURL := ReleaseBaseURL
+	HTTPClient = &http.Client{
+		Transport: redirectTransport{srvURL: srv.URL},
+		Timeout:   120 * time.Second,
+	}
+	ReleaseBaseURL = srv.URL
+	t.Cleanup(func() {
+		HTTPClient = origClient
+		ReleaseBaseURL = origBaseURL
+	})
+}
+
 func TestExtractFullsendFromTarGz_PathTraversal(t *testing.T) {
 	var buf bytes.Buffer
 	gw := gzip.NewWriter(&buf)
@@ -412,6 +443,132 @@ func TestResolveForRun_PrefersReleaseBeforeCrossCompile(t *testing.T) {
 	assert.Equal(t, SourceReleaseDownload, result.Source)
 }
 
+func TestDownloadRelease_ExceedsMaxSize(t *testing.T) {
+	origLimit := maxDownloadSize
+	maxDownloadSize = 512
+	t.Cleanup(func() { maxDownloadSize = origLimit })
+
+	content := bytes.Repeat([]byte("x"), 2000)
+
+	var tarBuf bytes.Buffer
+	gw, err := gzip.NewWriterLevel(&tarBuf, gzip.NoCompression)
+	require.NoError(t, err)
+	tw := tar.NewWriter(gw)
+	require.NoError(t, tw.WriteHeader(&tar.Header{
+		Name:     "fullsend",
+		Size:     int64(len(content)),
+		Mode:     0o755,
+		Typeflag: tar.TypeReg,
+	}))
+	_, err = tw.Write(content)
+	require.NoError(t, err)
+	require.NoError(t, tw.Close())
+	require.NoError(t, gw.Close())
+
+	tarBytes := tarBuf.Bytes()
+	h := sha256.Sum256(tarBytes)
+	checksumBody := fmt.Sprintf("%s  fullsend_1.0.0_linux_amd64.tar.gz\n", hex.EncodeToString(h[:]))
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/v1.0.0/checksums.txt" {
+			fmt.Fprint(w, checksumBody)
+		} else if r.URL.Path == "/v1.0.0/fullsend_1.0.0_linux_amd64.tar.gz" {
+			w.Write(tarBytes)
+		} else {
+			http.NotFound(w, r)
+		}
+	}))
+	defer srv.Close()
+	withTestReleaseServer(t, srv)
+
+	destPath := filepath.Join(t.TempDir(), "fullsend")
+	err = DownloadRelease("1.0.0", "amd64", destPath)
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "exceeds maximum size")
+}
+
+func TestResolveForRun_CrossCompileFallback(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping cross-compilation in short mode")
+	}
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.NotFound(w, r)
+	}))
+	defer srv.Close()
+	withTestReleaseServer(t, srv)
+
+	result, err := ResolveForRun("0.4.0", "amd64")
+	require.NoError(t, err)
+	t.Cleanup(func() { os.RemoveAll(result.TmpDir) })
+	assert.Equal(t, SourceCheckoutBuild, result.Source)
+}
+
+func TestResolveForRun_LatestReleaseFallback(t *testing.T) {
+	var tarBuf bytes.Buffer
+	gw := gzip.NewWriter(&tarBuf)
+	tw := tar.NewWriter(gw)
+	content := []byte("latest release binary")
+	require.NoError(t, tw.WriteHeader(&tar.Header{
+		Name:     "fullsend",
+		Size:     int64(len(content)),
+		Mode:     0o755,
+		Typeflag: tar.TypeReg,
+	}))
+	_, err := tw.Write(content)
+	require.NoError(t, err)
+	require.NoError(t, tw.Close())
+	require.NoError(t, gw.Close())
+
+	tarBytes := tarBuf.Bytes()
+	h := sha256.Sum256(tarBytes)
+	correctHash := hex.EncodeToString(h[:])
+	checksumBody := fmt.Sprintf("%s  fullsend_9.9.9_linux_amd64.tar.gz\n", correctHash)
+
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/repos/fullsend-ai/fullsend/releases/latest" {
+			fmt.Fprint(w, `{"tag_name":"v9.9.9"}`)
+		} else if r.URL.Path == "/v9.9.9/checksums.txt" {
+			fmt.Fprint(w, checksumBody)
+		} else if r.URL.Path == "/v9.9.9/fullsend_9.9.9_linux_amd64.tar.gz" {
+			w.Write(tarBytes)
+		} else {
+			http.NotFound(w, r)
+		}
+	}))
+	defer srv.Close()
+	withTestReleaseServer(t, srv)
+
+	origDir, err := os.Getwd()
+	require.NoError(t, err)
+	tmpDir := t.TempDir()
+	require.NoError(t, os.Chdir(tmpDir))
+	t.Cleanup(func() { _ = os.Chdir(origDir) })
+
+	result, err := ResolveForRun("dev", "amd64")
+	require.NoError(t, err)
+	t.Cleanup(func() { os.RemoveAll(result.TmpDir) })
+	assert.Equal(t, SourceReleaseDownload, result.Source)
+}
+
+func TestResolveForRun_AllStrategiesFail(t *testing.T) {
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.NotFound(w, r)
+	}))
+	defer srv.Close()
+	withTestReleaseServer(t, srv)
+
+	origDir, err := os.Getwd()
+	require.NoError(t, err)
+	tmpDir := t.TempDir()
+	require.NoError(t, os.Chdir(tmpDir))
+	t.Cleanup(func() { _ = os.Chdir(origDir) })
+
+	_, err = ResolveForRun("dev", "amd64")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "all strategies failed")
+}
+
 func TestResolveExplicit_ValidatesELF(t *testing.T) {
 	tmp := filepath.Join(t.TempDir(), "not-elf")
 	require.NoError(t, os.WriteFile(tmp, []byte("not binary"), 0o644))

internal/cli/admin.go

@@ -272,6 +272,9 @@ Inference authentication:
 			if err := appsetup.ValidateAppSet(appSet); err != nil {
 				return fmt.Errorf("invalid --app-set: %w", err)
 			}
+			if err := validateVendorBinaryFlags(vendorBinary, fullsendBinary); err != nil {
+				return err
+			}
 
 			arg := args[0]
 			if strings.Contains(arg, "/") {
@@ -543,7 +546,7 @@ Inference authentication:
 	cmd.Flags().StringVar(&agents, "agents", strings.Join(config.DefaultAgentRoles(), ","), "comma-separated agent roles")
 	cmd.Flags().BoolVar(&dryRun, "dry-run", false, "preview changes without making them")
 	cmd.Flags().BoolVar(&skipAppSetup, "skip-app-setup", false, "skip GitHub App creation/setup")
-	cmd.Flags().BoolVar(&vendorBinary, "vendor-fullsend-binary", false, "cross-compile and vendor the fullsend binary for development iteration")
+	cmd.Flags().BoolVar(&vendorBinary, "vendor-fullsend-binary", false, "resolve and upload a linux/amd64 fullsend binary for CI")
 	cmd.Flags().StringVar(&fullsendBinary, "fullsend-binary", "", "path to a Linux fullsend binary to upload when vendoring (default: auto-resolve)")
 	cmd.Flags().BoolVar(&enrollAllFlag, "enroll-all", false, "enroll all repositories without prompting")
 	cmd.Flags().BoolVar(&enrollNoneFlag, "enroll-none", false, "skip repository enrollment without prompting")

internal/cli/github.go

@@ -90,6 +90,9 @@ values (mint URL, WIF provider, project ID) are provided as flags.`,
 			if err := appsetup.ValidateAppSet(cfg.appSet); err != nil {
 				return fmt.Errorf("invalid --app-set: %w", err)
 			}
+			if err := validateVendorBinaryFlags(cfg.vendorBinary, cfg.fullsendBinary); err != nil {
+				return err
+			}
 
 			if err := validateMintURLHTTPS(cfg.mintURL); err != nil {
 				return err
@@ -133,7 +136,7 @@ values (mint URL, WIF provider, project ID) are provided as flags.`,
 	cmd.Flags().StringVar(&cfg.appSet, "app-set", appsetup.DefaultAppSet, "app set name prefix for GitHub Apps")
 	cmd.Flags().BoolVar(&cfg.enrollAll, "enroll-all", false, "enroll all repositories without prompting")
 	cmd.Flags().BoolVar(&cfg.enrollNone, "enroll-none", false, "skip repository enrollment without prompting")
-	cmd.Flags().BoolVar(&cfg.vendorBinary, "vendor-fullsend-binary", false, "cross-compile and upload the fullsend binary")
+	cmd.Flags().BoolVar(&cfg.vendorBinary, "vendor-fullsend-binary", false, "resolve and upload a linux/amd64 fullsend binary for CI")
 	cmd.Flags().StringVar(&cfg.fullsendBinary, "fullsend-binary", "", "path to a Linux fullsend binary to upload when vendoring (default: auto-resolve)")
 	cmd.Flags().BoolVar(&cfg.dryRun, "dry-run", false, "preview changes without making them")
 

internal/cli/run.go

@@ -912,7 +912,7 @@ func bootstrapCommon(sandboxName, fullsendBinary string, h *harness.Harness) err
 	}
 	if localBinary != "" {
 		if err := binary.ValidateLinuxBinary(localBinary, sandboxArch()); err != nil {
-			return fmt.Errorf("fullsend binary %q is not valid for the sandbox: %w", localBinary, err)
+			return fmt.Errorf("fullsend binary %q is not valid for the sandbox: %w\nSet FULLSEND_SANDBOX_ARCH to override the target architecture", localBinary, err)
 		}
 		// Use UploadDir (tarball-based) instead of Upload for the binary.
 		// Upload silently fails for large files (~16MB); the tarball
@@ -1233,7 +1233,7 @@ func runOIDCRefresh(ctx context.Context, sandboxName, oidcURL, oidcAuth string,
 	}
 }
 
-var oidcHTTPClient = &http.Client{Timeout: 30 * time.Second}
+var oidcHTTPClient = &http.Client{Timeout: 120 * time.Second} // matches pre-refactor shared httpClient timeout
 
 func refreshOIDCToken(ctx context.Context, sandboxName, oidcURL, oidcAuth string) error {
 	req, err := http.NewRequestWithContext(ctx, "GET", oidcURL, nil)

internal/cli/vendor.go

@@ -13,6 +13,13 @@ import (
 
 const vendorArch = binary.DefaultArch
 
+func validateVendorBinaryFlags(vendorBinary bool, fullsendBinary string) error {
+	if fullsendBinary != "" && !vendorBinary {
+		return fmt.Errorf("--fullsend-binary requires --vendor-fullsend-binary")
+	}
+	return nil
+}
+
 // makeVendorFunc returns a VendorFunc closure that uploads a fullsend binary
 // using the vendoring acquisition policy.
 func makeVendorFunc(fullsendBinary string) layers.VendorFunc {

internal/cli/vendor_test.go

@@ -15,6 +15,16 @@ import (
 	"github.com/fullsend-ai/fullsend/internal/ui"
 )
 
+func TestValidateVendorBinaryFlags(t *testing.T) {
+	require.NoError(t, validateVendorBinaryFlags(false, ""))
+	require.NoError(t, validateVendorBinaryFlags(true, ""))
+	require.NoError(t, validateVendorBinaryFlags(true, "/tmp/fullsend"))
+
+	err := validateVendorBinaryFlags(false, "/tmp/fullsend")
+	require.Error(t, err)
+	assert.Contains(t, err.Error(), "--fullsend-binary requires --vendor-fullsend-binary")
+}
+
 func TestInstallCmd_HasFullsendBinaryFlag(t *testing.T) {
 	cmd := newInstallCmd()
 	flag := cmd.Flags().Lookup("fullsend-binary")