Skip to content
Closed
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
50 changes: 50 additions & 0 deletions docs/adr/59572-reuse-compatible-logs-json-run-records.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# ADR-59572: Reuse compatible logs JSON run records

**Date**: 2026-09-09
**Status**: Draft
**Deciders**: gh-aw maintainers

---

### Context

The `gh aw logs` command currently recomputes run records by downloading and processing artifacts even when equivalent `logs --json` output from an earlier run already exists. This PR adds a new `--cached-json` input, threads it through both standard and stdin-based log collection flows, and rebuilds report output from a mix of reused cached records and newly processed runs. The diff shows strict cache-safety checks around run identity, completion state, attempt, repository, conclusion, and update time, plus explicit fallbacks when requested filters require artifact-level evidence not preserved in compact JSON. The repository needs a documented decision on whether prior JSON output is an acceptable cache source for logs analysis and reporting.

### Decision

We will allow `gh aw logs` to reuse prior `--json` output as a cache source when a cached record can be proven compatible with the current workflow run and requested analysis mode. The implementation will only reuse completed runs with matching run ID, repository, attempt, conclusion, and update timestamp, and it will disable cached reuse for artifact-dependent filters or analysis modes that require richer evidence than compact JSON retains. When reuse is valid, the command will preserve cached run records in rebuilt JSON output and recompute aggregate totals from the combined cached and newly processed data.

### Alternatives Considered

#### Alternative 1: Always re-download and reprocess run artifacts

The command could continue treating every invocation as a fresh collection pass with no reuse of earlier JSON output. This was considered because it is the simplest behavior and avoids the risk of stale cached data. It was not chosen because the PR explicitly adds compatibility checks and tests to avoid unnecessary artifact work for unchanged completed runs.

#### Alternative 2: Reuse cached JSON for all runs and filter modes without validation

Another option would be to accept any previous logs JSON as authoritative and skip most per-run validation and mode checks. This was considered because it would maximize performance improvements and implementation simplicity. It was not chosen because the diff adds explicit guards for repository, attempt, conclusion, updated timestamp, engine filters, and artifact-dependent modes, showing that unvalidated reuse would be unsafe.

#### Alternative 3: Introduce a dedicated internal cache format instead of reusing prior JSON output

The project could have created a separate opaque cache artifact tailored specifically for reuse rather than depending on user-visible JSON output. This was considered because a dedicated format could carry richer evidence and fewer compatibility constraints. It was not chosen in this PR because the implementation intentionally reuses existing `logs --json` output, preserving current user workflows and avoiding an additional cache format.

### Consequences

#### Positive
- Re-running `gh aw logs` can avoid downloading and reprocessing artifacts for unchanged completed runs, reducing cost and latency.
- Cache reuse remains conservative because compatibility checks reject stale or insufficient cached records.
- Rebuilt reports can combine cached and fresh records while preserving existing JSON output structure.

#### Negative
- The logs pipeline becomes more complex because download, stdin, filtering, and aggregation paths must all account for cached records.
- Aggregate values may be approximate when compact cached JSON omits detailed evidence that richer artifact processing would have produced.
- Users must understand when `--cached-json` is ignored due to incompatible filters or analysis modes.

#### Neutral
- `LogsDownloadOptions`, `StdinLogsOptions`, and related orchestration types now carry a `CachedJSON` field through multiple entry points.
- Report aggregation now has a separate path for accumulating totals from cached `RunData` values.
- The feature relies on previous JSON output remaining parseable and structurally compatible with the current `LogsData` schema.

---

*ADR created by [adr-writer agent]. Review and finalize before changing status from Draft to Accepted.*
127 changes: 127 additions & 0 deletions pkg/cli/logs_cached_json.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
package cli

import (
"encoding/json"
"errors"
"fmt"
"os"
"strconv"
"strings"
)

type cachedLogsRuns map[int64]RunData

func loadCachedLogsJSON(path string) (cachedLogsRuns, error) {
if path == "" {
return nil, nil
}
data, err := os.ReadFile(path)
if err != nil {
return nil, fmt.Errorf("failed to read cached logs JSON: %w", err)
}
var logsData LogsData
if err := json.Unmarshal(data, &logsData); err != nil {
return nil, fmt.Errorf("failed to parse cached logs JSON: %w", err)
}
if logsData.Runs == nil {
return nil, errors.New("failed to parse cached logs JSON: missing runs array")
}
runs := make(cachedLogsRuns, len(logsData.Runs))
for _, run := range logsData.Runs {
if err := normalizeCachedLogRun(&run); err != nil {
return nil, err
}
if run.RunID != 0 {
runs[run.RunID] = run
}
}
logsCacheLog.Printf("Loaded %d run records from cached logs JSON", len(runs))
return runs, nil
}

func (runs cachedLogsRuns) lookup(run WorkflowRun, filters runFilterOpts) (RunData, bool) {
cached, ok := runs[run.DatabaseID]
if !ok {
return RunData{}, false
}
if cached.Status != "completed" || run.Status != "completed" || cached.Conclusion != run.Conclusion {
return RunData{}, false
}
if cached.RunAttempt == "" || run.Attempt <= 0 || cached.RunAttempt != strconv.Itoa(run.Attempt) {
return RunData{}, false
}
if cached.UpdatedAt.IsZero() || run.UpdatedAt.IsZero() || !cached.UpdatedAt.Equal(run.UpdatedAt) {
return RunData{}, false
}
if cached.Repository == "" || run.Repository == "" || !strings.EqualFold(cached.Repository, run.Repository) {
return RunData{}, false
}
if filters.engine != "" &&
!strings.EqualFold(filters.engine, cached.EngineID) &&
!strings.EqualFold(filters.engine, cached.Engine) &&
!strings.EqualFold(filters.engine, cached.Agent) {
return RunData{}, false
}
// Compact logs JSON does not retain enough per-run evidence to safely
// re-evaluate these artifact-dependent filters.
if filters.runtime != "" || filters.noStaged || filters.firewallOnly || filters.noFirewall ||
filters.safeOutputType != "" || filters.filteredIntegrity || filters.evalsOnly || filters.gradersOnly {
return RunData{}, false
}
return cached, true
}

func normalizeCachedLogRun(run *RunData) error {
if run.RunAttempt == "" {
return nil
}
attempt, err := strconv.Atoi(run.RunAttempt)
if err != nil || attempt <= 0 {
return fmt.Errorf("failed to parse cached logs JSON: invalid run_attempt for run %d", run.RunID)
}
run.RunAttempt = strconv.Itoa(attempt)
return nil
}

func cachedJSONCanSatisfy(artifactFilter []string, parse, audit, train, toolGraph bool) bool {

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.

pkg/cli/logs_cached_json.go:L74: yagni: dedicated wrapper adds an extra concept around a single predicate. Inline at callsite.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

No code change for this one. I kept cachedJSONCanSatisfy centralized because the same usage-only predicate guards cache loading in both download and stdin paths.

return isUsageOnlyArtifactFilter(artifactFilter) && !parse && !audit && !train && !toolGraph
}

func processedRunFromCachedData(data RunData) ProcessedRun {
return ProcessedRun{
Run: WorkflowRun{
DatabaseID: data.RunID,
Number: data.Number,
URL: data.URL,
Status: data.Status,
Conclusion: data.Conclusion,
WorkflowName: data.WorkflowName,
WorkflowPath: data.WorkflowPath,
CreatedAt: data.CreatedAt,
StartedAt: data.StartedAt,
UpdatedAt: data.UpdatedAt,
Event: data.Event,
HeadBranch: data.Branch,
HeadSha: data.HeadSHA,
DisplayTitle: data.DisplayTitle,
Repository: data.Repository,
Actor: data.Actor,
Duration: parseDurationString(data.Duration),
ActionMinutes: data.ActionMinutes,
TokenUsage: data.TokenUsage,
Turns: data.Turns,
ErrorCount: data.ErrorCount,
WarningCount: data.WarningCount,
MissingToolCount: data.MissingToolCount,
MissingDataCount: data.MissingDataCount,
SafeItemsCount: data.SafeItemsCount,
},

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.

pkg/cli/logs_cached_json.go:L107: yagni: pointer in only bypasses recomputation. Keep in and build output directly, no extra cross-type coupling.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 47882d5 by removing cached logs_path from ProcessedRun.Run. I kept cachedData because it preserves the original cached RunData for output without artifact reprocessing.

AwContext: data.AwContext,
TaskDomain: data.TaskDomain,
BehaviorFingerprint: data.BehaviorFingerprint,
AgenticAssessments: data.AgenticAssessments,
TokenUsage: data.TokenUsageSummary,
WorkingSet: data.WorkingSet,
cachedData: &data,
}
}
191 changes: 191 additions & 0 deletions pkg/cli/logs_cached_json_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,191 @@
//go:build !integration

package cli

import (
"context"
"encoding/json"
"os"
"path/filepath"
"testing"
"time"

"github.com/github/gh-aw/pkg/constants"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

func TestLoadCachedLogsJSON(t *testing.T) {
path := filepath.Join(t.TempDir(), "logs.json")
data, err := json.Marshal(LogsData{Runs: []RunData{
{RunID: 42, WorkflowName: "cached-workflow"},
{RunID: 0, WorkflowName: "invalid"},
}})
require.NoError(t, err)
require.NoError(t, os.WriteFile(path, data, 0o600))

runs, err := loadCachedLogsJSON(path)

require.NoError(t, err)
require.Len(t, runs, 1)
assert.Equal(t, "cached-workflow", runs[42].WorkflowName)
}

func TestLoadCachedLogsJSONRejectsInvalidInput(t *testing.T) {
path := filepath.Join(t.TempDir(), "logs.json")
require.NoError(t, os.WriteFile(path, []byte(`{"summary":{}}`), 0o600))

_, err := loadCachedLogsJSON(path)

require.ErrorContains(t, err, "missing runs array")
}

func TestLoadCachedLogsJSONRejectsInvalidRunAttempt(t *testing.T) {
path := filepath.Join(t.TempDir(), "logs.json")
require.NoError(t, os.WriteFile(path, []byte(`{"runs":[{"run_id":42,"run_attempt":"bogus"}]}`), 0o600))

_, err := loadCachedLogsJSON(path)

require.ErrorContains(t, err, "invalid run_attempt")
}

func TestCachedLogsLookupHonorsRepositoryAndFilters(t *testing.T) {
updatedAt := time.Date(2026, time.September, 1, 0, 0, 0, 0, time.UTC)
runs := cachedLogsRuns{
42: {RunID: 42, Repository: "github/gh-aw", EngineID: "copilot", Status: "completed", Conclusion: "success", RunAttempt: "1", UpdatedAt: updatedAt},
}
run := WorkflowRun{DatabaseID: 42, Repository: "github/gh-aw", Status: "completed", Conclusion: "success", Attempt: 1, UpdatedAt: updatedAt}

_, ok := runs.lookup(run, runFilterOpts{engine: "copilot"})
assert.True(t, ok)
_, ok = runs.lookup(run, runFilterOpts{engine: "claude"})
assert.False(t, ok)
_, ok = runs.lookup(run, runFilterOpts{runtime: "gvisor"})
assert.False(t, ok)
_, ok = runs.lookup(WorkflowRun{DatabaseID: 42, Repository: "other/repo", Status: "completed", Conclusion: "success", Attempt: 1, UpdatedAt: updatedAt}, runFilterOpts{})
assert.False(t, ok)
}

func TestCachedLogsLookupRejectsChangedRun(t *testing.T) {
updatedAt := time.Date(2026, time.September, 1, 0, 0, 0, 0, time.UTC)
runs := cachedLogsRuns{
42: {
RunID: 42,
Status: "completed",
Conclusion: "success",
RunAttempt: "1",
UpdatedAt: updatedAt,
},
}

tests := []WorkflowRun{
{DatabaseID: 42, Status: "in_progress", Conclusion: "", Attempt: 1, UpdatedAt: updatedAt},
{DatabaseID: 42, Status: "completed", Conclusion: "failure", Attempt: 1, UpdatedAt: updatedAt},
{DatabaseID: 42, Status: "completed", Conclusion: "success", Attempt: 2, UpdatedAt: updatedAt},
{DatabaseID: 42, Status: "completed", Conclusion: "success", Attempt: 1, UpdatedAt: updatedAt.Add(time.Minute)},
}
for _, run := range tests {
_, ok := runs.lookup(run, runFilterOpts{})
assert.False(t, ok)
}
}

func TestCachedLogsLookupRejectsUnknownIdentity(t *testing.T) {
updatedAt := time.Date(2026, time.September, 1, 0, 0, 0, 0, time.UTC)
runs := cachedLogsRuns{
42: {RunID: 42, Repository: "github/gh-aw", Status: "completed", Conclusion: "success", RunAttempt: "1", UpdatedAt: updatedAt},
43: {RunID: 43, Status: "completed", Conclusion: "success", UpdatedAt: updatedAt},
}

tests := []WorkflowRun{
{DatabaseID: 42, Repository: "github/gh-aw", Status: "completed", Conclusion: "success", UpdatedAt: updatedAt},
{DatabaseID: 42, Status: "completed", Conclusion: "success", Attempt: 1, UpdatedAt: updatedAt},
{DatabaseID: 43, Status: "completed", Conclusion: "success", UpdatedAt: updatedAt},
}
for _, run := range tests {
_, ok := runs.lookup(run, runFilterOpts{})
assert.False(t, ok)
}
}

func TestCachedJSONCanSatisfy(t *testing.T) {
usageFilter := []string{constants.UsageArtifactName.String()}
assert.True(t, cachedJSONCanSatisfy(usageFilter, false, false, false, false))
assert.False(t, cachedJSONCanSatisfy(nil, false, false, false, false))
assert.False(t, cachedJSONCanSatisfy(usageFilter, true, false, false, false))
assert.False(t, cachedJSONCanSatisfy(usageFilter, false, true, false, false))
assert.False(t, cachedJSONCanSatisfy(usageFilter, false, false, true, false))
assert.False(t, cachedJSONCanSatisfy(usageFilter, false, false, false, true))
}

func TestDownloadRunArtifactsConcurrentReusesCachedJSONRecord(t *testing.T) {
cached := RunData{
RunID: 42,
WorkflowName: "cached-workflow",
Repository: "github/gh-aw",
Status: "completed",
Conclusion: "success",
RunAttempt: "1",
UpdatedAt: time.Date(2026, time.September, 1, 0, 0, 0, 0, time.UTC),
LogsPath: "/previous/run-42",
}

results := downloadRunArtifactsConcurrent(context.Background(), []WorkflowRun{{DatabaseID: 42, Repository: "github/gh-aw", Status: "completed", Conclusion: "success", Attempt: 1, UpdatedAt: cached.UpdatedAt}}, runArtifactsConcurrentOptions{
outputDir: t.TempDir(),
maxRuns: 1,
cachedRuns: cachedLogsRuns{42: cached},
storageLimit: newLogsStorageLimit(t.TempDir(), 0, false),
})

require.Len(t, results, 1)
require.NotNil(t, results[0].CachedRun)
assert.True(t, results[0].Cached)
assert.Equal(t, cached, *results[0].CachedRun)
}

func TestBuildLogsDataPreservesCachedRunRecord(t *testing.T) {
cached := RunData{
RunID: 42,
WorkflowName: "cached-workflow",
Status: "completed",
Conclusion: "success",
Duration: "2m0s",
TokenUsageSummary: &TokenUsageSummary{TotalSteeringEvents: 2},
TokenUsage: 1200,
Turns: 4,
ErrorCount: 1,
WarningCount: 2,
GitHubAPICalls: 3,
TemporaryIDMappings: 4,
ChainedTargetCount: 1,
ChainedFollowupActionCount: 2,
DelegatedTempTargetCount: 1,
TemporaryIDMapStatus: temporaryIDMapStatusMissing,
EngineID: "copilot",
CreatedAt: time.Date(2026, time.September, 1, 0, 0, 0, 0, time.UTC),
LogsPath: "/previous/run-42",
Classification: "normal",
IntentionalFailure: true,
}

processedRun := processedRunFromCachedData(cached)
assert.Empty(t, processedRun.Run.LogsPath)

data := buildLogsData([]ProcessedRun{processedRun}, t.TempDir(), nil)

require.Equal(t, []RunData{cached}, data.Runs)
assert.Equal(t, 1, data.Summary.TotalRuns)
assert.Equal(t, "2.0m", data.Summary.TotalDuration)
assert.Equal(t, 2, data.Summary.TotalSteeringEvents)
assert.Equal(t, 1200, data.Summary.TotalTokens)
assert.Equal(t, 4, data.Summary.TotalTurns)
assert.Equal(t, 3, data.Summary.TotalGitHubAPICalls)
assert.Equal(t, 1, data.Summary.RunsWithTemporaryIDChains)
assert.Equal(t, 1, data.Summary.RunsWithDelegatedTempTargets)
assert.Equal(t, 1, data.Summary.RunsWithMissingTemporaryIDMap)
assert.Equal(t, 4, data.Summary.TotalTemporaryIDMappings)
assert.Equal(t, 1, data.Summary.TotalChainedTargets)
assert.Equal(t, 2, data.Summary.TotalChainedFollowupActions)
assert.Equal(t, map[string]int{"copilot": 1}, data.Summary.EngineCounts)
assert.Equal(t, 1, data.Summary.IntentionalFailureRuns)
}
Loading
Loading