From 31569f171f7a9c8339a676fece93b8d31de694cf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Jul 2026 11:28:12 +0000 Subject: [PATCH 01/10] Initial plan From 3cd425f395897f3ff59efc41f6bd4f16791c67ed Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Jul 2026 11:53:16 +0000 Subject: [PATCH 02/10] Remove total_tokens from logs summary - always zero, misleading Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/logs_ci_scenario_test.go | 16 +++------------- pkg/cli/logs_empty_runs_test.go | 16 ++-------------- pkg/cli/logs_format_compact.go | 2 -- pkg/cli/logs_format_tsv.go | 10 +++++----- pkg/cli/logs_json_clean_test.go | 10 +++++----- pkg/cli/logs_json_test.go | 7 ------- pkg/cli/logs_report.go | 4 ---- pkg/cli/logs_report_test.go | 1 - pkg/cli/logs_summary_file_test.go | 4 ---- pkg/cli/mcp_schema_test.go | 1 - 10 files changed, 15 insertions(+), 56 deletions(-) diff --git a/pkg/cli/logs_ci_scenario_test.go b/pkg/cli/logs_ci_scenario_test.go index 6dd5aca0d49..701b5b986ff 100644 --- a/pkg/cli/logs_ci_scenario_test.go +++ b/pkg/cli/logs_ci_scenario_test.go @@ -78,8 +78,7 @@ func TestLogsJSONOutputWithNoRuns(t *testing.T) { t.Errorf("Expected TotalRuns to be 0, got %d", logsData.Summary.TotalRuns) } - // Most importantly: verify total_tokens field exists - // This is what the CI test checks with jq -e '.summary.total_tokens' + // Verify all expected summary fields exist var jsonMap map[string]any if err := json.Unmarshal([]byte(output), &jsonMap); err != nil { t.Fatalf("Failed to parse JSON as map: %v", err) @@ -90,14 +89,8 @@ func TestLogsJSONOutputWithNoRuns(t *testing.T) { t.Fatalf("Expected summary to be a map, got %T", jsonMap["summary"]) } - // This is the exact check the CI does - if _, exists := summary["total_tokens"]; !exists { - t.Errorf("Expected total_tokens field to exist in summary (CI test would fail). Summary: %+v", summary) - } - - // Verify all expected summary fields exist expectedFields := []string{ - "total_runs", "total_duration", "total_tokens", "total_cost", + "total_runs", "total_duration", "total_turns", "total_errors", "total_warnings", "total_missing_tools", "total_episodes", "high_confidence_episodes", } @@ -268,7 +261,7 @@ func TestLogsJSONOutputStructure(t *testing.T) { // Verify summary has all required fields summary := parsed["summary"].(map[string]any) requiredFields := []string{ - "total_runs", "total_duration", "total_tokens", + "total_runs", "total_duration", "total_turns", "total_errors", "total_warnings", "total_missing_tools", "total_episodes", "high_confidence_episodes", } @@ -340,7 +333,4 @@ func TestSummaryFileWrittenWithNoRuns(t *testing.T) { if parsed.Summary.TotalRuns != 0 { t.Errorf("Expected TotalRuns to be 0, got %d", parsed.Summary.TotalRuns) } - if parsed.Summary.TotalTokens != 0 { - t.Errorf("Expected TotalTokens to be 0, got %d", parsed.Summary.TotalTokens) - } } diff --git a/pkg/cli/logs_empty_runs_test.go b/pkg/cli/logs_empty_runs_test.go index 2d6345d2d6b..fbb6d7ecdac 100644 --- a/pkg/cli/logs_empty_runs_test.go +++ b/pkg/cli/logs_empty_runs_test.go @@ -22,9 +22,6 @@ func TestBuildLogsDataEmptyRuns(t *testing.T) { if logsData.Summary.TotalRuns != 0 { t.Errorf("Expected TotalRuns to be 0, got %d", logsData.Summary.TotalRuns) } - if logsData.Summary.TotalTokens != 0 { - t.Errorf("Expected TotalTokens to be 0, got %d", logsData.Summary.TotalTokens) - } // Verify runs array is empty if len(logsData.Runs) != 0 { @@ -68,23 +65,14 @@ func TestRenderLogsJSONEmptyRuns(t *testing.T) { if parsedData.Summary.TotalRuns != 0 { t.Errorf("Expected TotalRuns 0, got %d", parsedData.Summary.TotalRuns) } - if parsedData.Summary.TotalTokens != 0 { - t.Errorf("Expected TotalTokens 0, got %d", parsedData.Summary.TotalTokens) - } - // Verify the JSON contains the total_tokens field - // This is the key test - the field should be present even when zero + // Verify the JSON summary is valid var jsonMap map[string]any if err := json.Unmarshal([]byte(output), &jsonMap); err != nil { t.Fatalf("Failed to parse JSON as map: %v", err) } - summary, ok := jsonMap["summary"].(map[string]any) - if !ok { + if _, ok := jsonMap["summary"].(map[string]any); !ok { t.Fatalf("Expected summary to be a map, got %T", jsonMap["summary"]) } - - if _, exists := summary["total_tokens"]; !exists { - t.Errorf("Expected total_tokens field to exist in summary, but it was missing. Summary: %+v", summary) - } } diff --git a/pkg/cli/logs_format_compact.go b/pkg/cli/logs_format_compact.go index c88db1cc808..94aa9d2f36d 100644 --- a/pkg/cli/logs_format_compact.go +++ b/pkg/cli/logs_format_compact.go @@ -62,7 +62,6 @@ func renderLogsCompact(data LogsData) { summaryParts := []string{ "runs=" + strconv.Itoa(s.TotalRuns), "duration=" + s.TotalDuration, - "tokens=" + strconv.Itoa(s.TotalTokens), "turns=" + strconv.Itoa(s.TotalTurns), "errors=" + strconv.Itoa(s.TotalErrors), } @@ -229,7 +228,6 @@ func renderLogsCompactVerbose(data LogsData) { summaryParts := []string{ "runs=" + strconv.Itoa(s.TotalRuns), "duration=" + s.TotalDuration, - "tokens=" + strconv.Itoa(s.TotalTokens), "action_min=" + fmt.Sprintf("%.1f", s.TotalActionMinutes), "turns=" + strconv.Itoa(s.TotalTurns), "errors=" + strconv.Itoa(s.TotalErrors), diff --git a/pkg/cli/logs_format_tsv.go b/pkg/cli/logs_format_tsv.go index e73b96ccfc2..55ede67aba1 100644 --- a/pkg/cli/logs_format_tsv.go +++ b/pkg/cli/logs_format_tsv.go @@ -18,7 +18,7 @@ var logsTSVLog = logger.New("cli:logs_format_tsv") // // Output format: // -// Line 1: Summary line (total_runs, total_duration, total_tokens, total_errors) +// Line 1: Summary line (total_runs, total_duration, total_turns, total_errors) // Line 2: Column headers // Lines 3+: One line per run with tab-separated fields func renderLogsTSV(data LogsData) { @@ -26,8 +26,8 @@ func renderLogsTSV(data LogsData) { s := data.Summary // Summary line with key aggregates - fmt.Fprintf(os.Stdout, "# %d runs | %s duration | %d tokens | %d turns | %d errors\n", - s.TotalRuns, s.TotalDuration, s.TotalTokens, s.TotalTurns, s.TotalErrors) + fmt.Fprintf(os.Stdout, "# %d runs | %s duration | %d turns | %d errors\n", + s.TotalRuns, s.TotalDuration, s.TotalTurns, s.TotalErrors) if len(data.Runs) == 0 { return @@ -109,8 +109,8 @@ func renderLogsTSVVerbose(data LogsData) { logsTSVLog.Printf("Rendering %d runs as verbose TSV", data.Summary.TotalRuns) s := data.Summary - fmt.Fprintf(os.Stdout, "# %d runs | %s duration | %d tokens | %d turns | %d errors | %d missing_tools | %d github_api_calls\n", - s.TotalRuns, s.TotalDuration, s.TotalTokens, s.TotalTurns, s.TotalErrors, s.TotalMissingTools, s.TotalGitHubAPICalls) + fmt.Fprintf(os.Stdout, "# %d runs | %s duration | %d turns | %d errors | %d missing_tools | %d github_api_calls\n", + s.TotalRuns, s.TotalDuration, s.TotalTurns, s.TotalErrors, s.TotalMissingTools, s.TotalGitHubAPICalls) if len(data.Runs) == 0 { return diff --git a/pkg/cli/logs_json_clean_test.go b/pkg/cli/logs_json_clean_test.go index 73d3b80316a..534e6d6e37f 100644 --- a/pkg/cli/logs_json_clean_test.go +++ b/pkg/cli/logs_json_clean_test.go @@ -62,9 +62,9 @@ func TestJSONOutputNotCorruptedByStderr(t *testing.T) { t.Fatalf("Expected summary to be a map, got %T", jsonMap["summary"]) } - // This is what the CI test checks: jq -e '.summary.total_tokens' - if _, exists := summary["total_tokens"]; !exists { - t.Errorf("Expected total_tokens field in summary. Summary: %+v", summary) + // Verify the summary contains required fields + if _, exists := summary["total_runs"]; !exists { + t.Errorf("Expected total_runs field in summary. Summary: %+v", summary) } // Verify the output doesn't contain any stderr-like messages @@ -202,7 +202,7 @@ func TestStderrMessagesAfterJSON(t *testing.T) { t.Fatalf("Expected summary in JSON part, got %T", jsonMap["summary"]) } - if _, exists := summary["total_tokens"]; !exists { - t.Errorf("Expected total_tokens in JSON summary. Summary: %+v", summary) + if _, exists := summary["total_runs"]; !exists { + t.Errorf("Expected total_runs in JSON summary. Summary: %+v", summary) } } diff --git a/pkg/cli/logs_json_test.go b/pkg/cli/logs_json_test.go index fb612307646..913e0555c67 100644 --- a/pkg/cli/logs_json_test.go +++ b/pkg/cli/logs_json_test.go @@ -111,9 +111,6 @@ func TestBuildLogsData(t *testing.T) { if logsData.Summary.TotalRuns != 2 { t.Errorf("Expected TotalRuns to be 2, got %d", logsData.Summary.TotalRuns) } - if logsData.Summary.TotalTokens != 1500 { - t.Errorf("Expected TotalTokens to be 1500, got %d", logsData.Summary.TotalTokens) - } if logsData.Summary.TotalTurns != 5 { t.Errorf("Expected TotalTurns to be 5, got %d", logsData.Summary.TotalTurns) } @@ -195,7 +192,6 @@ func TestRenderLogsJSON(t *testing.T) { Summary: LogsSummary{ TotalRuns: 2, TotalDuration: "8m0s", - TotalTokens: 1500, TotalTurns: 5, TotalErrors: 1, TotalWarnings: 1, @@ -276,9 +272,6 @@ func TestRenderLogsJSON(t *testing.T) { if parsedData.Summary.TotalRuns != 2 { t.Errorf("Expected TotalRuns 2, got %d", parsedData.Summary.TotalRuns) } - if parsedData.Summary.TotalTokens != 1500 { - t.Errorf("Expected TotalTokens 1500, got %d", parsedData.Summary.TotalTokens) - } if parsedData.Summary.TotalEpisodes != 1 { t.Errorf("Expected TotalEpisodes 1, got %d", parsedData.Summary.TotalEpisodes) } diff --git a/pkg/cli/logs_report.go b/pkg/cli/logs_report.go index 71c98eaac69..54989a22f10 100644 --- a/pkg/cli/logs_report.go +++ b/pkg/cli/logs_report.go @@ -56,7 +56,6 @@ type ContinuationData struct { type LogsSummary struct { TotalRuns int `json:"total_runs" console:"header:Total Runs"` TotalDuration string `json:"total_duration" console:"header:Total Duration"` - TotalTokens int `json:"total_tokens" console:"header:Total Tokens,format:number"` TotalAIC float64 `json:"total_aic,omitempty"` TotalActionMinutes float64 `json:"total_action_minutes" console:"header:Total Action Minutes"` TotalTurns int `json:"total_turns" console:"header:Total Turns"` @@ -157,7 +156,6 @@ func buildLogsData(processedRuns []ProcessedRun, outputDir string, continuation // Build summary var totalDuration time.Duration - var totalTokens int var totalAIC float64 var totalActionMinutes float64 var totalTurns int @@ -191,7 +189,6 @@ func buildLogsData(processedRuns []ProcessedRun, outputDir string, continuation if run.Duration > 0 { totalDuration += run.Duration } - totalTokens += run.TokenUsage if pr.TokenUsage != nil { totalAIC += pr.TokenUsage.TotalAIC } @@ -347,7 +344,6 @@ func buildLogsData(processedRuns []ProcessedRun, outputDir string, continuation summary := LogsSummary{ TotalRuns: len(processedRuns), TotalDuration: timeutil.FormatDuration(totalDuration), - TotalTokens: totalTokens, TotalAIC: totalAIC, TotalActionMinutes: totalActionMinutes, TotalTurns: totalTurns, diff --git a/pkg/cli/logs_report_test.go b/pkg/cli/logs_report_test.go index 4346a21290a..18f4c4fc657 100644 --- a/pkg/cli/logs_report_test.go +++ b/pkg/cli/logs_report_test.go @@ -19,7 +19,6 @@ func TestRenderLogsConsoleUnified(t *testing.T) { Summary: LogsSummary{ TotalRuns: 2, TotalDuration: "10m30s", - TotalTokens: 2500, TotalTurns: 8, TotalErrors: 1, TotalWarnings: 3, diff --git a/pkg/cli/logs_summary_file_test.go b/pkg/cli/logs_summary_file_test.go index 6ea3d65fa2c..fc46f3612b9 100644 --- a/pkg/cli/logs_summary_file_test.go +++ b/pkg/cli/logs_summary_file_test.go @@ -21,7 +21,6 @@ func TestWriteSummaryFile(t *testing.T) { Summary: LogsSummary{ TotalRuns: 3, TotalDuration: "1h30m", - TotalTokens: 15000, TotalTurns: 25, TotalErrors: 2, TotalWarnings: 5, @@ -76,9 +75,6 @@ func TestWriteSummaryFile(t *testing.T) { if parsedData.Summary.TotalRuns != logsData.Summary.TotalRuns { t.Errorf("Expected TotalRuns %d, got %d", logsData.Summary.TotalRuns, parsedData.Summary.TotalRuns) } - if parsedData.Summary.TotalTokens != logsData.Summary.TotalTokens { - t.Errorf("Expected TotalTokens %d, got %d", logsData.Summary.TotalTokens, parsedData.Summary.TotalTokens) - } if len(parsedData.Runs) != len(logsData.Runs) { t.Errorf("Expected %d runs, got %d", len(logsData.Runs), len(parsedData.Runs)) } diff --git a/pkg/cli/mcp_schema_test.go b/pkg/cli/mcp_schema_test.go index 837498344c2..0ac7e1639c2 100644 --- a/pkg/cli/mcp_schema_test.go +++ b/pkg/cli/mcp_schema_test.go @@ -382,7 +382,6 @@ func TestGeneratedSchemasValidateRealOutput(t *testing.T) { Summary: LogsSummary{ TotalRuns: 5, TotalDuration: "10m30s", - TotalTokens: 15000, TotalTurns: 25, }, Runs: []RunData{ From 4107b58c46a0b6df2bbaf608885db2336978cd69 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Jul 2026 12:37:38 +0000 Subject: [PATCH 03/10] fix stale action pin fallback and logs assertions Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/cli/logs_empty_runs_test.go | 6 +++++- pkg/cli/logs_json_clean_test.go | 2 +- pkg/cli/logs_json_stderr_order_test.go | 18 +++++++++--------- pkg/workflow/action_pins.go | 16 ++++++++++++++-- pkg/workflow/action_pins_test.go | 21 +++++++++++++++++++++ 5 files changed, 50 insertions(+), 13 deletions(-) diff --git a/pkg/cli/logs_empty_runs_test.go b/pkg/cli/logs_empty_runs_test.go index fbb6d7ecdac..9c778c569b7 100644 --- a/pkg/cli/logs_empty_runs_test.go +++ b/pkg/cli/logs_empty_runs_test.go @@ -72,7 +72,11 @@ func TestRenderLogsJSONEmptyRuns(t *testing.T) { t.Fatalf("Failed to parse JSON as map: %v", err) } - if _, ok := jsonMap["summary"].(map[string]any); !ok { + summary, ok := jsonMap["summary"].(map[string]any) + if !ok { t.Fatalf("Expected summary to be a map, got %T", jsonMap["summary"]) } + if _, exists := summary["total_runs"]; !exists { + t.Fatalf("Expected total_runs field in summary. Summary: %+v", summary) + } } diff --git a/pkg/cli/logs_json_clean_test.go b/pkg/cli/logs_json_clean_test.go index 534e6d6e37f..f4f217d9a38 100644 --- a/pkg/cli/logs_json_clean_test.go +++ b/pkg/cli/logs_json_clean_test.go @@ -62,7 +62,7 @@ func TestJSONOutputNotCorruptedByStderr(t *testing.T) { t.Fatalf("Expected summary to be a map, got %T", jsonMap["summary"]) } - // Verify the summary contains required fields + // Verify the summary contains the stable total_runs field if _, exists := summary["total_runs"]; !exists { t.Errorf("Expected total_runs field in summary. Summary: %+v", summary) } diff --git a/pkg/cli/logs_json_stderr_order_test.go b/pkg/cli/logs_json_stderr_order_test.go index c6ade42d0a7..3d9ee39b179 100644 --- a/pkg/cli/logs_json_stderr_order_test.go +++ b/pkg/cli/logs_json_stderr_order_test.go @@ -91,7 +91,7 @@ func TestLogsJSONOutputBeforeStderr(t *testing.T) { t.Fatalf("Failed to re-marshal parsed JSON: %v", err) } - // Most importantly: verify total_tokens field exists in stdout JSON + // Most importantly: verify a stable summary field exists in stdout JSON var jsonMap map[string]any if err := json.Unmarshal([]byte(stdoutOutput), &jsonMap); err != nil { t.Fatalf("Failed to parse stdout JSON as map: %v", err) @@ -102,9 +102,9 @@ func TestLogsJSONOutputBeforeStderr(t *testing.T) { t.Fatalf("Expected summary to be a map in stdout JSON, got %T", jsonMap["summary"]) } - // This is the exact check the CI does with jq -e '.summary.total_tokens' - if _, exists := summary["total_tokens"]; !exists { - t.Errorf("Expected total_tokens field to exist in stdout JSON summary (CI test would fail). Summary: %+v", summary) + // This mirrors a jq existence check against a summary field that is always present. + if _, exists := summary["total_runs"]; !exists { + t.Errorf("Expected total_runs field to exist in stdout JSON summary. Summary: %+v", summary) } // Verify stderr contains the warning message (after JSON was output) @@ -218,7 +218,7 @@ func TestLogsJSONAndStderrRedirected(t *testing.T) { t.Logf("Warning: Had to extract JSON from mixed output. This suggests stderr messages may be interfering.") } - // Verify total_tokens exists + // Verify a stable summary field exists var jsonMap map[string]any if err := json.Unmarshal([]byte(output), &jsonMap); err == nil { // JSON parsed cleanly - this is good @@ -227,14 +227,14 @@ func TestLogsJSONAndStderrRedirected(t *testing.T) { t.Fatalf("Expected summary to be a map, got %T", jsonMap["summary"]) } - if _, exists := summary["total_tokens"]; !exists { - t.Errorf("Expected total_tokens field to exist in summary. Summary: %+v", summary) + if _, exists := summary["total_runs"]; !exists { + t.Errorf("Expected total_runs field to exist in summary. Summary: %+v", summary) } } else { // If the entire output isn't valid JSON, we have a problem // Try using jq-like parsing that CI uses - if !strings.Contains(output, `"total_tokens"`) { - t.Errorf("Expected to find 'total_tokens' field somewhere in output. Output: %s", output) + if !strings.Contains(output, `"total_runs"`) { + t.Errorf("Expected to find 'total_runs' field somewhere in output. Output: %s", output) } } } diff --git a/pkg/workflow/action_pins.go b/pkg/workflow/action_pins.go index bce817d975d..d99160ab764 100644 --- a/pkg/workflow/action_pins.go +++ b/pkg/workflow/action_pins.go @@ -6,6 +6,7 @@ import ( actionpins "github.com/github/gh-aw/pkg/actionpins" "github.com/github/gh-aw/pkg/logger" + "github.com/github/gh-aw/pkg/semverutil" ) var actionPinsLog = logger.New("workflow:action_pins") @@ -99,11 +100,22 @@ func (c *Compiler) getActionPin(repo string) string { } // Check the cache for any existing entry for this repo (regardless of version). - // Compiler-generated actions don't specify versions, so we use any cached entry we have. + // Compiler-generated actions don't specify versions, so prefer a cached entry only + // when it is at least as new as the latest embedded pin. cache := c.GetSharedActionCache() resolver := c.GetSharedActionResolver() + latestEmbedded, hasEmbedded := getLatestActionPinByRepo(repo) if cache != nil { if cacheKey, entry, found := cache.FindAnyEntryForRepo(repo); found { + if hasEmbedded { + cachedVersion := semverutil.ParseVersion(entry.Version) + embeddedVersion := semverutil.ParseVersion(latestEmbedded.Version) + if cachedVersion == nil || embeddedVersion == nil || semverutil.Compare(entry.Version, latestEmbedded.Version) < 0 { + actionPinsLog.Printf("Ignoring stale cache entry for compiler-generated action %s: cache=%s embedded=%s", + repo, entry.Version, latestEmbedded.Version) + return actionpins.FormatPinnedActionReference(repo, latestEmbedded.SHA, latestEmbedded.Version) + } + } // Mark this cache key as used so it won't be pruned as orphaned if resolver != nil { resolver.MarkCacheKeyAsUsed(cacheKey) @@ -112,7 +124,7 @@ func (c *Compiler) getActionPin(repo string) string { } } - // Fall back to embedded pins if no cache entry exists + // Fall back to embedded pins if no suitable cache entry exists return getActionPin(repo) } diff --git a/pkg/workflow/action_pins_test.go b/pkg/workflow/action_pins_test.go index 24c69755cee..7e1806baca5 100644 --- a/pkg/workflow/action_pins_test.go +++ b/pkg/workflow/action_pins_test.go @@ -1671,3 +1671,24 @@ func TestGHESArtifactCompatPinsExist(t *testing.T) { } } } + +func TestGetActionPinPrefersLatestEmbeddedOverStaleCache(t *testing.T) { + c := NewCompiler() + c.actionCache = &ActionCache{ + Entries: map[string]ActionCacheEntry{ + "actions/cache/restore@v4": { + Repo: "actions/cache/restore", + Version: "v4", + SHA: "0057852bfaa89a56745cba8c7296529d2fc39830", + }, + }, + } + c.actionResolver = NewActionResolver(c.actionCache) + + got := c.getActionPin("actions/cache/restore") + want := getActionPin("actions/cache/restore") + + if got != want { + t.Fatalf("expected compiler-generated action pin to prefer latest embedded pin %q, got %q", want, got) + } +} From 21f74f19c5dd2cbb9b87b81bdcd524fdca5efe31 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Jul 2026 12:41:19 +0000 Subject: [PATCH 04/10] clarify stale action pin version handling Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/action_pins.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pkg/workflow/action_pins.go b/pkg/workflow/action_pins.go index d99160ab764..ad7c95cf41c 100644 --- a/pkg/workflow/action_pins.go +++ b/pkg/workflow/action_pins.go @@ -110,7 +110,12 @@ func (c *Compiler) getActionPin(repo string) string { if hasEmbedded { cachedVersion := semverutil.ParseVersion(entry.Version) embeddedVersion := semverutil.ParseVersion(latestEmbedded.Version) - if cachedVersion == nil || embeddedVersion == nil || semverutil.Compare(entry.Version, latestEmbedded.Version) < 0 { + if cachedVersion == nil || embeddedVersion == nil { + actionPinsLog.Printf("Ignoring cache entry with unparseable version for compiler-generated action %s: cache=%s embedded=%s", + repo, entry.Version, latestEmbedded.Version) + return actionpins.FormatPinnedActionReference(repo, latestEmbedded.SHA, latestEmbedded.Version) + } + if embeddedVersion.IsNewer(cachedVersion) { actionPinsLog.Printf("Ignoring stale cache entry for compiler-generated action %s: cache=%s embedded=%s", repo, entry.Version, latestEmbedded.Version) return actionpins.FormatPinnedActionReference(repo, latestEmbedded.SHA, latestEmbedded.Version) From 56e894639d3eaec5ee9bf877955d72e33df060c6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Jul 2026 12:45:03 +0000 Subject: [PATCH 05/10] expand action pin determinism coverage Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/action_pins.go | 9 +++-- pkg/workflow/action_pins_test.go | 60 +++++++++++++++++++++++++++----- 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/pkg/workflow/action_pins.go b/pkg/workflow/action_pins.go index ad7c95cf41c..b15c202561a 100644 --- a/pkg/workflow/action_pins.go +++ b/pkg/workflow/action_pins.go @@ -110,8 +110,13 @@ func (c *Compiler) getActionPin(repo string) string { if hasEmbedded { cachedVersion := semverutil.ParseVersion(entry.Version) embeddedVersion := semverutil.ParseVersion(latestEmbedded.Version) - if cachedVersion == nil || embeddedVersion == nil { - actionPinsLog.Printf("Ignoring cache entry with unparseable version for compiler-generated action %s: cache=%s embedded=%s", + if cachedVersion == nil { + actionPinsLog.Printf("Ignoring cache entry with unparseable cached version for compiler-generated action %s: cache=%s embedded=%s", + repo, entry.Version, latestEmbedded.Version) + return actionpins.FormatPinnedActionReference(repo, latestEmbedded.SHA, latestEmbedded.Version) + } + if embeddedVersion == nil { + actionPinsLog.Printf("Ignoring embedded version with unparseable version for compiler-generated action %s: cache=%s embedded=%s", repo, entry.Version, latestEmbedded.Version) return actionpins.FormatPinnedActionReference(repo, latestEmbedded.SHA, latestEmbedded.Version) } diff --git a/pkg/workflow/action_pins_test.go b/pkg/workflow/action_pins_test.go index 7e1806baca5..12dff257226 100644 --- a/pkg/workflow/action_pins_test.go +++ b/pkg/workflow/action_pins_test.go @@ -1673,22 +1673,64 @@ func TestGHESArtifactCompatPinsExist(t *testing.T) { } func TestGetActionPinPrefersLatestEmbeddedOverStaleCache(t *testing.T) { - c := NewCompiler() - c.actionCache = &ActionCache{ - Entries: map[string]ActionCacheEntry{ - "actions/cache/restore@v4": { + tests := []struct { + name string + cacheEntry ActionCacheEntry + want string + }{ + { + name: "prefers newer embedded pin over stale cache", + cacheEntry: ActionCacheEntry{ Repo: "actions/cache/restore", Version: "v4", SHA: "0057852bfaa89a56745cba8c7296529d2fc39830", }, + want: getActionPin("actions/cache/restore"), + }, + { + name: "keeps equal cached version", + cacheEntry: ActionCacheEntry{ + Repo: "actions/cache/restore", + Version: "v6.1.0", + SHA: "55cc8345863c7cc4c66a329aec7e433d2d1c52a9", + }, + want: getActionPin("actions/cache/restore"), + }, + { + name: "keeps newer cached version", + cacheEntry: ActionCacheEntry{ + Repo: "actions/cache/restore", + Version: "v999.0.0", + SHA: "1111111111111111111111111111111111111111", + }, + want: "actions/cache/restore@1111111111111111111111111111111111111111 # v999.0.0", + }, + { + name: "falls back when cached version is unparseable", + cacheEntry: ActionCacheEntry{ + Repo: "actions/cache/restore", + Version: "not-a-version", + SHA: "2222222222222222222222222222222222222222", + }, + want: getActionPin("actions/cache/restore"), }, } - c.actionResolver = NewActionResolver(c.actionCache) - got := c.getActionPin("actions/cache/restore") - want := getActionPin("actions/cache/restore") + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := NewCompiler() + cacheKey := formatActionCacheKey(tt.cacheEntry.Repo, tt.cacheEntry.Version) + c.actionCache = &ActionCache{ + Entries: map[string]ActionCacheEntry{ + cacheKey: tt.cacheEntry, + }, + } + c.actionResolver = NewActionResolver(c.actionCache) - if got != want { - t.Fatalf("expected compiler-generated action pin to prefer latest embedded pin %q, got %q", want, got) + got := c.getActionPin(tt.cacheEntry.Repo) + if got != tt.want { + t.Fatalf("expected compiler-generated action pin %q, got %q", tt.want, got) + } + }) } } From 1bbbb96377caeee3f1b6310558c1273906a4cd12 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Jul 2026 12:48:37 +0000 Subject: [PATCH 06/10] fix embedded action pin parse fallback Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/action_pins.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pkg/workflow/action_pins.go b/pkg/workflow/action_pins.go index b15c202561a..6fdfb19a2c2 100644 --- a/pkg/workflow/action_pins.go +++ b/pkg/workflow/action_pins.go @@ -116,9 +116,12 @@ func (c *Compiler) getActionPin(repo string) string { return actionpins.FormatPinnedActionReference(repo, latestEmbedded.SHA, latestEmbedded.Version) } if embeddedVersion == nil { - actionPinsLog.Printf("Ignoring embedded version with unparseable version for compiler-generated action %s: cache=%s embedded=%s", + actionPinsLog.Printf("Using cached version for compiler-generated action %s because embedded version is unparseable: cache=%s embedded=%s", repo, entry.Version, latestEmbedded.Version) - return actionpins.FormatPinnedActionReference(repo, latestEmbedded.SHA, latestEmbedded.Version) + if resolver != nil { + resolver.MarkCacheKeyAsUsed(cacheKey) + } + return actionpins.FormatPinnedActionReference(repo, entry.SHA, entry.Version) } if embeddedVersion.IsNewer(cachedVersion) { actionPinsLog.Printf("Ignoring stale cache entry for compiler-generated action %s: cache=%s embedded=%s", From ddd6ce3f8ab975d2bb9ac535bcf6943125577ea6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Jul 2026 12:52:13 +0000 Subject: [PATCH 07/10] clarify cached action pin equality handling Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com> --- pkg/workflow/action_pins.go | 1 + pkg/workflow/action_pins_test.go | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/pkg/workflow/action_pins.go b/pkg/workflow/action_pins.go index 6fdfb19a2c2..74dc64b991d 100644 --- a/pkg/workflow/action_pins.go +++ b/pkg/workflow/action_pins.go @@ -128,6 +128,7 @@ func (c *Compiler) getActionPin(repo string) string { repo, entry.Version, latestEmbedded.Version) return actionpins.FormatPinnedActionReference(repo, latestEmbedded.SHA, latestEmbedded.Version) } + // Equal or newer cached versions intentionally fall through to the cache entry below. } // Mark this cache key as used so it won't be pruned as orphaned if resolver != nil { diff --git a/pkg/workflow/action_pins_test.go b/pkg/workflow/action_pins_test.go index 12dff257226..90cd365c495 100644 --- a/pkg/workflow/action_pins_test.go +++ b/pkg/workflow/action_pins_test.go @@ -1673,6 +1673,11 @@ func TestGHESArtifactCompatPinsExist(t *testing.T) { } func TestGetActionPinPrefersLatestEmbeddedOverStaleCache(t *testing.T) { + latestCacheRestorePin, ok := getLatestActionPinByRepo("actions/cache/restore") + if !ok { + t.Fatal("expected embedded pin for actions/cache/restore") + } + tests := []struct { name string cacheEntry ActionCacheEntry @@ -1691,8 +1696,8 @@ func TestGetActionPinPrefersLatestEmbeddedOverStaleCache(t *testing.T) { name: "keeps equal cached version", cacheEntry: ActionCacheEntry{ Repo: "actions/cache/restore", - Version: "v6.1.0", - SHA: "55cc8345863c7cc4c66a329aec7e433d2d1c52a9", + Version: latestCacheRestorePin.Version, + SHA: latestCacheRestorePin.SHA, }, want: getActionPin("actions/cache/restore"), }, From d16e86c2ec66bf68c37e7c8ec403851ae67b3ed6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Jul 2026 13:05:11 +0000 Subject: [PATCH 08/10] Restore token summary when available Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/logs_empty_runs_test.go | 3 ++ pkg/cli/logs_format_compact.go | 3 ++ pkg/cli/logs_format_tsv.go | 17 +++++--- pkg/cli/logs_format_tsv_test.go | 75 +++++++++++++++++++++++++++++++++ pkg/cli/logs_json_test.go | 3 ++ pkg/cli/logs_report.go | 4 ++ 6 files changed, 100 insertions(+), 5 deletions(-) create mode 100644 pkg/cli/logs_format_tsv_test.go diff --git a/pkg/cli/logs_empty_runs_test.go b/pkg/cli/logs_empty_runs_test.go index 9c778c569b7..fce953a2ce9 100644 --- a/pkg/cli/logs_empty_runs_test.go +++ b/pkg/cli/logs_empty_runs_test.go @@ -79,4 +79,7 @@ func TestRenderLogsJSONEmptyRuns(t *testing.T) { if _, exists := summary["total_runs"]; !exists { t.Fatalf("Expected total_runs field in summary. Summary: %+v", summary) } + if _, exists := summary["total_tokens"]; exists { + t.Fatalf("Expected total_tokens to be omitted when token data is unavailable. Summary: %+v", summary) + } } diff --git a/pkg/cli/logs_format_compact.go b/pkg/cli/logs_format_compact.go index 94aa9d2f36d..839c133501c 100644 --- a/pkg/cli/logs_format_compact.go +++ b/pkg/cli/logs_format_compact.go @@ -68,6 +68,9 @@ func renderLogsCompact(data LogsData) { if s.TotalAIC > 0 { summaryParts = append(summaryParts, "aic="+formatCompactAIC(s.TotalAIC)) } + if s.TotalTokens > 0 { + summaryParts = append(summaryParts, "tokens="+strconv.Itoa(s.TotalTokens)) + } if s.TotalWarnings > 0 { summaryParts = append(summaryParts, "warnings="+strconv.Itoa(s.TotalWarnings)) } diff --git a/pkg/cli/logs_format_tsv.go b/pkg/cli/logs_format_tsv.go index 55ede67aba1..3e61c07f046 100644 --- a/pkg/cli/logs_format_tsv.go +++ b/pkg/cli/logs_format_tsv.go @@ -12,13 +12,20 @@ import ( var logsTSVLog = logger.New("cli:logs_format_tsv") +func logsTSVSummaryTokens(totalTokens int) string { + if totalTokens == 0 { + return "n/a" + } + return strconv.Itoa(totalTokens) +} + // renderLogsTSV outputs the logs data as tab-separated values for maximum token efficiency. // This format is ~24x more compact than JSON, making it ideal for agentic consumption // where LLM context window tokens are the primary constraint. // // Output format: // -// Line 1: Summary line (total_runs, total_duration, total_turns, total_errors) +// Line 1: Summary line (total_runs, total_duration, total_tokens, total_turns, total_errors) // Line 2: Column headers // Lines 3+: One line per run with tab-separated fields func renderLogsTSV(data LogsData) { @@ -26,8 +33,8 @@ func renderLogsTSV(data LogsData) { s := data.Summary // Summary line with key aggregates - fmt.Fprintf(os.Stdout, "# %d runs | %s duration | %d turns | %d errors\n", - s.TotalRuns, s.TotalDuration, s.TotalTurns, s.TotalErrors) + fmt.Fprintf(os.Stdout, "# %d runs | %s duration | %s tokens | %d turns | %d errors\n", + s.TotalRuns, s.TotalDuration, logsTSVSummaryTokens(s.TotalTokens), s.TotalTurns, s.TotalErrors) if len(data.Runs) == 0 { return @@ -109,8 +116,8 @@ func renderLogsTSVVerbose(data LogsData) { logsTSVLog.Printf("Rendering %d runs as verbose TSV", data.Summary.TotalRuns) s := data.Summary - fmt.Fprintf(os.Stdout, "# %d runs | %s duration | %d turns | %d errors | %d missing_tools | %d github_api_calls\n", - s.TotalRuns, s.TotalDuration, s.TotalTurns, s.TotalErrors, s.TotalMissingTools, s.TotalGitHubAPICalls) + fmt.Fprintf(os.Stdout, "# %d runs | %s duration | %s tokens | %d turns | %d errors | %d missing_tools | %d github_api_calls\n", + s.TotalRuns, s.TotalDuration, logsTSVSummaryTokens(s.TotalTokens), s.TotalTurns, s.TotalErrors, s.TotalMissingTools, s.TotalGitHubAPICalls) if len(data.Runs) == 0 { return diff --git a/pkg/cli/logs_format_tsv_test.go b/pkg/cli/logs_format_tsv_test.go new file mode 100644 index 00000000000..893e7698959 --- /dev/null +++ b/pkg/cli/logs_format_tsv_test.go @@ -0,0 +1,75 @@ +//go:build !integration + +package cli + +import ( + "bytes" + "os" + "strings" + "testing" +) + +func captureStdout(t *testing.T, fn func()) string { + t.Helper() + + oldStdout := os.Stdout + r, w, err := os.Pipe() + if err != nil { + t.Fatalf("Failed to create stdout pipe: %v", err) + } + os.Stdout = w + + fn() + + _ = w.Close() + os.Stdout = oldStdout + + var buf bytes.Buffer + _, _ = buf.ReadFrom(r) + return buf.String() +} + +func TestRenderLogsTSVSummaryPreservesTokenField(t *testing.T) { + output := captureStdout(t, func() { + renderLogsTSV(LogsData{ + Summary: LogsSummary{ + TotalRuns: 2, + TotalDuration: "8m0s", + TotalTurns: 5, + TotalErrors: 1, + }, + }) + }) + + lines := strings.Split(strings.TrimSpace(output), "\n") + if len(lines) == 0 { + t.Fatal("Expected TSV output") + } + if got, want := lines[0], "# 2 runs | 8m0s duration | n/a tokens | 5 turns | 1 errors"; got != want { + t.Fatalf("Unexpected TSV summary line:\n got: %q\nwant: %q", got, want) + } +} + +func TestRenderLogsTSVVerboseSummaryPreservesTokenField(t *testing.T) { + output := captureStdout(t, func() { + renderLogsTSVVerbose(LogsData{ + Summary: LogsSummary{ + TotalRuns: 2, + TotalDuration: "8m0s", + TotalTokens: 1500, + TotalTurns: 5, + TotalErrors: 1, + TotalMissingTools: 2, + TotalGitHubAPICalls: 3, + }, + }) + }) + + lines := strings.Split(strings.TrimSpace(output), "\n") + if len(lines) == 0 { + t.Fatal("Expected TSV verbose output") + } + if got, want := lines[0], "# 2 runs | 8m0s duration | 1500 tokens | 5 turns | 1 errors | 2 missing_tools | 3 github_api_calls"; got != want { + t.Fatalf("Unexpected TSV verbose summary line:\n got: %q\nwant: %q", got, want) + } +} diff --git a/pkg/cli/logs_json_test.go b/pkg/cli/logs_json_test.go index 913e0555c67..e1368faa4be 100644 --- a/pkg/cli/logs_json_test.go +++ b/pkg/cli/logs_json_test.go @@ -111,6 +111,9 @@ func TestBuildLogsData(t *testing.T) { if logsData.Summary.TotalRuns != 2 { t.Errorf("Expected TotalRuns to be 2, got %d", logsData.Summary.TotalRuns) } + if logsData.Summary.TotalTokens != 1500 { + t.Errorf("Expected TotalTokens to be 1500, got %d", logsData.Summary.TotalTokens) + } if logsData.Summary.TotalTurns != 5 { t.Errorf("Expected TotalTurns to be 5, got %d", logsData.Summary.TotalTurns) } diff --git a/pkg/cli/logs_report.go b/pkg/cli/logs_report.go index 54989a22f10..98aabc5926b 100644 --- a/pkg/cli/logs_report.go +++ b/pkg/cli/logs_report.go @@ -57,6 +57,7 @@ type LogsSummary struct { TotalRuns int `json:"total_runs" console:"header:Total Runs"` TotalDuration string `json:"total_duration" console:"header:Total Duration"` TotalAIC float64 `json:"total_aic,omitempty"` + TotalTokens int `json:"total_tokens,omitempty" console:"header:Total Tokens,format:number,omitempty"` TotalActionMinutes float64 `json:"total_action_minutes" console:"header:Total Action Minutes"` TotalTurns int `json:"total_turns" console:"header:Total Turns"` TotalSteeringEvents int `json:"total_steering_events,omitempty" console:"header:Total Steering Events,format:number,omitempty"` @@ -157,6 +158,7 @@ func buildLogsData(processedRuns []ProcessedRun, outputDir string, continuation // Build summary var totalDuration time.Duration var totalAIC float64 + var totalTokens int var totalActionMinutes float64 var totalTurns int var totalSteeringEvents int @@ -192,6 +194,7 @@ func buildLogsData(processedRuns []ProcessedRun, outputDir string, continuation if pr.TokenUsage != nil { totalAIC += pr.TokenUsage.TotalAIC } + totalTokens += run.TokenUsage totalActionMinutes += run.ActionMinutes totalTurns += run.Turns if pr.TokenUsage != nil { @@ -345,6 +348,7 @@ func buildLogsData(processedRuns []ProcessedRun, outputDir string, continuation TotalRuns: len(processedRuns), TotalDuration: timeutil.FormatDuration(totalDuration), TotalAIC: totalAIC, + TotalTokens: totalTokens, TotalActionMinutes: totalActionMinutes, TotalTurns: totalTurns, TotalSteeringEvents: totalSteeringEvents, From 53fa70531a3c54dd67ffa3afcdf6455609c0b039 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Jul 2026 13:08:43 +0000 Subject: [PATCH 09/10] Reuse shared stdout capture in TSV tests Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/logs_format_tsv_test.go | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) diff --git a/pkg/cli/logs_format_tsv_test.go b/pkg/cli/logs_format_tsv_test.go index 893e7698959..e13ce2a9fea 100644 --- a/pkg/cli/logs_format_tsv_test.go +++ b/pkg/cli/logs_format_tsv_test.go @@ -3,34 +3,12 @@ package cli import ( - "bytes" - "os" "strings" "testing" ) -func captureStdout(t *testing.T, fn func()) string { - t.Helper() - - oldStdout := os.Stdout - r, w, err := os.Pipe() - if err != nil { - t.Fatalf("Failed to create stdout pipe: %v", err) - } - os.Stdout = w - - fn() - - _ = w.Close() - os.Stdout = oldStdout - - var buf bytes.Buffer - _, _ = buf.ReadFrom(r) - return buf.String() -} - func TestRenderLogsTSVSummaryPreservesTokenField(t *testing.T) { - output := captureStdout(t, func() { + output, _ := captureOutput(t, func() error { renderLogsTSV(LogsData{ Summary: LogsSummary{ TotalRuns: 2, @@ -39,6 +17,7 @@ func TestRenderLogsTSVSummaryPreservesTokenField(t *testing.T) { TotalErrors: 1, }, }) + return nil }) lines := strings.Split(strings.TrimSpace(output), "\n") @@ -51,7 +30,7 @@ func TestRenderLogsTSVSummaryPreservesTokenField(t *testing.T) { } func TestRenderLogsTSVVerboseSummaryPreservesTokenField(t *testing.T) { - output := captureStdout(t, func() { + output, _ := captureOutput(t, func() error { renderLogsTSVVerbose(LogsData{ Summary: LogsSummary{ TotalRuns: 2, @@ -63,6 +42,7 @@ func TestRenderLogsTSVVerboseSummaryPreservesTokenField(t *testing.T) { TotalGitHubAPICalls: 3, }, }) + return nil }) lines := strings.Split(strings.TrimSpace(output), "\n") From e70416f0ef13b6206b928867088878be53296d2c Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 1 Jul 2026 13:12:16 +0000 Subject: [PATCH 10/10] Clarify TSV token formatter naming Co-authored-by: gh-aw-bot <259018956+gh-aw-bot@users.noreply.github.com> --- pkg/cli/logs_format_tsv.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/cli/logs_format_tsv.go b/pkg/cli/logs_format_tsv.go index 3e61c07f046..20768dde1ce 100644 --- a/pkg/cli/logs_format_tsv.go +++ b/pkg/cli/logs_format_tsv.go @@ -12,7 +12,7 @@ import ( var logsTSVLog = logger.New("cli:logs_format_tsv") -func logsTSVSummaryTokens(totalTokens int) string { +func formatTSVSummaryTokens(totalTokens int) string { if totalTokens == 0 { return "n/a" } @@ -34,7 +34,7 @@ func renderLogsTSV(data LogsData) { s := data.Summary // Summary line with key aggregates fmt.Fprintf(os.Stdout, "# %d runs | %s duration | %s tokens | %d turns | %d errors\n", - s.TotalRuns, s.TotalDuration, logsTSVSummaryTokens(s.TotalTokens), s.TotalTurns, s.TotalErrors) + s.TotalRuns, s.TotalDuration, formatTSVSummaryTokens(s.TotalTokens), s.TotalTurns, s.TotalErrors) if len(data.Runs) == 0 { return @@ -117,7 +117,7 @@ func renderLogsTSVVerbose(data LogsData) { s := data.Summary fmt.Fprintf(os.Stdout, "# %d runs | %s duration | %s tokens | %d turns | %d errors | %d missing_tools | %d github_api_calls\n", - s.TotalRuns, s.TotalDuration, logsTSVSummaryTokens(s.TotalTokens), s.TotalTurns, s.TotalErrors, s.TotalMissingTools, s.TotalGitHubAPICalls) + s.TotalRuns, s.TotalDuration, formatTSVSummaryTokens(s.TotalTokens), s.TotalTurns, s.TotalErrors, s.TotalMissingTools, s.TotalGitHubAPICalls) if len(data.Runs) == 0 { return