Merge pull request #474 from depot/rob/dep-4015-cli-ci-run-job-flag-regressions

fix: match --job flag against reusable workflow job keys

Author: robstolarz

pkg/cmd/ci/logs.go

@@ -217,21 +217,20 @@ func findLogsJob(resp *civ1.GetRunStatusResponse, originalID, jobKey, workflowFi
 		return nil, "", fmt.Errorf("run %s has no jobs", resp.RunId)
 	}
 
-	// Match by job key (--job flag): exact match on full key or short name.
+	// Match by job key (--job flag): exact > suffix > segment, best tier wins.
 	if jobKey != "" {
-		var exact, short []jobCandidate
+		bestTier := 0
+		tierMatches := map[int][]jobCandidate{}
 		for _, c := range candidates {
-			if c.job.JobKey == jobKey {
-				exact = append(exact, c)
-			} else if jobKeyShort(c.job.JobKey) == jobKey {
-				short = append(short, c)
+			if tier := matchJobKey(c.job.JobKey, jobKey); tier > 0 {
+				tierMatches[tier] = append(tierMatches[tier], c)
+				if bestTier == 0 || tier < bestTier {
+					bestTier = tier
+				}
 			}
 		}
 
-		matches := exact
-		if len(matches) == 0 {
-			matches = short
-		}
+		matches := tierMatches[bestTier]
 
 		displayNames := jobDisplayNames(candidates)
 		switch len(matches) {
@@ -244,12 +243,23 @@ func findLogsJob(resp *civ1.GetRunStatusResponse, originalID, jobKey, workflowFi
 		case 1:
 			return matches[0].job, matches[0].workflowPath, nil
 		default:
-			// Same job key in multiple workflows — need --workflow.
-			var paths []string
+			// Check if ambiguity is cross-workflow or within a single workflow.
+			uniquePaths := map[string]struct{}{}
 			for _, m := range matches {
-				paths = append(paths, m.workflowPath)
+				uniquePaths[m.workflowPath] = struct{}{}
+			}
+			if len(uniquePaths) > 1 {
+				paths := make([]string, 0, len(uniquePaths))
+				for path := range uniquePaths {
+					paths = append(paths, path)
+				}
+				return nil, "", fmt.Errorf("job %q exists in multiple workflows, specify one with --workflow: %s", jobKey, strings.Join(paths, ", "))
+			}
+			keys := make([]string, len(matches))
+			for i, m := range matches {
+				keys[i] = displayNames[m.job.JobKey]
 			}
-			return nil, "", fmt.Errorf("job %q exists in multiple workflows, specify one with --workflow: %s", jobKey, strings.Join(paths, ", "))
+			return nil, "", fmt.Errorf("job %q matches multiple jobs, use a more specific --job value: %s", jobKey, strings.Join(keys, ", "))
 		}
 	}
 

pkg/cmd/ci/logs_test.go

@@ -1,6 +1,7 @@
 package ci
 
 import (
+	"strings"
 	"testing"
 
 	civ1 "github.com/depot/cli/pkg/proto/depot/ci/v1"
@@ -314,6 +315,107 @@ func TestJobDisplayNames_NoColon(t *testing.T) {
 	}
 }
 
+func TestFindLogsJob_SegmentMatch_ReusableWorkflow(t *testing.T) {
+	// Reusable workflow: user passes "bazel" but key is "pr.yaml:bazel:build"
+	resp := &civ1.GetRunStatusResponse{
+		RunId: "run-1",
+		Workflows: []*civ1.WorkflowStatus{
+			{
+				WorkflowPath: ".depot/workflows/pr.yaml",
+				Jobs: []*civ1.JobStatus{
+					{JobId: "job-1", JobKey: "pr.yaml:detect_changes:build", Status: "finished"},
+					{JobId: "job-2", JobKey: "pr.yaml:bazel:build", Status: "running"},
+					{JobId: "job-3", JobKey: "pr.yaml:test_dashboard:test", Status: "queued"},
+				},
+			},
+		},
+	}
+
+	job, _, err := findLogsJob(resp, "run-1", "bazel", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if job.JobId != "job-2" {
+		t.Fatalf("expected job ID %q, got %q", "job-2", job.JobId)
+	}
+}
+
+func TestFindLogsJob_SegmentMatch_InlineWorkflow(t *testing.T) {
+	// CLI-triggered run: key is "_inline_0.yaml:bazel:build"
+	resp := &civ1.GetRunStatusResponse{
+		RunId: "run-1",
+		Workflows: []*civ1.WorkflowStatus{
+			{
+				WorkflowPath: "",
+				Jobs: []*civ1.JobStatus{
+					{JobId: "job-1", JobKey: "_inline_0.yaml:bazel:build", Status: "running"},
+				},
+			},
+		},
+	}
+
+	job, _, err := findLogsJob(resp, "run-1", "bazel", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if job.JobId != "job-1" {
+		t.Fatalf("expected job ID %q, got %q", "job-1", job.JobId)
+	}
+}
+
+func TestFindLogsJob_SegmentMatch_AmbiguousSameWorkflow(t *testing.T) {
+	// Parent job expands to multiple nested jobs within the same workflow.
+	// Error should suggest a more specific --job, not --workflow.
+	resp := &civ1.GetRunStatusResponse{
+		RunId: "run-1",
+		Workflows: []*civ1.WorkflowStatus{
+			{
+				WorkflowPath: ".depot/workflows/ci.yml",
+				Jobs: []*civ1.JobStatus{
+					{JobId: "job-1", JobKey: "ci.yml:backend:build", Status: "running"},
+					{JobId: "job-2", JobKey: "ci.yml:backend:test", Status: "running"},
+				},
+			},
+		},
+	}
+
+	_, _, err := findLogsJob(resp, "run-1", "backend", "")
+	if err == nil {
+		t.Fatal("expected error for ambiguous segment match")
+	}
+	if strings.Contains(err.Error(), "--workflow") {
+		t.Fatalf("error should suggest --job, not --workflow: %v", err)
+	}
+	if !strings.Contains(err.Error(), "more specific --job") {
+		t.Fatalf("expected 'more specific --job' hint, got: %v", err)
+	}
+}
+
+func TestFindLogsJob_ShortPreferredOverSegment(t *testing.T) {
+	// Short name match should take priority over segment match
+	resp := &civ1.GetRunStatusResponse{
+		RunId: "run-1",
+		Workflows: []*civ1.WorkflowStatus{
+			{
+				WorkflowPath: ".depot/workflows/ci.yml",
+				Jobs: []*civ1.JobStatus{
+					{JobId: "job-segment", JobKey: "ci.yml:build:test", Status: "running"},
+					{JobId: "job-short", JobKey: "ci.yml:build", Status: "running"},
+				},
+			},
+		},
+	}
+
+	// "build" short-matches "ci.yml:build" and segment-matches "ci.yml:build:test"
+	job, _, err := findLogsJob(resp, "run-1", "build", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if job.JobId != "job-short" {
+		t.Fatalf("expected short match (job-short), got %q", job.JobId)
+	}
+}
+
 func TestJobKeyShort_MultipleColons(t *testing.T) {
 	got := jobKeyShort("ci.yml:foo:bar")
 	if got != "foo:bar" {

pkg/cmd/ci/ssh.go

@@ -208,22 +208,31 @@ func findJob(resp *civ1.GetRunStatusResponse, jobKey, originalID string) (*civ1.
 		return nil, &retryableJobError{msg: fmt.Sprintf("run %s has no jobs yet", resp.RunId)}
 	}
 
-	// Match by job key (--job flag).
+	// Match by job key (--job flag), preferring exact > suffix > segment matches.
 	if jobKey != "" {
+		bestTier := 0
+		tierMatches := map[int][]*civ1.JobStatus{}
 		for _, j := range allJobs {
-			if j.JobKey == jobKey {
-				return j, nil
+			if tier := matchJobKey(j.JobKey, jobKey); tier > 0 {
+				tierMatches[tier] = append(tierMatches[tier], j)
+				if bestTier == 0 || tier < bestTier {
+					bestTier = tier
+				}
 			}
 		}
-		// Inline workflows get prefixed keys (e.g. "_inline_0.yaml:lint_typecheck"),
-		// so fall back to a suffix match when the user passes just the job name.
-		for _, j := range allJobs {
-			if strings.HasSuffix(j.JobKey, ":"+jobKey) {
-				return j, nil
+		matches := tierMatches[bestTier]
+		if len(matches) == 0 {
+			// Job might not exist yet if workflows are still being expanded.
+			return nil, &retryableJobError{msg: fmt.Sprintf("job %q not found yet", jobKey)}
+		}
+		if len(matches) > 1 {
+			keys := make([]string, len(matches))
+			for i, j := range matches {
+				keys[i] = j.JobKey
 			}
+			fmt.Fprintf(os.Stderr, "Note: %q matched multiple jobs (%s), using %s\n", jobKey, strings.Join(keys, ", "), matches[0].JobKey)
 		}
-		// Job might not exist yet if workflows are still being expanded.
-		return nil, &retryableJobError{msg: fmt.Sprintf("job %q not found yet", jobKey)}
+		return matches[0], nil
 	}
 
 	// Match by job ID (user passed a job ID as the positional arg).
@@ -270,6 +279,28 @@ func workflowErrorMessage(resp *civ1.GetRunStatusResponse) string {
 	return ""
 }
 
+// matchJobKey returns the match tier of userKey against the full jobKey.
+// Returns 0 for no match. Lower non-zero values are higher priority:
+//
+//	1 = exact match ("build" == "build")
+//	2 = suffix match ("_inline_0.yaml:build" ends with ":build")
+//	3 = segment match ("pr.yaml:bazel:build" contains ":bazel:" as a segment)
+func matchJobKey(jobKey, userKey string) int {
+	if jobKey == userKey {
+		return 1
+	}
+	if strings.HasSuffix(jobKey, ":"+userKey) {
+		return 2
+	}
+	// Check if userKey appears as a complete colon-delimited segment anywhere
+	// in the key. Handles reusable workflow keys like "pr.yaml:bazel:build"
+	// where "bazel" is an intermediate segment.
+	if strings.Contains(":"+jobKey+":", ":"+userKey+":") {
+		return 3
+	}
+	return 0
+}
+
 func printSSHInfo(sandboxID, sessionID, output string) error {
 	if output == "json" {
 		enc := json.NewEncoder(os.Stdout)