Skip to content

Commit e768cb1

Browse files
pelikhanCopilot
andauthored
Clarify GitHub API quota failures and reset diagnostics (#68737)
* fix: clarify GitHub quota failure boundaries and reset diagnostics Distinguish exhausted and reserved API quota from authentication failures using failed-response metadata. Keep daily accounting fail-closed and review dismissal actor validation unchanged. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix: preserve quota diagnostics across wrapped read failures Address review feedback with bounded cause traversal, sanitized access-failure evidence, HTTP-date Retry-After validation, and summaries for every attributed restore read failure. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 71bc3c5 commit e768cb1

11 files changed

Lines changed: 523 additions & 26 deletions

‎actions/setup/js/check_daily_aic_workflow_guardrail.cjs‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const { getErrorMessage } = require("./error_helpers.cjs");
1313
const { createRateLimitAwareGithub } = require("./github_rate_limit_logger.cjs");
1414
const { scanDailyAIC } = require("./daily_aic_scan.cjs");
1515
const { API_READ_OPERATIONS, createAPIBudget, retryNotBefore, safeResponseHeaders, isQuotaError, getAPIReadFailure } = require("./daily_aic_api_budget.cjs");
16+
const { getGitHubAPIDiagnostic } = require("./github_api_diagnostics.cjs");
1617
const { loadBillableJobs, allBillableJobsNotStarted, allBillableJobsSkipped, sumCoveredComponents } = require("./daily_aic_component_coverage.cjs");
1718
const { readLedgerEntries } = require("./daily_aic_repo_memory_ledger.cjs");
1819

@@ -843,7 +844,8 @@ async function main(options = {}) {
843844
const reason = readFailure
844845
? `${readFailure.operation} read stopped (${readFailure.status ? `HTTP ${readFailure.status}` : "transport or read error"}; attempts: ${readFailure.attempts}; reason: ${readFailure.reason}). Accounting remains unverified; agent startup is blocked.`
845846
: getErrorMessage(error);
846-
const message = `Daily workflow AI Credits are unknown: ${reason}`;
847+
const diagnostic = getGitHubAPIDiagnostic(error);
848+
const message = `Daily workflow AI Credits are unknown: ${reason}${diagnostic ? ` ${diagnostic.message}` : ""}`;
847849
core.setOutput("daily_ai_credits_guardrail_status", status);
848850
core.setOutput("daily_ai_credits_guardrail_error", message);
849851
logDailyGuardrail("AIC inspection failed", { status, error: readFailure || getErrorMessage(error) });

‎actions/setup/js/check_daily_aic_workflow_guardrail.test.cjs‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,7 @@ describe("check_daily_aic_workflow_guardrail", () => {
507507
["repository_history", 502, {}, 3, "retry_exhausted", "transient_error"],
508508
["workflow_history", 503, { "retry-after": "120" }, 1, "retry_restricted", "transient_error"],
509509
["workflow_history", 502, { "x-ratelimit-remaining": "100" }, 1, "quota_stop", "transient_error"],
510+
["workflow_history", 403, { "x-ratelimit-remaining": "0" }, 1, "quota_stop", "transient_error"],
510511
["workflow_history", 403, {}, 1, "non_transient", "structural_error"],
511512
])("main() safely attributes %s HTTP %i while keeping unknown history blocked", async (operation, httpStatus, headers, attempts, reason, guardrailStatus) => {
512513
vi.useFakeTimers();
@@ -534,7 +535,13 @@ describe("check_daily_aic_workflow_guardrail", () => {
534535
const pending = runMain();
535536
await vi.runAllTimersAsync();
536537
await pending;
537-
const message = `Daily workflow AI Credits are unknown: ${operation} read stopped (HTTP ${httpStatus}; attempts: ${attempts}; reason: ${reason}). Accounting remains unverified; agent startup is blocked.`;
538+
const diagnostic =
539+
reason === "quota_stop"
540+
? ` GitHub API ${headers["x-ratelimit-remaining"] === "0" ? "quota_exhausted" : "quota_reserve"} (HTTP ${httpStatus}; resource=unknown; remaining=${headers["x-ratelimit-remaining"]}; limit=unknown; used=unknown; reset=unknown; retry_not_before=unknown)`
541+
: httpStatus === 403
542+
? " GitHub API permission_denied (HTTP 403; Check the credential's access to this operation; quota exhaustion was not established.)"
543+
: "";
544+
const message = `Daily workflow AI Credits are unknown: ${operation} read stopped (HTTP ${httpStatus}; attempts: ${attempts}; reason: ${reason}). Accounting remains unverified; agent startup is blocked.${diagnostic}`;
538545
expect(outputs).toMatchObject({ daily_ai_credits_guardrail_status: guardrailStatus, daily_ai_credits_guardrail_error: message, daily_ai_credits_total: "", daily_ai_credits_estimated: "", daily_ai_credits_exceeded: "false" });
539546
expect(global.core.setFailed).toHaveBeenCalledWith(message);
540547
expect(failedRead).toHaveBeenCalledTimes(attempts);
@@ -552,6 +559,43 @@ describe("check_daily_aic_workflow_guardrail", () => {
552559
}
553560
});
554561

562+
it("main() keeps wrapped quota failures classified and blocks unknown accounting", async () => {
563+
const outputs = {};
564+
const quotaError = httpError(403, "API rate limit exceeded for installation; private-payload");
565+
quotaError.response.headers = { "x-ratelimit-remaining": "0", "x-ratelimit-reset": "1791972360" };
566+
const wrapped = new Error("unable to download artifact: private-payload", { cause: quotaError });
567+
const getCurrent = vi.fn().mockRejectedValue(wrapped);
568+
global.core = {
569+
setOutput: (key, value) => {
570+
outputs[key] = value;
571+
},
572+
setFailed: vi.fn(),
573+
info: vi.fn(),
574+
warning: vi.fn(),
575+
};
576+
global.github = { rest: { actions: { getWorkflowRun: getCurrent } } };
577+
global.context = { repo: { owner: "test-owner", repo: "test-repo" }, runId: 42 };
578+
process.env.GH_AW_MAX_DAILY_AI_CREDITS = "1000";
579+
process.env.GH_AW_GITHUB_TOKEN = "synthetic";
580+
try {
581+
await runMain();
582+
expect(outputs.daily_ai_credits_guardrail_status).toBe("transient_error");
583+
expect(outputs.daily_ai_credits_total).toBe("");
584+
expect(outputs.daily_ai_credits_guardrail_error).toContain("current_run read stopped");
585+
expect(outputs.daily_ai_credits_guardrail_error).toContain("quota_exhausted");
586+
expect(outputs.daily_ai_credits_guardrail_error).toContain("reset=2026-10-14T10:06:00.000Z");
587+
expect(global.core.setFailed).toHaveBeenCalledWith(outputs.daily_ai_credits_guardrail_error);
588+
expect(getCurrent).toHaveBeenCalledOnce();
589+
expect(JSON.stringify([outputs, global.core.info.mock.calls, global.core.setFailed.mock.calls])).not.toContain("private-payload");
590+
} finally {
591+
delete global.core;
592+
delete global.github;
593+
delete global.context;
594+
delete process.env.GH_AW_MAX_DAILY_AI_CREDITS;
595+
delete process.env.GH_AW_GITHUB_TOKEN;
596+
}
597+
});
598+
555599
it("main() counts business requests instead of unrelated rate-limit snapshots", async () => {
556600
// The unrelated quota endpoint must not supply the business-request budget.
557601
const coreInfos = [];

‎actions/setup/js/daily_aic_api_budget.cjs‎

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
// @ts-check
22

3-
const SAFE_HEADERS = ["x-ratelimit-limit", "x-ratelimit-remaining", "x-ratelimit-used", "x-ratelimit-reset", "retry-after"];
3+
const { getGitHubAPIQuota, getGitHubAPIDiagnostic, getSanitizedGitHubAPICause } = require("./github_api_diagnostics.cjs");
4+
5+
const SAFE_HEADERS = ["x-ratelimit-limit", "x-ratelimit-remaining", "x-ratelimit-used", "x-ratelimit-reset", "x-ratelimit-resource", "retry-after"];
46
const API_READ_OPERATIONS = Object.freeze({
57
CURRENT_RUN: "current_run",
68
WORKFLOW_HISTORY: "workflow_history",
@@ -37,12 +39,7 @@ function apiError(status, headers, message) {
3739
}
3840

3941
function retryNotBefore(headers, now = Date.now(), quotaStopped = false) {
40-
const safe = safeResponseHeaders(headers);
41-
const reset = quotaStopped || safe["x-ratelimit-remaining"] === "0" ? Number(safe["x-ratelimit-reset"]) * 1000 : NaN;
42-
const retry = safe["retry-after"];
43-
const retryAt = retry == null ? NaN : /^\d+$/.test(retry) ? now + Number(retry) * 1000 : Date.parse(retry);
44-
const times = [reset, retryAt].filter(value => Number.isFinite(value) && value > now);
45-
return times.length ? new Date(Math.max(...times)).toISOString() : "";
42+
return getGitHubAPIQuota(headers, now, quotaStopped).retryNotBefore;
4643
}
4744

4845
function getErrorStatus(error) {
@@ -52,7 +49,7 @@ function getErrorStatus(error) {
5249
function isQuotaError(error) {
5350
const headers = safeResponseHeaders(error?.response?.headers);
5451
const status = getErrorStatus(error);
55-
return error?.code === "DAILY_AIC_QUOTA_STOP" || status === 429 || (status === 403 && (headers["x-ratelimit-remaining"] === "0" || headers["retry-after"] != null));
52+
return error?.code === "DAILY_AIC_QUOTA_STOP" || status === 429 || (status === 403 && (headers["x-ratelimit-remaining"] === "0" || headers["retry-after"] != null)) || getGitHubAPIDiagnostic(error)?.quota === true;
5653
}
5754

5855
function isTransientError(error) {
@@ -72,13 +69,17 @@ function createAPIBudget(reserve = 100) {
7269
let requests = 0;
7370
let latest = {};
7471
const budget = {
75-
observe(response) {
72+
observe(response, cause = undefined) {
7673
requests++;
7774
latest = safeResponseHeaders(response.headers);
7875
const remaining = latest["x-ratelimit-remaining"];
7976
const limited = response.status === 429 || (response.status === 403 && (remaining === "0" || latest["retry-after"] != null));
8077
if (limited || (remaining != null && Number(remaining) <= reserve)) {
81-
throw Object.assign(apiError(response.status, latest, "Daily AIC inspection stopped to preserve GitHub API quota"), { code: "DAILY_AIC_QUOTA_STOP" });
78+
const sanitizedCause = getSanitizedGitHubAPICause(cause);
79+
const error = Object.assign(apiError(response.status, latest, "Daily AIC inspection stopped to preserve GitHub API quota"), { code: "DAILY_AIC_QUOTA_STOP", ...(sanitizedCause ? { cause: sanitizedCause } : {}) });
80+
const diagnostic = getGitHubAPIDiagnostic(error);
81+
if (diagnostic) error.message += `: ${diagnostic.message}`;
82+
throw error;
8283
}
8384
},
8485
async request(operation, observe = true, readOperation) {
@@ -92,7 +93,7 @@ function createAPIBudget(reserve = 100) {
9293
try {
9394
response = await operation();
9495
} catch (error) {
95-
if (observe && error?.response) budget.observe(error.response);
96+
if (observe && error?.response) budget.observe(error.response, error);
9697
throw error;
9798
}
9899
if (observe) budget.observe(response);
@@ -111,6 +112,8 @@ function createAPIBudget(reserve = 100) {
111112
const failure = Object.freeze({ attempts: attempt + 1, status: status !== null && Number.isInteger(status) && status >= 100 && status <= 599 ? status : null, reason, requests, operation: operationName });
112113
if (error && typeof error === "object") READ_FAILURES.set(error, failure);
113114
core.info(`[daily-workflow-aic] Stopped daily AIC read: ${JSON.stringify(failure)}`);
115+
const diagnostic = getGitHubAPIDiagnostic(error);
116+
if (diagnostic) core.info(`[daily-workflow-aic] ${operationName}: ${diagnostic.message}`);
114117
throw error;
115118
}
116119
const delayMs = 1000 * 2 ** attempt;
@@ -120,12 +123,13 @@ function createAPIBudget(reserve = 100) {
120123
}
121124
},
122125
snapshot() {
126+
const metadata = getGitHubAPIQuota(latest);
123127
return {
124128
requests,
125-
remaining: Number(latest["x-ratelimit-remaining"] || 0),
126-
limit: Number(latest["x-ratelimit-limit"] || 0),
127-
used: Number(latest["x-ratelimit-used"] || 0),
128-
reset: latest["x-ratelimit-reset"] ? new Date(Number(latest["x-ratelimit-reset"]) * 1000).toISOString() : "",
129+
remaining: metadata.remaining ?? 0,
130+
limit: metadata.limit ?? 0,
131+
used: metadata.used ?? 0,
132+
reset: metadata.reset,
129133
};
130134
},
131135
};

‎actions/setup/js/daily_aic_api_budget.test.cjs‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { createRequire } from "node:module";
2+
import { inspect } from "node:util";
23
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
34

45
const require = createRequire(import.meta.url);
56
const { API_READ_OPERATIONS, apiError, createAPIBudget, isTransientError, retryNotBefore, getAPIReadFailure } = require("./daily_aic_api_budget.cjs");
7+
const { getGitHubAPIDiagnostic } = require("./github_api_diagnostics.cjs");
68

79
beforeEach(() => {
810
vi.useFakeTimers();
@@ -16,6 +18,27 @@ afterEach(() => {
1618
const response = (status = 200, headers = { "x-ratelimit-remaining": "4000" }) => ({ status, headers });
1719

1820
describe("bounded daily AIC read retries", () => {
21+
it("classifies a headerless installation rate limit without retrying or publishing its payload", async () => {
22+
const error = apiError(403, {}, "API rate limit exceeded for installation; secret-test-value");
23+
const request = vi.fn().mockRejectedValue(error);
24+
await expect(createAPIBudget().request(request, true, API_READ_OPERATIONS.CURRENT_RUN)).rejects.toBe(error);
25+
expect(getAPIReadFailure(error)).toMatchObject({ attempts: 1, reason: "quota_stop", operation: "current_run" });
26+
expect(request).toHaveBeenCalledOnce();
27+
const logs = global.core.info.mock.calls.flat().join("\n");
28+
expect(logs).toContain("current_run: GitHub API rate_limited");
29+
expect(logs).toContain("remaining=unknown");
30+
expect(logs).not.toContain("secret-test-value");
31+
});
32+
33+
it("does not let malformed reset metadata replace the original failure", async () => {
34+
const request = vi.fn().mockRejectedValue(apiError(403, { "x-ratelimit-remaining": "0", "x-ratelimit-reset": "9999999999999999" }, "private payload"));
35+
const budget = createAPIBudget();
36+
await expect(budget.request(request)).rejects.toMatchObject({ code: "DAILY_AIC_QUOTA_STOP" });
37+
expect(budget.snapshot().reset).toBe("");
38+
expect(retryNotBefore({ "x-ratelimit-remaining": "0", "x-ratelimit-reset": "9999999999999999" })).toBe("");
39+
expect(request).toHaveBeenCalledOnce();
40+
});
41+
1942
it("retries only the failed read and observes each response once", async () => {
2043
const budget = createAPIBudget();
2144
const request = vi
@@ -58,6 +81,69 @@ describe("bounded daily AIC read retries", () => {
5881
expect(global.core.info).toHaveBeenCalledWith(`[daily-workflow-aic] Stopped daily AIC read: {"attempts":1,"status":${status},"reason":"quota_stop","requests":1,"operation":"unknown"}`);
5982
});
6083

84+
it("classifies a synthetic HTTP 403 reserve stop without permission evidence as quota_reserve", () => {
85+
let stopped;
86+
try {
87+
createAPIBudget(100).observe(response(403, { "x-ratelimit-remaining": "50" }));
88+
} catch (error) {
89+
stopped = error;
90+
}
91+
expect(stopped).toMatchObject({ code: "DAILY_AIC_QUOTA_STOP", status: 403 });
92+
expect(getGitHubAPIDiagnostic(stopped)).toMatchObject({ quota: true, classification: "quota_reserve" });
93+
});
94+
95+
it.each([
96+
[401, "Bad credentials", "50", "authentication_failed"],
97+
[403, "Resource not accessible by integration", "50", "permission_denied"],
98+
[403, "Resource not accessible by integration", "0", "permission_denied"],
99+
[403, "API rate limit exceeded for installation", "0", "quota_exhausted"],
100+
[403, "Unspecified error", "50", "quota_reserve"],
101+
])("preserves original HTTP %i access evidence while stopping quota-restricted reads", async (status, message, remaining, classification) => {
102+
const original = apiError(status, { "x-ratelimit-remaining": remaining }, `${message}; secret-test-value`);
103+
const request = vi.fn().mockRejectedValue(original);
104+
const stopped = await createAPIBudget()
105+
.request(request)
106+
.catch(error => error);
107+
expect(stopped).toMatchObject({ code: "DAILY_AIC_QUOTA_STOP", status });
108+
expect(getGitHubAPIDiagnostic(stopped)).toMatchObject({ quota: true, classification });
109+
if (classification === "quota_exhausted" || classification === "quota_reserve") {
110+
expect(stopped.cause).toBeUndefined();
111+
} else {
112+
expect(stopped.cause).not.toBe(original);
113+
expect(stopped.cause).toMatchObject({ status, message: status === 401 ? "Bad credentials" : "Permission denied" });
114+
expect(Object.keys(stopped.cause)).toEqual(["status"]);
115+
}
116+
expect(request).toHaveBeenCalledOnce();
117+
expect(vi.getTimerCount()).toBe(0);
118+
const logs = global.core.info.mock.calls.flat().join("\n");
119+
expect(logs).toContain(`GitHub API ${classification}`);
120+
expect(logs).toContain('"reason":"quota_stop"');
121+
expect(logs).not.toContain("secret-test-value");
122+
});
123+
124+
it.each([
125+
[401, "Bad credentials", "authentication_failed"],
126+
[403, "Resource not accessible by integration", "permission_denied"],
127+
[403, "Resource not accessible by personal access token", "permission_denied"],
128+
[403, "API rate limit exceeded", "quota_exhausted"],
129+
])("never exposes original HTTP %i error properties during terminal inspection", async (status, message, classification) => {
130+
const original = apiError(status, { "x-ratelimit-remaining": "0" }, `${message}; secret-message-value`);
131+
original.request = { url: "https://example.invalid/?token=secret-url-value", headers: { authorization: "secret-request-header-value" }, body: "secret-request-body-value" };
132+
original.response.headers.authorization = "secret-response-header-value";
133+
original.response.headers.location = "https://example.invalid/?token=secret-location-value";
134+
original.response.body = "secret-response-body-value";
135+
original.cause = new Error("secret-nested-cause-value");
136+
const stopped = await createAPIBudget()
137+
.request(() => Promise.reject(original))
138+
.catch(error => error);
139+
expect(stopped).toMatchObject({ code: "DAILY_AIC_QUOTA_STOP", status });
140+
expect(getGitHubAPIDiagnostic(stopped)).toMatchObject({ quota: true, classification });
141+
const terminal = inspect(stopped, { depth: null, showHidden: true });
142+
expect(terminal).not.toMatch(/secret-[\w-]+-value|example\.invalid/);
143+
expect(terminal).toContain("DAILY_AIC_QUOTA_STOP");
144+
expect(vi.getTimerCount()).toBe(0);
145+
});
146+
61147
it("does not retry a server error with an explicit retry restriction", async () => {
62148
const request = vi.fn(async () => response(503, { "retry-after": "120" }));
63149
await expect(createAPIBudget().request(request)).rejects.toMatchObject({ status: 503 });

0 commit comments

Comments
 (0)