Skip to content

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

Merged
robstolarz merged 4 commits intomainfrom
rob/dep-4015-cli-ci-run-job-flag-regressions
Mar 31, 2026
Merged

fix: match --job flag against reusable workflow job keys#474
robstolarz merged 4 commits intomainfrom
rob/dep-4015-cli-ci-run-job-flag-regressions

Conversation

@robstolarz
Copy link
Copy Markdown
Contributor

@robstolarz robstolarz commented Mar 31, 2026

Summary

  • Reusable workflows produce multi-segment job keys like pr.yaml:bazel:build or _inline_0.yaml:bazel:build. The --job flag only tried exact and suffix matches, so --job=bazel would fail because the key ends with :build, not :bazel.
  • Introduces matchJobKey() with 3-tier priority (exact > suffix > segment) shared by findJob (ssh/run) and findLogsJob (logs). When multiple jobs match at the same tier, the first is auto-selected with a stderr warning.
  • Verified against real prod data (pr.yaml:bazel:build keys from Unkey runs) and a live e2e test with a reusable workflow (_inline_0.yaml:wrapper:reusable-job matched by --job=wrapper).

Test plan

  • Unit tests: TestMatchJobKey covers all tiers and partial-segment rejection
  • Unit tests: TestFindJob_SegmentMatch_* for ssh/run path
  • Unit tests: TestFindLogsJob_SegmentMatch_* for logs path
  • Unit tests: TestFindJob_SegmentMatch_Ambiguous verifies multi-match auto-select
  • Unit tests: priority tests confirm suffix is preferred over segment
  • E2e: ran ci run with a reusable workflow, confirmed ci logs --job wrapper matched _inline_0.yaml:wrapper:reusable-job
  • E2e: confirmed unpatched CLI fails with job "wrapper" not found

Closes DEP-4015

🤖 Generated with Claude Code


Note

Medium Risk
Changes job-selection logic for ci logs/ci ssh, which could alter which job is picked (or when an error is raised) for runs with overlapping job keys. Covered by new unit tests, but mis-selection would impact CLI UX/automation.

Overview
Fixes --job matching for reusable-workflow job keys (e.g. pr.yaml:bazel:build) by introducing shared tiered matching via matchJobKey (exact > suffix > segment) and using the best-tier matches in both findJob (ci ssh) and findLogsJob (ci logs).

Updates ambiguity handling: ci logs now distinguishes cross-workflow collisions (still requires --workflow) vs same-workflow multi-matches (asks for a more specific --job), and ci ssh warns to stderr when multiple jobs match and proceeds with the first.

Adds focused unit tests for the new match tiers, segment-matching scenarios (inline + reusable workflows), and priority/ambiguity behavior in both commands.

Written by Cursor Bugbot for commit d93cc99. This will update automatically on new commits. Configure here.

…low keys

Reusable workflows produce multi-segment job keys like
"pr.yaml:bazel:build" or "_inline_0.yaml:bazel:build", where the
user-provided name ("bazel") is an intermediate colon-delimited segment.
The existing matching only tried exact and suffix matches, so --job=bazel
would fail because the key ends with ":build", not ":bazel".

Introduces matchJobKey() with a 3-tier priority (exact > suffix >
segment) shared by findJob (ssh/run) and findLogsJob (logs). When
multiple jobs match at the same tier, the first is auto-selected with a
stderr warning.

Verified against a real CI run: _inline_0.yaml:wrapper:reusable-job now
correctly matches --job=wrapper.

Closes DEP-4015

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@linear
Copy link
Copy Markdown

linear bot commented Mar 31, 2026

Copy link
Copy Markdown

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

Bugbot Autofix prepared a fix for the issue found in the latest run.

  • ✅ Fixed: Misleading multi-match error message in logs path
    • Changed the default case in findLogsJob to auto-select the first match and print a warning to stderr (matching findJob behavior in ssh.go) instead of erroring with a misleading --workflow suggestion.

Create PR

Or push these changes by commenting:

@cursor push e5b2ea8ca4
Preview (e5b2ea8ca4)
diff --git a/pkg/cmd/ci/logs.go b/pkg/cmd/ci/logs.go
--- a/pkg/cmd/ci/logs.go
+++ b/pkg/cmd/ci/logs.go
@@ -243,12 +243,12 @@
 		case 1:
 			return matches[0].job, matches[0].workflowPath, nil
 		default:
-			// Same job key in multiple workflows — need --workflow.
-			var paths []string
-			for _, m := range matches {
-				paths = append(paths, m.workflowPath)
+			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, ", "))
+			fmt.Fprintf(os.Stderr, "Note: %q matched multiple jobs (%s), using %s\n", jobKey, strings.Join(keys, ", "), displayNames[matches[0].job.JobKey])
+			return matches[0].job, matches[0].workflowPath, nil
 		}
 	}
 

diff --git a/pkg/cmd/ci/logs_test.go b/pkg/cmd/ci/logs_test.go
--- a/pkg/cmd/ci/logs_test.go
+++ b/pkg/cmd/ci/logs_test.go
@@ -97,7 +97,7 @@
 	}
 }
 
-func TestFindLogsJob_DuplicateJobKeyRequiresWorkflow(t *testing.T) {
+func TestFindLogsJob_DuplicateJobKeySelectsFirst(t *testing.T) {
 	resp := &civ1.GetRunStatusResponse{
 		RunId: "run-1",
 		Workflows: []*civ1.WorkflowStatus{
@@ -116,10 +116,16 @@
 		},
 	}
 
-	_, _, err := findLogsJob(resp, "run-1", "build", "")
-	if err == nil {
-		t.Fatal("expected error for duplicate job key without --workflow")
+	job, path, err := findLogsJob(resp, "run-1", "build", "")
+	if err != nil {
+		t.Fatal(err)
 	}
+	if job.JobId != "job-1" {
+		t.Fatalf("expected job ID %q, got %q", "job-1", job.JobId)
+	}
+	if path != ".depot/workflows/ci.yml" {
+		t.Fatalf("expected workflow path %q, got %q", ".depot/workflows/ci.yml", path)
+	}
 }
 
 func TestFindLogsJob_DuplicateJobKeyWithWorkflowFilter(t *testing.T) {
@@ -244,7 +250,7 @@
 	}
 }
 
-func TestFindLogsJob_SuffixMatchAmbiguous(t *testing.T) {
+func TestFindLogsJob_SuffixMatchAmbiguousSelectsFirst(t *testing.T) {
 	resp := &civ1.GetRunStatusResponse{
 		RunId: "run-1",
 		Workflows: []*civ1.WorkflowStatus{
@@ -263,10 +269,16 @@
 		},
 	}
 
-	_, _, err := findLogsJob(resp, "run-1", "build", "")
-	if err == nil {
-		t.Fatal("expected error for ambiguous suffix match across workflows")
+	job, path, err := findLogsJob(resp, "run-1", "build", "")
+	if err != nil {
+		t.Fatal(err)
 	}
+	if job.JobId != "job-1" {
+		t.Fatalf("expected job ID %q, got %q", "job-1", job.JobId)
+	}
+	if path != ".depot/workflows/ci.yml" {
+		t.Fatalf("expected workflow path %q, got %q", ".depot/workflows/ci.yml", path)
+	}
 }
 
 func TestJobDisplayNames_UniqueShortNames(t *testing.T) {

You can send follow-ups to this agent here.

Comment thread pkg/cmd/ci/logs.go
robstolarz and others added 2 commits March 31, 2026 10:15
When segment matching produces multiple matches within the same workflow
(e.g., --job=backend matching both backend:build and backend:test), the
error now suggests a more specific --job value instead of --workflow.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Comment thread pkg/cmd/ci/logs.go Outdated
Comment on lines +252 to +256
var paths []string
for _, m := range matches {
paths = append(paths, m.workflowPath)
}
return nil, "", fmt.Errorf("job %q exists in multiple workflows, specify one with --workflow: %s", jobKey, strings.Join(paths, ", "))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Duplicate workflow paths in error message. When multiple jobs match across workflows, the paths slice is built by iterating through all matches, which can contain multiple jobs from the same workflow. This results in duplicate workflow names in the error message.

Example: If 3 jobs match (2 from "workflow1", 1 from "workflow2"), the error shows: "job X exists in multiple workflows, specify one with --workflow: workflow1, workflow1, workflow2"

Fix: Build paths from the uniquePaths map keys instead:

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, ", "))
}
Suggested change
var paths []string
for _, m := range matches {
paths = append(paths, m.workflowPath)
}
return nil, "", fmt.Errorf("job %q exists in multiple workflows, specify one with --workflow: %s", jobKey, strings.Join(paths, ", "))
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, ", "))

Spotted by Graphite

Fix in Graphite


Is this helpful? React 👍 or 👎 to let us know.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@robstolarz robstolarz merged commit 3d6e090 into main Mar 31, 2026
11 checks passed
@robstolarz robstolarz deleted the rob/dep-4015-cli-ci-run-job-flag-regressions branch March 31, 2026 19:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants