Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 55 additions & 12 deletions pkg/cli/logs_github_api.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"slices"
"strconv"
"strings"
"time"

"github.com/github/gh-aw/pkg/console"
"github.com/github/gh-aw/pkg/logger"
Expand All @@ -26,6 +27,50 @@ import (

var logsGitHubAPILog = logger.New("cli:logs_github_api")

// buildCreatedFilter constructs a single --created filter value from the provided date
// bounds. Using a single --created flag is required because gh run list treats --created
// as a single string flag; supplying it multiple times only keeps the last value, silently
// discarding earlier bounds (see https://github.com/cli/cli/blob/trunk/pkg/cmd/run/list/list.go).
//
// When both a lower bound (startDate) and an upper bound are present the function uses
// GitHub's range syntax ("start..end") so that both bounds are enforced in one expression.
//
// beforeDate is an exclusive upper bound used for cursor-based pagination. Because the
// range syntax is inclusive on both ends, one second is subtracted from beforeDate so that
// the run at the cursor position is not returned again on the next page.
func buildCreatedFilter(startDate, endDate, beforeDate string) string {
// Determine the effective inclusive upper bound.
var upper string
if beforeDate != "" {
// beforeDate is exclusive (< beforeDate); convert to inclusive by subtracting 1 s.
t, err := time.Parse(time.RFC3339, beforeDate)
if err == nil {
upper = t.Add(-time.Second).Format(time.RFC3339)
} else {
// Unparseable beforeDate: use it as-is and treat as inclusive best-effort.
// Log a warning so the caller knows the exact exclusive bound may be missed.
logsGitHubAPILog.Printf("buildCreatedFilter: could not parse beforeDate %q as RFC3339, using as-is: %v", beforeDate, err)
upper = beforeDate
}
} else if endDate != "" {
upper = endDate
}

switch {
case startDate != "" && upper != "":
return startDate + ".." + upper
case startDate != "":
return ">=" + startDate
case beforeDate != "":
// No startDate, but we have a pagination cursor: keep the original < form.
return "<" + beforeDate
case endDate != "":
return "<=" + endDate
default:
return ""
}
}

// fetchJobStatuses gets job information for a workflow run and counts failed jobs
func fetchJobStatuses(runID int64, verbose bool) (int, error) {
logsGitHubAPILog.Printf("Fetching job statuses: runID=%d", runID)
Expand Down Expand Up @@ -123,9 +168,9 @@ func fetchJobDetails(runID int64, verbose bool) ([]JobInfoWithDuration, error) {
type ListWorkflowRunsOptions struct {
WorkflowName string // filter by specific workflow (if empty, fetches all agentic workflows)
Limit int // maximum number of runs to fetch in this API call (batch size)
StartDate string // filter by creation date (>=)
EndDate string // filter by creation date (<=)
BeforeDate string // used for pagination (fetch runs created before this date)
StartDate string // filter by creation date (>=); combined with EndDate/BeforeDate into a single --created range
EndDate string // filter by creation date (<=); combined with StartDate into a single --created range
BeforeDate string // exclusive upper bound used for pagination (<); combined with StartDate into a single --created range
Ref string // filter by branch or tag name
BeforeRunID int64 // filter by run database ID (< this ID)
AfterRunID int64 // filter by run database ID (> this ID)
Expand Down Expand Up @@ -163,15 +208,13 @@ func listWorkflowRunsWithPagination(opts ListWorkflowRunsOptions) ([]WorkflowRun
if opts.Limit > 0 {
args = append(args, "--limit", strconv.Itoa(opts.Limit))
}
if opts.StartDate != "" {
args = append(args, "--created", ">="+opts.StartDate)
}
if opts.EndDate != "" {
args = append(args, "--created", "<="+opts.EndDate)
}
// Add beforeDate filter for pagination
if opts.BeforeDate != "" {
args = append(args, "--created", "<"+opts.BeforeDate)
// Build a single --created filter that covers the full date range.
// gh run list's --created flag is a single string (not a slice); passing it
// multiple times only keeps the last value and silently drops earlier bounds.
// buildCreatedFilter combines all bounds into one expression so that every
// bound is honoured.
if createdFilter := buildCreatedFilter(opts.StartDate, opts.EndDate, opts.BeforeDate); createdFilter != "" {
args = append(args, "--created", createdFilter)
}
// Add ref filter (uses --branch flag which also works for tags)
if opts.Ref != "" {
Expand Down
91 changes: 91 additions & 0 deletions pkg/cli/logs_github_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,97 @@ func TestWorkflowRunUnmarshal(t *testing.T) {
assert.Empty(t, runs[0].WorkflowPath, "WorkflowPath should be empty when 'path' field is absent")
}

// TestBuildCreatedFilter verifies that buildCreatedFilter always produces a single
// --created expression that enforces all supplied date bounds. The key invariant is that
// StartDate is never silently dropped, which was the root cause of the bug where runs
// outside the requested window were returned (multiple --created flags were used but gh
// CLI only honours the last one).
func TestBuildCreatedFilter(t *testing.T) {
tests := []struct {
name string
startDate string
endDate string
beforeDate string
want string
}{
{
name: "no bounds",
want: "",
},
{
name: "start date only",
startDate: "2026-04-17",
want: ">=2026-04-17",
},
{
name: "end date only",
endDate: "2026-04-17",
want: "<=2026-04-17",
},
{
name: "start and end date",
startDate: "2026-04-17",
endDate: "2026-04-17",
want: "2026-04-17..2026-04-17",
},
{
name: "start and end date different days",
startDate: "2026-04-01",
endDate: "2026-04-30",
want: "2026-04-01..2026-04-30",
},
{
name: "before date only (pagination cursor)",
beforeDate: "2026-04-17T12:00:00Z",
// No startDate: keep the original < form.
want: "<2026-04-17T12:00:00Z",
},
{
name: "start date and before date (pagination with lower bound)",
startDate: "2026-04-01",
beforeDate: "2026-04-17T12:00:01Z",
// beforeDate is exclusive; subtract 1 s for inclusive range syntax.
want: "2026-04-01..2026-04-17T12:00:00Z",
},
{
name: "start date, end date, and before date",
startDate: "2026-04-01",
endDate: "2026-04-30",
beforeDate: "2026-04-17T12:00:01Z",
// beforeDate takes precedence over endDate as the pagination upper bound.
want: "2026-04-01..2026-04-17T12:00:00Z",
},
{
name: "before date at second boundary",
startDate: "2026-04-17T00:00:00Z",
beforeDate: "2026-04-17T00:00:01Z",
// Subtracting 1 s from beforeDate gives exactly startDate.
want: "2026-04-17T00:00:00Z..2026-04-17T00:00:00Z",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := buildCreatedFilter(tt.startDate, tt.endDate, tt.beforeDate)
assert.Equal(t, tt.want, got)
})
}
}

// TestBuildCreatedFilterStartDateAlwaysEnforced verifies that when both startDate and
// beforeDate are set, the returned filter contains the startDate so that the lower bound
// is always honoured. This is the regression test for the original bug.
func TestBuildCreatedFilterStartDateAlwaysEnforced(t *testing.T) {
startDate := "2026-04-17"
beforeDate := "2026-04-17T23:59:59Z"

filter := buildCreatedFilter(startDate, "", beforeDate)

// The filter must start with the startDate so it is part of the expression sent to gh.
assert.True(t, strings.HasPrefix(filter, startDate),
"filter %q must start with startDate %q so the lower bound is enforced", filter, startDate)
}

// TestListWorkflowRunsErrorHandling verifies the error classification logic in
// listWorkflowRunsWithPagination. In particular it checks that:
// - "Unknown JSON field" (capital U, as emitted by gh CLI) is treated as an
Expand Down