Skip to content
Merged
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
7 changes: 5 additions & 2 deletions actions/setup/js/send_otlp_span.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1967,7 +1967,10 @@ async function sendJobConclusionSpan(spanName, options = {}) {
const detectionReason = process.env.GH_AW_DETECTION_REASON || "";
const runtimeMetrics = readAgentRuntimeMetrics();
// Read once and reuse for both gh-aw.aic and gen_ai.usage.* attributes.
const agentUsage = normalizeRuntimeTokenUsage(readJSONIfExists("/tmp/gh-aw/agent_usage.json")) || runtimeMetrics.tokenUsage || {};
const agentUsageFilePath = "/tmp/gh-aw/agent_usage.json";
const agentUsageRaw = readJSONIfExists(agentUsageFilePath);
const agentUsageNormalized = normalizeRuntimeTokenUsage(agentUsageRaw);
const agentUsage = agentUsageNormalized || runtimeMetrics.tokenUsage || {};
Comment thread
Copilot marked this conversation as resolved.
// Mark the span as an error when the agent job failed, timed out, or was cancelled.
const isAgentTimedOut = agentConclusion === "timed_out";
const isAgentFailure = agentConclusion === "failure" || isAgentTimedOut;
Expand Down Expand Up @@ -2072,7 +2075,7 @@ async function sendJobConclusionSpan(spanName, options = {}) {
const aiCreditsFromEnv = normalizeNonNegativeNumber(process.env.GH_AW_AIC);
const aiCreditsFromFile = agentUsage.ai_credits;
Comment thread
pelikhan marked this conversation as resolved.
const aiCreditsFromMetrics = runtimeMetrics.tokenUsage?.ai_credits;
const aiCredits = jobEmitsOwnTokenUsage ? (aiCreditsFromEnv ?? (aiCreditsFromFile > 0 ? aiCreditsFromFile : (aiCreditsFromMetrics ?? aiCreditsFromFile))) : undefined;
Comment thread
pelikhan marked this conversation as resolved.
const aiCredits = jobEmitsOwnTokenUsage ? (aiCreditsFromEnv ?? ((aiCreditsFromFile ?? 0) > 0 ? aiCreditsFromFile : (aiCreditsFromMetrics ?? aiCreditsFromFile))) : undefined;

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.

[/tdd] The null-safety fix is correct for TypeScript strict checks, but there is no regression test covering the case where agentUsage.ai_credits is undefined or null. Without a test, a future refactor could silently revert the guard.

💡 Suggested test pattern
it("aiCredits falls back to metrics when aiCreditsFromFile is undefined", () => {
  // Arrange: agentUsage has no ai_credits field
  const agentUsage = {}; // ai_credits is undefined
  const runtimeMetrics = { tokenUsage: { ai_credits: 42 } };

  // Act
  const aiCreditsFromFile = agentUsage.ai_credits; // undefined
  const aiCreditsFromMetrics = runtimeMetrics.tokenUsage?.ai_credits;
  const aiCredits = (aiCreditsFromFile ?? 0) > 0
    ? aiCreditsFromFile
    : (aiCreditsFromMetrics ?? aiCreditsFromFile);

  // Assert
  expect(aiCredits).toBe(42);
});

Also note: the runtime behaviour of (aiCreditsFromFile ?? 0) > 0 is identical to the original aiCreditsFromFile > 0 for undefined/null inputs — the fix is purely for TypeScript strict-null compliance. A comment explaining the intent would prevent future confusion.

if (typeof aiCredits === "number") {
attributes.push(buildAttr("gh-aw.aic", aiCredits));
}
Expand Down
Loading