fix(logs): resolve workflow display name when --repo is set and local lock files exist#33656
Conversation
…files exist When the MCP server sets GITHUB_REPOSITORY (causing --repo to be appended via appendRepoFlagFromEnv), the logs command was bypassing local workflow name resolution entirely and passing the raw workflow ID (e.g. "audit-workflows") directly to `gh run list --workflow`. Because GitHub Actions identifies workflows by their display name (e.g. "Agentic Workflow Audit Agent") rather than the workflow file's base name, this caused `gh run list` to fail with: could not find any workflows named audit-workflows The fix: when --repo is set, attempt FindWorkflowName resolution using local lock files first. If the local lock files are present (i.e. querying the current repo via GITHUB_REPOSITORY), the display name is resolved and passed to the API. If local resolution fails (genuinely cross-repo scenario with no local lock files), fall back to normalizeWorkflowID for the same behaviour as before. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes gh aw logs workflow filtering when --repo is set (notably in MCP server contexts where GITHUB_REPOSITORY auto-injects --repo) by attempting to resolve the workflow’s GitHub Actions display name from local lock files before falling back to normalized workflow IDs.
Changes:
- Update logs command workflow-name resolution to try
workflow.FindWorkflowNameeven when--repois set, with a fallback tonormalizeWorkflowIDon resolution failure. - Add a unit test covering the
--repo+ local lock file present case, and update existing test commentary for the remote/no-local-files path.
Show a summary per file
| File | Description |
|---|---|
| pkg/cli/logs_command.go | Attempts local display-name resolution under --repo to make gh run list --workflow succeed in MCP server scenarios. |
| pkg/cli/logs_command_test.go | Adds coverage for --repo with local lock files; refines existing test docs for the fallback behavior. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 2/2 changed files
- Comments generated: 2
| repoOverrideEarly, _ := cmd.Flags().GetString("repo") | ||
| if repoOverrideEarly != "" { | ||
| // When --repo is specified, bypass local file-based workflow name | ||
| // resolution. Normalize the input (strip .md/.lock.yml extensions) | ||
| // and use it directly as the workflow filter for the remote repo. | ||
| // When --repo is specified, try local file-based resolution first. | ||
| // This handles the common case where --repo refers to the current | ||
| // repository (e.g. set by GITHUB_REPOSITORY in MCP server containers), | ||
| // so local lock files are available and we can resolve the display name. | ||
| // | ||
| // Note: the argument must be a workflow ID (e.g. "test-claude"), | ||
| // not a display name (e.g. "Test Claude"). Display-name lookup | ||
| // requires local lock files, which are unavailable for remote repos. | ||
| workflowName = normalizeWorkflowID(args[0]) | ||
| logsCommandLog.Printf("Using normalized workflow name for remote repo: %s", workflowName) | ||
| // If local resolution fails (truly remote repo with no local lock files), | ||
| // fall back to using the normalized workflow ID directly. | ||
| if resolved, resolveErr := workflow.FindWorkflowName(args[0]); resolveErr == nil { | ||
| workflowName = resolved | ||
| logsCommandLog.Printf("Resolved workflow name for repo-overridden query: %s -> %s", args[0], workflowName) | ||
| } else { | ||
| workflowName = normalizeWorkflowID(args[0]) | ||
| logsCommandLog.Printf("Using normalized workflow name for remote repo: %s", workflowName) | ||
| } |
| tmpDir := t.TempDir() | ||
| workflowsDir := filepath.Join(tmpDir, ".github", "workflows") | ||
| require.NoError(t, os.MkdirAll(workflowsDir, 0755)) | ||
|
|
||
| // Create the markdown file (required by ResolveWorkflowName). | ||
| // The frontmatter name field is the GitHub Actions display name; the workflow | ||
| // ID is derived from the filename ("my-test-workflow"). | ||
| require.NoError(t, os.WriteFile(filepath.Join(workflowsDir, "my-test-workflow.md"), []byte("---\nname: My Test Workflow Display Name\n---\n"), 0644)) | ||
|
|
||
| // Create a lock file whose display name differs from the workflow ID. | ||
| // This simulates the real scenario where audit-workflows.lock.yml has | ||
| // name: "Agentic Workflow Audit Agent" while the ID is "audit-workflows". | ||
| lockContent := "name: \"My Test Workflow Display Name\"\non: push\n" | ||
| require.NoError(t, os.WriteFile(filepath.Join(workflowsDir, "my-test-workflow.lock.yml"), []byte(lockContent), 0644)) | ||
|
|
||
| t.Setenv("GH_AW_WORKFLOWS_DIR", workflowsDir) | ||
|
|
||
| cmd := NewLogsCommand() | ||
| cmd.SetArgs([]string{"my-test-workflow", "--repo", "owner/repo"}) | ||
| cmd.SetOut(nil) | ||
| cmd.SetErr(nil) | ||
|
|
|
@copilot review all comments |
Address review feedback: - Add repoIsLocal() helper that checks GITHUB_REPOSITORY env var first, then falls back to GetCurrentRepoSlug(). Local FindWorkflowName is only called when --repo is confirmed to be the current repository, preventing incorrect display-name mapping for cross-repo queries. - Fix test isolation: chdir into tmpDir, set GITHUB_REPOSITORY=owner/repo to match --repo, and pass --output to tmpDir so no files land in the repo root during the test. Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Both comments addressed in the latest commit:
|
The
logstool's--workflow-namefilter always failed in the MCP server context becauseGITHUB_REPOSITORYcauses--repoto be appended automatically, and the--repopath inlogs_command.gobypassedFindWorkflowNameentirely — passing the raw workflow ID (e.g.audit-workflows) directly togh run list --workflowinstead of the GitHub Actions display name (e.g."Agentic Workflow Audit Agent"from the lock file).Changes
pkg/cli/logs_command.go: When--repois set, attemptFindWorkflowNameresolution against local lock files first. Fall back tonormalizeWorkflowIDonly when local resolution fails (truly cross-repo with no local lock files).pkg/cli/logs_command_test.go: AddsTestLogsCommand_RepoUsesLocalResolutionWhenLockFileExiststo cover the case where local lock files are present and--repois set. The existingTestLogsCommand_RepoBypassesLocalWorkflowResolution(no local files) continues to validate the fallback path.