Fix Gen3 ping and upload progress output

.github/workflows/syfon-backend-e2e.yaml

@@ -62,12 +62,9 @@ jobs:
           git checkout --detach "${{ steps.syfon-ref.outputs.ref }}"
           git rev-parse --verify HEAD
 
-      - name: Install test prerequisites
+      - name: Initialize Git LFS
         working-directory: git-drs
-        run: |
-          sudo apt-get update
-          sudo apt-get install -y git-lfs
-          git lfs install --local
+        run: git lfs install --local
 
       - name: Run Docker-backed git-drs integration tests
         working-directory: git-drs

cmd/ping/main.go

@@ -1,7 +1,9 @@
 package ping
 
 import (
+	"bytes"
 	"context"
+	"encoding/json"
 	"fmt"
 	"io"
 	"log/slog"
@@ -35,8 +37,8 @@ type healthInfo struct {
 }
 
 var pingHealth = func(ctx context.Context, gc *remoteruntime.GitContext) (healthInfo, error) {
-	if gc != nil && gc.IsReadOnly() {
-		serviceInfo, err := pingTerraServiceInfo(ctx, gc.Endpoint)
+	if gc != nil && (gc.IsReadOnly() || gc.RemoteType == config.Gen3ServerType) {
+		serviceInfo, err := pingDRSServiceInfo(ctx, gc.Endpoint)
 		return healthInfo{ServiceInfo: serviceInfo}, err
 	}
 	return healthInfo{}, gc.Client.Health().Ping(ctx)
@@ -76,7 +78,7 @@ var Cmd = &cobra.Command{
 		}
 		fmt.Println("health: ok")
 		if strings.TrimSpace(health.ServiceInfo) != "" {
-			fmt.Printf("service-info: %s\n", health.ServiceInfo)
+			printServiceInfo(health.ServiceInfo)
 		}
 
 		scopeInfo, err := pingScopeAccess(cmd.Context(), gc)
@@ -102,6 +104,15 @@ var Cmd = &cobra.Command{
 	},
 }
 
+func printServiceInfo(raw string) {
+	var formatted bytes.Buffer
+	if err := json.Indent(&formatted, []byte(raw), "  ", "  "); err != nil {
+		fmt.Printf("service-info: %s\n", raw)
+		return
+	}
+	fmt.Printf("service-info:\n  %s\n", formatted.String())
+}
+
 func resolveStatus(args []string, logger *slog.Logger) (statusInfo, *remoteruntime.GitContext, error) {
 	cfg, err := config.LoadConfig()
 	if err != nil {
@@ -230,11 +241,11 @@ func checkScopeAccess(ctx context.Context, gc *remoteruntime.GitContext) (scopeA
 	return info, nil
 }
 
-func pingTerraServiceInfo(ctx context.Context, endpoint string) (string, error) {
+func pingDRSServiceInfo(ctx context.Context, endpoint string) (string, error) {
 	if ctx == nil {
 		ctx = context.Background()
 	}
-	serviceInfoURL, err := terraServiceInfoURL(endpoint)
+	serviceInfoURL, err := drsServiceInfoURL(endpoint)
 	if err != nil {
 		return "", err
 	}
@@ -249,25 +260,25 @@ func pingTerraServiceInfo(ctx context.Context, endpoint string) (string, error)
 	defer resp.Body.Close()
 	serviceInfo, readErr := io.ReadAll(io.LimitReader(resp.Body, 1<<20))
 	if resp.StatusCode < http.StatusOK || resp.StatusCode >= http.StatusMultipleChoices {
-		return "", fmt.Errorf("terra DRS service-info returned %s", resp.Status)
+		return "", fmt.Errorf("DRS service-info returned %s", resp.Status)
 	}
 	if readErr != nil {
 		return "", readErr
 	}
 	return strings.TrimSpace(string(serviceInfo)), nil
 }
 
-func terraServiceInfoURL(endpoint string) (string, error) {
+func drsServiceInfoURL(endpoint string) (string, error) {
 	endpoint = strings.TrimSpace(endpoint)
 	if endpoint == "" {
-		return "", fmt.Errorf("terra endpoint is empty")
+		return "", fmt.Errorf("DRS endpoint is empty")
 	}
 	parsed, err := url.Parse(endpoint)
 	if err != nil {
 		return "", err
 	}
 	if parsed.Scheme == "" || parsed.Host == "" {
-		return "", fmt.Errorf("terra endpoint must be an absolute URL: %q", endpoint)
+		return "", fmt.Errorf("DRS endpoint must be an absolute URL: %q", endpoint)
 	}
 	parsed.Path = strings.TrimRight(parsed.Path, "/") + "/ga4gh/drs/v1/service-info"
 	parsed.RawQuery = ""

cmd/ping/main_test.go

@@ -66,6 +66,29 @@ func TestPingCmdArgs(t *testing.T) {
 	}
 }
 
+func TestPingHealthGen3UsesDRSServiceInfo(t *testing.T) {
+	var requests int
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path != "/ga4gh/drs/v1/service-info" {
+			t.Errorf("unexpected health path: %s", r.URL.Path)
+			http.NotFound(w, r)
+			return
+		}
+		requests++
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"id":"gen3-drs"}`))
+	}))
+	t.Cleanup(server.Close)
+	gc := &remoteruntime.GitContext{RemoteType: config.Gen3ServerType, Endpoint: server.URL}
+	got, err := pingHealth(t.Context(), gc)
+	if err != nil {
+		t.Fatalf("Gen3 DRS service-info ping failed: %v", err)
+	}
+	if requests != 1 || got.ServiceInfo != `{"id":"gen3-drs"}` {
+		t.Fatalf("unexpected Gen3 ping result: requests=%d info=%q", requests, got.ServiceInfo)
+	}
+}
+
 func TestAcceptancePingTerraDRSServer(t *testing.T) {
 	var serviceInfoRequests int
 	terraDRS := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -116,7 +139,7 @@ func TestAcceptancePingTerraDRSServer(t *testing.T) {
 		"type: terra",
 		"endpoint: " + terraDRS.URL,
 		"health: ok",
-		`service-info: {"id":"terra-drs","name":"Terra DRS","type":{"group":"org.ga4gh","artifact":"drs","version":"1.0.0"}}`,
+		"service-info:\n  {\n    \"id\": \"terra-drs\"",
 	} {
 		if !strings.Contains(got, want) {
 			t.Fatalf("expected output to contain %q, got %q", want, got)
@@ -291,7 +314,7 @@ func TestPingRunEPrintsServiceInfo(t *testing.T) {
 	if _, err := io.Copy(&buf, r); err != nil {
 		t.Fatalf("read stdout: %v", err)
 	}
-	if got, want := buf.String(), `service-info: {"id":"terra-drs","name":"Terra DRS"}`; !strings.Contains(got, want) {
+	if got, want := buf.String(), "service-info:\n  {\n    \"id\": \"terra-drs\",\n    \"name\": \"Terra DRS\"\n  }"; !strings.Contains(got, want) {
 		t.Fatalf("expected output to contain %q, got %q", want, got)
 	}
 }

cmd/ping/terra_integration_test.go

@@ -18,7 +18,7 @@ func TestIntegrationPingTerraDRSServer(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
 	t.Cleanup(cancel)
 
-	if _, err := pingTerraServiceInfo(ctx, endpoint); err != nil {
+	if _, err := pingDRSServiceInfo(ctx, endpoint); err != nil {
 		t.Fatalf("ping Terra DRS service-info endpoint %q: %v", endpoint, err)
 	}
 }

docs/commands.md

@@ -129,7 +129,7 @@ What it checks:
 
 - prints the selected remote, remote type, endpoint, scope, bucket, storage prefix, and auth mode
 - runs a health check against the selected remote
-- for Terra DRS remotes, pings the GA4GH DRS service-info endpoint at `<endpoint>/ga4gh/drs/v1/service-info`
+- for Gen3 and Terra DRS remotes, pings the GA4GH DRS service-info endpoint at `<endpoint>/ga4gh/drs/v1/service-info`
 - for Terra/AnVIL TDR-hosted data in production, use `https://data.terra.bio` as the endpoint; Terra also uses DRSHub for DRS URI resolution, but DRSHub is a resolver service rather than the GA4GH DRS service-info host
 - for scoped Syfon-style remotes, verifies that the configured organization/project and bucket are visible and readable
 

internal/transfer/progress_push.go

@@ -22,14 +22,15 @@ type metadataProgress struct {
 }
 
 type UploadProgressRenderer struct {
-	mu        sync.Mutex
-	base      *Renderer
-	err       error
-	planned   bool
-	plan      UploadPlanSummary
-	metadata  metadataProgress
-	files     map[string]*uploadFileProgress
-	fileOrder []string
+	mu         sync.Mutex
+	base       *Renderer
+	err        error
+	planned    bool
+	hadUploads bool
+	plan       UploadPlanSummary
+	metadata   metadataProgress
+	files      map[string]*uploadFileProgress
+	fileOrder  []string
 }
 
 func NewUploadProgressRenderer(out io.Writer) *UploadProgressRenderer {
@@ -102,6 +103,7 @@ func (r *UploadProgressRenderer) OnUploadPlan(plan UploadPlanSummary) {
 
 	r.plan = plan
 	r.planned = plan.TotalFiles > 0
+	r.hadUploads = r.hadUploads || r.planned
 	r.files = make(map[string]*uploadFileProgress, len(plan.Files))
 	r.fileOrder = r.fileOrder[:0]
 	for _, file := range plan.Files {
@@ -178,7 +180,7 @@ func (r *UploadProgressRenderer) Finish() error {
 func (r *UploadProgressRenderer) HadUploads() bool {
 	r.mu.Lock()
 	defer r.mu.Unlock()
-	return r != nil && r.planned
+	return r != nil && r.hadUploads
 }
 
 func (r *UploadProgressRenderer) Err() error {

internal/transfer/progress_push_test.go

@@ -27,6 +27,9 @@ func TestUploadProgressRendererTTY(t *testing.T) {
 	r.OnUploadProgress(UploadProgressEvent{OID: "oid-2", Path: "b.bin", BytesSoFar: 0, TotalBytes: 100, Phase: UploadProgressUploading})
 	r.OnUploadProgress(UploadProgressEvent{OID: "oid-2", Path: "b.bin", BytesSoFar: 100, TotalBytes: 100, Phase: UploadProgressCompleted})
 	r.Finish()
+	if !r.HadUploads() {
+		t.Fatal("completed upload plan must remain visible after Finish")
+	}
 
 	got := out.String()
 	if !strings.Contains(got, "a.bin [============            ]  50.0% 50 B/100 B") {
@@ -79,6 +82,9 @@ func TestUploadProgressRendererMetadataOnly(t *testing.T) {
 	r.OnMetadataProgress(MetadataProgressEvent{Completed: 0, Total: 3, Phase: MetadataProgressRegistering})
 	r.OnMetadataProgress(MetadataProgressEvent{Completed: 3, Total: 3, Phase: MetadataProgressCompleted})
 	r.Finish()
+	if r.HadUploads() {
+		t.Fatal("metadata-only plan reported a payload upload")
+	}
 
 	got := out.String()
 	if !strings.Contains(got, "registering metadata") {