Merge branch 'main' into copilot/send-telemetry-data-file-handling

pelikhan · web-flow · commit c188cf827c71 · 2026-06-10T23:57:42.000-07:00
diff --git a/actions/setup/js/send_otlp_span.cjs b/actions/setup/js/send_otlp_span.cjs
@@ -161,6 +161,25 @@ function parseBooleanEnv(value) {
   return undefined;
 }
 
+/**
+ * Resolve the job name for conclusion spans.
+ *
+ * Normally this comes from INPUT_JOB_NAME (job-name action input), but some
+ * deployment paths can miss that env var in the post step. In that case, fall
+ * back to parsing the conclusion span name ("gh-aw.<job>.conclusion").
+ *
+ * @param {string} spanName
+ * @returns {string}
+ */
+function resolveConclusionJobName(spanName) {
+  const inputJobName = (process.env.INPUT_JOB_NAME || "").trim();
+  if (inputJobName) {
+    return inputJobName;
+  }
+  const match = /^gh-aw\.([^.]+)\.conclusion$/.exec(spanName);
+  return match ? match[1] : "";
+}
+
 /**
  * Parse setup-time aw_context passed via environment before aw_info.json exists.
  *
@@ -2005,7 +2024,7 @@ async function sendJobConclusionSpan(spanName, options = {}) {
   const awmgVersion = (typeof awInfo.awmg_version === "string" ? awInfo.awmg_version : "") || process.env.GH_AW_INFO_AWMG_VERSION || "";
   const bodyModified = typeof awInfo.body_modified === "boolean" ? awInfo.body_modified : parseBooleanEnv(process.env.GH_AW_INFO_BODY_MODIFIED);
   const trackerId = process.env.GH_AW_TRACKER_ID || awInfo.tracker_id || "";
-  const jobName = process.env.INPUT_JOB_NAME || "";
+  const jobName = resolveConclusionJobName(spanName);
   const jobEmitsOwnTokenUsage = jobName === "agent" || jobName === "detection" || (!!engineId && jobName === engineId);
   const runId = process.env.GITHUB_RUN_ID || "";
   const runAttempt = awInfo.run_attempt || process.env.GITHUB_RUN_ATTEMPT || "1";
diff --git a/actions/setup/js/send_otlp_span.test.cjs b/actions/setup/js/send_otlp_span.test.cjs
@@ -3564,6 +3564,23 @@ describe("sendJobConclusionSpan", () => {
     expect(aicAttr.value.doubleValue).toBe(0.125);
   });
 
+  it("includes gh-aw.aic when INPUT_JOB_NAME is missing but span name is gh-aw.agent.conclusion", async () => {
+    const mockFetch = vi.fn().mockResolvedValue({ ok: true, status: 200, statusText: "OK" });
+    vi.stubGlobal("fetch", mockFetch);
+
+    process.env.GH_AW_OTLP_ENDPOINTS = JSON.stringify([{ url: "https://traces.example.com" }]);
+    process.env.GH_AW_AIC = "0.125";
+    delete process.env.INPUT_JOB_NAME;
+
+    await sendJobConclusionSpan("gh-aw.agent.conclusion");
+
+    const body = JSON.parse(mockFetch.mock.calls[0][1].body);
+    const span = body.resourceSpans[0].scopeSpans[0].spans[0];
+    const aicAttr = span.attributes.find(a => a.key === "gh-aw.aic");
+    expect(aicAttr).toBeDefined();
+    expect(aicAttr.value.doubleValue).toBe(0.125);
+  });
+
   it("emits dashboard metrics and aliases on the conclusion span", async () => {
     const mockFetch = vi.fn().mockResolvedValue({ ok: true, status: 200, statusText: "OK" });
     vi.stubGlobal("fetch", mockFetch);
diff --git a/actions/setup/js/write_large_content_to_file.cjs b/actions/setup/js/write_large_content_to_file.cjs
@@ -15,10 +15,7 @@ const { generateCompactSchema } = require("./generate_compact_schema.cjs");
 function writeLargeContentToFile(content) {
   const logsDir = "/tmp/gh-aw/safeoutputs";
 
-  // Ensure directory exists
-  if (!fs.existsSync(logsDir)) {
-    fs.mkdirSync(logsDir, { recursive: true });
-  }
+  fs.mkdirSync(logsDir, { recursive: true });
 
   // Generate SHA256 hash of content
   const hash = crypto.createHash("sha256").update(content).digest("hex");
@@ -27,16 +24,11 @@ function writeLargeContentToFile(content) {
   const filename = `${hash}.json`;
   const filepath = path.join(logsDir, filename);
 
-  // Write content to file
   fs.writeFileSync(filepath, content, "utf8");
 
-  // Generate compact schema description for jq/agent
   const description = generateCompactSchema(content);
 
-  return {
-    filename: filename,
-    description: description,
-  };
+  return { filename, description };
 }
 
 module.exports = {
diff --git a/actions/setup/js/write_large_content_to_file.test.cjs b/actions/setup/js/write_large_content_to_file.test.cjs
@@ -114,4 +114,61 @@ describe("writeLargeContentToFile", () => {
     const written = fs.readFileSync(filepath, "utf8");
     expect(written).toBe(content);
   });
+
+  it("should handle non-JSON content gracefully", async () => {
+    const { writeLargeContentToFile } = await import("./write_large_content_to_file.cjs");
+
+    const content = "this is plain text, not JSON";
+    const result = writeLargeContentToFile(content);
+
+    expect(result.description).toBe("text content");
+    const filepath = path.join(testDir, result.filename);
+    expect(fs.readFileSync(filepath, "utf8")).toBe(content);
+  });
+
+  it("should handle empty object", async () => {
+    const { writeLargeContentToFile } = await import("./write_large_content_to_file.cjs");
+
+    const content = JSON.stringify({});
+    const result = writeLargeContentToFile(content);
+
+    expect(result.description).toBe("{}");
+  });
+
+  it("should handle empty array", async () => {
+    const { writeLargeContentToFile } = await import("./write_large_content_to_file.cjs");
+
+    const content = JSON.stringify([]);
+    const result = writeLargeContentToFile(content);
+
+    expect(result.description).toBe("[]");
+  });
+
+  it("should handle nested object (only top-level keys listed)", async () => {
+    const { writeLargeContentToFile } = await import("./write_large_content_to_file.cjs");
+
+    const content = JSON.stringify({ a: { b: 1 }, c: [1, 2] });
+    const result = writeLargeContentToFile(content);
+
+    expect(result.description).toBe("{a, c}");
+  });
+
+  it("should work when directory already exists", async () => {
+    const { writeLargeContentToFile } = await import("./write_large_content_to_file.cjs");
+
+    fs.mkdirSync(testDir, { recursive: true });
+    expect(fs.existsSync(testDir)).toBe(true);
+
+    const content = JSON.stringify({ already: "there" });
+    expect(() => writeLargeContentToFile(content)).not.toThrow();
+  });
+
+  it("should handle JSON primitive (number)", async () => {
+    const { writeLargeContentToFile } = await import("./write_large_content_to_file.cjs");
+
+    const content = JSON.stringify(42);
+    const result = writeLargeContentToFile(content);
+
+    expect(result.description).toBe("number");
+  });
 });
diff --git a/pkg/cli/outcome_domain_breakdown.go b/pkg/cli/outcome_domain_breakdown.go
@@ -1,8 +1,9 @@
 package cli
 
 import (
+	"cmp"
 	"fmt"
-	"sort"
+	"slices"
 	"strings"
 
 	"github.com/github/gh-aw/pkg/logger"
@@ -92,11 +93,11 @@ func ComputeDomainBreakdowns(reports []OutcomeReport) []DomainBreakdown {
 	}
 
 	// Sort by total_objective_value descending
-	sort.Slice(result, func(i, j int) bool {
-		if result[i].TotalObjectiveValue != result[j].TotalObjectiveValue {
-			return result[i].TotalObjectiveValue > result[j].TotalObjectiveValue
+	slices.SortFunc(result, func(a, b DomainBreakdown) int {
+		if a.TotalObjectiveValue != b.TotalObjectiveValue {
+			return cmp.Compare(b.TotalObjectiveValue, a.TotalObjectiveValue)
 		}
-		return result[i].Label < result[j].Label
+		return strings.Compare(a.Label, b.Label)
 	})
 
 	domainBreakdownLog.Printf("Computed domain breakdowns: domains=%d, total_attempted=%d", len(result), countTotalAttempted(result))
diff --git a/pkg/cli/outcome_domain_breakdown_test.go b/pkg/cli/outcome_domain_breakdown_test.go
@@ -0,0 +1,34 @@
+//go:build !integration
+
+package cli
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestComputeDomainBreakdowns_SortsByValueThenLabel(t *testing.T) {
+	reports := []OutcomeReport{
+		{
+			Result:          OutcomeAccepted,
+			ObjectiveValue:  20,
+			ObjectiveLabels: []string{"beta"},
+		},
+		{
+			Result:          OutcomeAccepted,
+			ObjectiveValue:  20,
+			ObjectiveLabels: []string{"alpha"},
+		},
+		{
+			Result:          OutcomeRejected,
+			ObjectiveLabels: []string{"gamma"},
+		},
+	}
+
+	breakdowns := ComputeDomainBreakdowns(reports)
+	require.Len(t, breakdowns, 3)
+	assert.Equal(t, []string{"alpha", "beta", "gamma"}, []string{breakdowns[0].Label, breakdowns[1].Label, breakdowns[2].Label})
+	assert.Equal(t, []int{20, 20, 0}, []int{breakdowns[0].TotalObjectiveValue, breakdowns[1].TotalObjectiveValue, breakdowns[2].TotalObjectiveValue})
+}
diff --git a/pkg/cli/outcomes_history.go b/pkg/cli/outcomes_history.go
@@ -1,10 +1,11 @@
 package cli
 
 import (
+	"cmp"
 	"encoding/json"
 	"fmt"
 	"os"
-	"sort"
+	"slices"
 	"strconv"
 	"strings"
 
@@ -246,32 +247,32 @@ func buildHistoricalObjectiveReport(source string, items []historicalGitHubItem,
 		})
 	}
 
-	sort.Slice(buckets, func(i, j int) bool {
-		if buckets[i].ContributedValue != buckets[j].ContributedValue {
-			return buckets[i].ContributedValue > buckets[j].ContributedValue
+	slices.SortFunc(buckets, func(a, b historicalObjectiveBucket) int {
+		if a.ContributedValue != b.ContributedValue {
+			return cmp.Compare(b.ContributedValue, a.ContributedValue)
 		}
-		if buckets[i].Count != buckets[j].Count {
-			return buckets[i].Count > buckets[j].Count
+		if a.Count != b.Count {
+			return cmp.Compare(b.Count, a.Count)
 		}
-		if buckets[i].MappedValue != buckets[j].MappedValue {
-			return buckets[i].MappedValue > buckets[j].MappedValue
+		if a.MappedValue != b.MappedValue {
+			return cmp.Compare(b.MappedValue, a.MappedValue)
 		}
-		return buckets[i].Label < buckets[j].Label
+		return strings.Compare(a.Label, b.Label)
 	})
 
-	sort.Slice(rows, func(i, j int) bool {
-		if rows[i].ObjectiveValue != rows[j].ObjectiveValue {
-			return rows[i].ObjectiveValue > rows[j].ObjectiveValue
+	slices.SortFunc(rows, func(a, b historicalObjectiveItem) int {
+		if a.ObjectiveValue != b.ObjectiveValue {
+			return cmp.Compare(b.ObjectiveValue, a.ObjectiveValue)
 		}
-		leftTime := rows[i].ClosedAt
+		leftTime := a.ClosedAt
 		if leftTime == "" {
-			leftTime = rows[i].MergedAt
+			leftTime = a.MergedAt
 		}
-		rightTime := rows[j].ClosedAt
+		rightTime := b.ClosedAt
 		if rightTime == "" {
-			rightTime = rows[j].MergedAt
+			rightTime = b.MergedAt
 		}
-		return leftTime < rightTime
+		return strings.Compare(leftTime, rightTime)
 	})
 
 	representative := make([]historicalObjectiveItem, 0, min(len(rows), 15))
