Skip to content

[code-simplifier] Simplify AI credits audit log parsing: extract shared helper, eliminate double file read #37731

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
pelikhan merged 4 commits into from
Jun 8, 2026
Merged

[code-simplifier] Simplify AI credits audit log parsing: extract shared helper, eliminate double file read #37731

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eaa855a
simplify: extract iterateAuditEntries helper and eliminate double aud…
github-actions[bot] Jun 8, 2026
997708d
Fix js-typecheck and lint regressions in AIC audit parsing
Copilot Jun 8, 2026
e14f4e2
Refine guardrail readability and log-shape intent
Copilot Jun 8, 2026
6249726
Document defensive accumulator behavior in audit parser
Copilot Jun 8, 2026
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
123 changes: 80 additions & 43 deletions actions/setup/js/ai_credits_context.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ const path = require("path");
const MAX_AI_CREDITS_FIELDS = new Set(["max_ai_credits", "maxAiCredits"]);
const AI_CREDITS_FIELDS = new Set(["ai_credits", "aiCredits"]);
const AI_CREDITS_RATE_LIMIT_ERROR_FIELDS = new Set(["ai_credits_rate_limit_error", "aiCreditsRateLimitError"]);
// Note: these text fields are intentionally broad (common field names like "error", "message") because
// rate-limit signals can appear in any of them. This asymmetry vs parseMaxAICreditsFromAuditLog is deliberate.
const AI_CREDITS_RATE_LIMIT_TEXT_FIELDS = new Set(["error", "message", "reason", "details", "detail", "type", "code"]);
const AI_CREDITS_RATE_LIMIT_PATTERNS = [/ai[\s_-]*credits?.*(?:rate[\s-]*limit|limit exceeded|budget exceeded|exceeded)/i, /(?:rate[\s-]*limit|too many requests).*(?:ai[\s_-]*credits?)/i, /\bai_credits_limit_exceeded\b/i];

Expand Down Expand Up @@ -74,12 +76,12 @@ function resolveFirewallAuditLogPath(auditJsonlPathOverride) {
candidateBases.push("/tmp/gh-aw/sandbox/firewall/logs");

for (const base of candidateBases) {
const logPath = path.join(base, "log.jsonl");
if (fs.existsSync(logPath)) return logPath;
const auditPath = path.join(base, "audit.jsonl");
if (fs.existsSync(auditPath)) return auditPath;
for (const filename of ["log.jsonl", "audit.jsonl"]) {
const candidate = path.join(base, filename);
if (fs.existsSync(candidate)) return candidate;
}
}
return path.join(candidateBases[0] || "/tmp/gh-aw/sandbox/firewall/audit", "log.jsonl");
return path.join(candidateBases[0], "log.jsonl");
}

/**
Expand Down Expand Up @@ -131,73 +133,108 @@ function parseAICreditsErrorInfoFromAuditEntry(entry) {
}

/**
* @param {string} [auditJsonlPathOverride]
* @returns {string}
* Reads a firewall audit JSONL file and calls accumulate for each parsed entry.
* Returns the accumulated result, or defaultValue on missing file or any error.
*
* @template T
* @param {string | undefined} auditJsonlPathOverride
* @param {T} defaultValue
* @param {((content: string) => boolean) | null} contentGuard - When non-null, called with raw file
* content before iteration; return false to skip parsing entirely (fast-path optimization).
* @param {(acc: T, entry: unknown) => T | undefined} accumulate - Callers should return a defined
* value; undefined is ignored defensively to preserve the previous accumulator.
* @returns {T}
*/
function parseMaxAICreditsFromAuditLog(auditJsonlPathOverride) {
function iterateAuditEntries(auditJsonlPathOverride, defaultValue, contentGuard, accumulate) {

@github-actions github-actions Bot Jun 8, 2026

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.

[/tdd] iterateAuditEntries is a non-trivial pure function with several independently-testable edge cases — but no test file exists for this module (unlike peer modules such as parse_mcp_gateway_log.test.cjs, error_codes.test.cjs, etc.).

💡 Suggested edge cases to cover 
// 1. Missing file → returns defaultValue
// 2. Empty file → returns defaultValue
// 3. contentGuard returns false → returns defaultValue without parsing
// 4. All lines are malformed JSON → returns defaultValue unchanged
// 5. Accumulate is called per-entry in order
// 6. Combined: only error-info entries do not bleed into maxAICredits field

The parseAuditLogCombined single-pass semantics (both parsers run on every line) are particularly worth a test to guard against future regressions when either entry-parser changes.

try {
const auditJsonlPath = resolveFirewallAuditLogPath(auditJsonlPathOverride);
if (!fs.existsSync(auditJsonlPath)) return "";
if (!fs.existsSync(auditJsonlPath)) return defaultValue;
const content = fs.readFileSync(auditJsonlPath, "utf8");
if (!content.trim() || !/(?:max_ai_credits|maxAiCredits)/.test(content)) return "";
let parsedMaxAICredits = "";
if (!content.trim()) return defaultValue;
if (contentGuard && !contentGuard(content)) return defaultValue;
let result = defaultValue;

@github-actions github-actions Bot Jun 8, 2026

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.

defaultValue aliased into result — mutation-safe only by convention, not contract. Any accumulate callback that mutates acc in-place instead of returning a fresh object will silently corrupt the defaultValue reference passed by the caller.

💡 Details and suggested fix

Currently all callers pass inline object literals ({ aiCredits: "", ... }), so each call site creates a fresh object and corruption is limited to a single invocation. However the function is a new generic abstraction that future contributors will re-use. Nothing in the signature or JSDoc prevents passing a module-level constant as defaultValue, which would be permanently corrupted on the first mutating accumulate.

Simplest fix: note the invariant in the JSDoc and optionally shallow-clone the seed:

// Option A: document the contract
/**
 * `@param` {(acc: T, entry: unknown) => T} accumulate - MUST return a new value/object; must not mutate acc.
 */

// Option B: defensive clone (only needed if T is an object)
let result = (defaultValue !== null && typeof defaultValue === "object")
  ? { ...defaultValue }
  : defaultValue;

Without this, the first caller that accidentally writes acc.field = value; return acc; will introduce a silent state-pollution bug that is very hard to diagnose.

for (const line of content.split("\n")) {
const trimmed = line.trim();
if (!trimmed || trimmed[0] !== "{") continue;
try {
const entry = JSON.parse(trimmed);
const value = parseMaxAICreditsFromAuditEntry(entry);
if (value) parsedMaxAICredits = value;
const nextResult = accumulate(result, JSON.parse(trimmed));
if (nextResult !== undefined) result = nextResult;
} catch {
// ignore malformed lines
}
}
return parsedMaxAICredits;
return result;
} catch {
return "";
return defaultValue;
}
}

/**
* @param {string} [auditJsonlPathOverride]
* @returns {string}
*/
function parseMaxAICreditsFromAuditLog(auditJsonlPathOverride) {
return iterateAuditEntries(
auditJsonlPathOverride,
"",
content => /(?:max_ai_credits|maxAiCredits)/.test(content),
(acc, entry) => parseMaxAICreditsFromAuditEntry(entry) || acc
);
}

/**
* @param {string} [auditJsonlPathOverride]
* @returns {{ aiCredits: string, rateLimitError: boolean }}
*/
function parseAICreditsErrorInfoFromAuditLog(auditJsonlPathOverride) {
try {
const auditJsonlPath = resolveFirewallAuditLogPath(auditJsonlPathOverride);
if (!fs.existsSync(auditJsonlPath)) return { aiCredits: "", rateLimitError: false };
const content = fs.readFileSync(auditJsonlPath, "utf8");
if (!content.trim()) return { aiCredits: "", rateLimitError: false };
let parsedAICredits = "";
let hasRateLimitError = false;
for (const line of content.split("\n")) {
const trimmed = line.trim();
if (!trimmed || trimmed[0] !== "{") continue;
try {
const entry = JSON.parse(trimmed);
const parsed = parseAICreditsErrorInfoFromAuditEntry(entry);
if (parsed.aiCredits) parsedAICredits = parsed.aiCredits;
if (parsed.rateLimitError) hasRateLimitError = true;
} catch {
// ignore malformed lines
}
}
return { aiCredits: parsedAICredits, rateLimitError: hasRateLimitError };
} catch {
return { aiCredits: "", rateLimitError: false };
}
// No content-guard fast-path: the rate-limit signal appears in common field names
// (error, message, reason…) that are present in almost every entry, making a
// field-name pre-scan near-useless. The asymmetry vs parseMaxAICreditsFromAuditLog
// is intentional — see AI_CREDITS_RATE_LIMIT_TEXT_FIELDS comment above.
/** @type {{ aiCredits: string, rateLimitError: boolean }} */
const initial = { aiCredits: "", rateLimitError: false };
return iterateAuditEntries(auditJsonlPathOverride, initial, null, (acc, entry) => {
const parsed = parseAICreditsErrorInfoFromAuditEntry(entry);
return {
aiCredits: parsed.aiCredits || acc.aiCredits,
rateLimitError: acc.rateLimitError || parsed.rateLimitError,
};
});
}

/**
* Single-pass combined read of the audit log, returning all AI credits fields at once.
* Used by resolveAICreditsFailureState to avoid reading the same file twice.
* No contentGuard is applied: rate-limit signal detection must scan all entries anyway,
* so a single full pass is cheaper than two guarded passes.
*
* @param {string} [auditJsonlPathOverride]
* @returns {{ aiCredits: string, maxAICredits: string, rateLimitError: boolean }}
*/
function parseAuditLogCombined(auditJsonlPathOverride) {
/** @type {{ aiCredits: string, maxAICredits: string, rateLimitError: boolean }} */
const initial = { aiCredits: "", maxAICredits: "", rateLimitError: false };
return iterateAuditEntries(auditJsonlPathOverride, initial, null, (acc, entry) => {
const errorInfo = parseAICreditsErrorInfoFromAuditEntry(entry);
const max = parseMaxAICreditsFromAuditEntry(entry);
return {
aiCredits: errorInfo.aiCredits || acc.aiCredits,
maxAICredits: max || acc.maxAICredits,
rateLimitError: acc.rateLimitError || errorInfo.rateLimitError,
};
});
}

/**
* @returns {{ aiCredits: string, maxAICredits: string, aiCreditsRateLimitError: boolean }}
*/
function resolveAICreditsFailureState() {
const parsedAICreditsErrorInfo = parseAICreditsErrorInfoFromAuditLog();
const { aiCredits: auditAICredits, maxAICredits: auditMaxAICredits, rateLimitError: auditRateLimitError } = parseAuditLogCombined();
const envAICredits = parsePositiveNumberString(process.env.GH_AW_AIC);
const envMaxAICredits = parsePositiveNumberString(process.env.GH_AW_MAX_AI_CREDITS);
const aiCredits = parsedAICreditsErrorInfo.aiCredits || envAICredits || "";
const maxAICredits = parseMaxAICreditsFromAuditLog() || envMaxAICredits || "";
const rawAICreditsRateLimitError = parsedAICreditsErrorInfo.rateLimitError || process.env.GH_AW_AI_CREDITS_RATE_LIMIT_ERROR === "true";
const aiCredits = auditAICredits || envAICredits || "";
const maxAICredits = auditMaxAICredits || envMaxAICredits || "";
const rawAICreditsRateLimitError = auditRateLimitError || process.env.GH_AW_AI_CREDITS_RATE_LIMIT_ERROR === "true";
const aiCreditsRateLimitError = shouldReportAICreditsRateLimitError(rawAICreditsRateLimitError, aiCredits, maxAICredits);
return { aiCredits, maxAICredits, aiCreditsRateLimitError };
}
Expand Down
15 changes: 7 additions & 8 deletions actions/setup/js/check_daily_aic_workflow_guardrail.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,10 @@ function logDailyGuardrail(message, details) {
*/
function shouldSkipDailyAICGuardrail() {
const eventName = process.env.GITHUB_EVENT_NAME || "";
if (eventName === "workflow_call" || eventName === "repository_dispatch") {
return true;
}
return eventName === "workflow_dispatch" && (process.env.GH_AW_WORKFLOW_DISPATCH_AW_CONTEXT || "").trim() !== "";
const isWorkflowCall = eventName === "workflow_call";
const isRepositoryDispatch = eventName === "repository_dispatch";
const hasDispatchContext = (process.env.GH_AW_WORKFLOW_DISPATCH_AW_CONTEXT || "").trim() !== "";
return isWorkflowCall || isRepositoryDispatch || (eventName === "workflow_dispatch" && hasDispatchContext);
}

/**
Expand Down Expand Up @@ -156,7 +156,7 @@ async function getRunAIC(artifactClient, runId, token, owner, repo) {
* @returns {string}
*/
function formatInteger(value) {
const safeValue = Number.isFinite(value) ? Math.round(value || 0) : 0;
const safeValue = typeof value === "number" && Number.isFinite(value) ? Math.round(value) : 0;
return INTEGER_FORMATTER.format(safeValue);
}

Expand Down Expand Up @@ -349,8 +349,6 @@ async function main() {
const cutoffMs = Date.now() - DAILY_WORKFLOW_WINDOW_MS;
/** @type {Array<{id:number, html_url:string, created_at:string, conclusion:string}>} */
const candidateRuns = [];
/** @type {Array<any>} */
let runs = [];
let page = 1;
let truncatedByRateLimit = false;
while (page <= MAX_WORKFLOW_RUN_PAGES) {
Expand All @@ -368,7 +366,7 @@ async function main() {
per_page: 100,
page,
});
runs = response.data.workflow_runs || [];
const runs = response.data.workflow_runs || [];
logDailyGuardrail("Received workflow runs page", {
page,
runCount: runs.length,
Expand Down Expand Up @@ -453,6 +451,7 @@ async function main() {
truncatedByRateLimit,
};
logDailyGuardrail("Completed AIC inspection window", {
// Keep these explicit to preserve existing log shape (exclude truncatedByRateLimit).
candidateRunsCount: summaryMeta.candidateRunsCount,
inspectedRunsCount: summaryMeta.inspectedRunsCount,
countedRunIds: countedRuns.map(run => run.id),
Expand Down
Loading