Skip to content

Commit a54a6e7

Browse files
authored
Block silent detection pass and classify Claude embedded 429 rate limits (#31081)
1 parent b77112c commit a54a6e7

6 files changed

Lines changed: 45 additions & 4 deletions

File tree

actions/setup/js/claude_harness.cjs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,11 @@ const MAX_DELAY_MS = 60000;
6161
const OVERLOADED_ERROR_PATTERN = /overloaded_error|"overloaded"/i;
6262

6363
// Pattern to detect Anthropic rate-limit errors (HTTP 429).
64-
const RATE_LIMIT_ERROR_PATTERN = /rate_limit_error|429 Too Many Requests/i;
64+
// Claude CLI may surface this as:
65+
// - transport-style text (e.g. "429 Too Many Requests")
66+
// - embedded stream-json result fields (e.g. "api_error_status":429)
67+
// - human-readable message text ("rate limit")
68+
const RATE_LIMIT_ERROR_PATTERN = /rate_limit_error|429 Too Many Requests|"api_error_status"\s*:\s*429|request rejected \(429\)|rate limit/i;
6569

6670
// Pattern to detect a clean max-turns exit from Claude Code.
6771
// Claude Code emits a JSON result object with "subtype":"error_max_turns" when the
@@ -341,6 +345,7 @@ if (typeof module !== "undefined" && module.exports) {
341345
module.exports = {
342346
resolveClaudePromptFileArgs,
343347
stripPromptFileArgs,
348+
isRateLimitError,
344349
isMaxTurnsExit,
345350
isNoDeferredMarkerError,
346351
};

actions/setup/js/claude_harness.test.cjs

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import os from "os";
55
import path from "path";
66

77
const require = createRequire(import.meta.url);
8-
const { resolveClaudePromptFileArgs, stripPromptFileArgs, isMaxTurnsExit, isNoDeferredMarkerError } = require("./claude_harness.cjs");
8+
const { resolveClaudePromptFileArgs, stripPromptFileArgs, isRateLimitError, isMaxTurnsExit, isNoDeferredMarkerError } = require("./claude_harness.cjs");
99

1010
describe("claude_harness.cjs", () => {
1111
describe("resolveClaudePromptFileArgs", () => {
@@ -108,6 +108,20 @@ describe("claude_harness.cjs", () => {
108108
});
109109
});
110110

111+
describe("isRateLimitError", () => {
112+
it("returns true for stream-json api_error_status 429", () => {
113+
expect(isRateLimitError('{"type":"result","subtype":"success","is_error":true,"api_error_status":429}')).toBe(true);
114+
});
115+
116+
it("returns true for stream-json request rejected 429 message", () => {
117+
expect(isRateLimitError("API Error: Request rejected (429) · This request would exceed your account's rate limit.")).toBe(true);
118+
});
119+
120+
it("returns false for non-rate-limit output", () => {
121+
expect(isRateLimitError('{"type":"result","subtype":"success","is_error":false}')).toBe(false);
122+
});
123+
});
124+
111125
describe("isNoDeferredMarkerError", () => {
112126
it("returns true for the canonical no-deferred-marker error message", () => {
113127
const output =

actions/setup/js/parse_threat_detection_results.cjs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,8 @@ async function main() {
263263
const logPath = path.join(threatDetectionDir, DETECTION_LOG_FILENAME);
264264
const runDetection = process.env.RUN_DETECTION;
265265
const continueOnError = process.env.GH_AW_DETECTION_CONTINUE_ON_ERROR !== "false";
266+
const detectionExecutionOutcome = process.env.DETECTION_AGENTIC_EXECUTION_OUTCOME || "";
267+
const detectionExecutionFailed = detectionExecutionOutcome === "failure";
266268
const isWarnMode = continueOnError;
267269

268270
/**
@@ -273,8 +275,9 @@ async function main() {
273275
* @param {string} message - Human-readable error message
274276
*/
275277
function setDetectionFailure(reason, message) {
278+
const mustFail = detectionExecutionFailed && (reason === "agent_failure" || reason === "parse_error");
276279
core.setOutput("reason", reason);
277-
if (isWarnMode) {
280+
if (isWarnMode && !mustFail) {
278281
core.warning(`⚠️ ${message}`);
279282
core.setOutput("conclusion", "warning");
280283
core.setOutput("success", "false");
@@ -309,6 +312,7 @@ async function main() {
309312
core.info("════════════════════════════════════════════════════════");
310313
core.info(`📋 RUN_DETECTION env: ${JSON.stringify(runDetection)}`);
311314
core.info(`📋 continue-on-error: ${continueOnError}`);
315+
core.info(`📋 detection execution outcome: ${JSON.stringify(detectionExecutionOutcome)}`);
312316
core.info(`📁 Threat detection directory: ${threatDetectionDir}`);
313317
core.info(`📄 Detection log path: ${logPath}`);
314318

actions/setup/js/parse_threat_detection_results.test.cjs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,6 +444,7 @@ describe("main", () => {
444444
// Reset environment variables
445445
delete process.env.RUN_DETECTION;
446446
delete process.env.GH_AW_DETECTION_CONTINUE_ON_ERROR;
447+
delete process.env.DETECTION_AGENTIC_EXECUTION_OUTCOME;
447448
// Re-import to get fresh module with mocks
448449
mod = await import("./parse_threat_detection_results.cjs");
449450
});
@@ -508,6 +509,18 @@ describe("main", () => {
508509
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Detection log file not found"));
509510
});
510511

512+
it("should fail when detection execution failed even in warn mode", async () => {
513+
process.env.DETECTION_AGENTIC_EXECUTION_OUTCOME = "failure";
514+
mockExistsSync.mockReturnValue(false);
515+
516+
await mod.main();
517+
518+
expect(mockCore.setOutput).toHaveBeenCalledWith("conclusion", "failure");
519+
expect(mockCore.setOutput).toHaveBeenCalledWith("success", "false");
520+
expect(mockCore.setOutput).toHaveBeenCalledWith("reason", "agent_failure");
521+
expect(mockCore.setFailed).toHaveBeenCalledWith(expect.stringContaining("Detection log file not found"));
522+
});
523+
511524
// Note: The following tests are skipped because mocking fs for CJS modules
512525
// is difficult in vitest (same issue as safe_output_validator.test.cjs).
513526
// The core parsing logic is thoroughly tested via parseDetectionLog above.

pkg/workflow/detection_success_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@ Create an issue.
7070
if !strings.Contains(detectionSection, "GH_AW_DETECTION_CONTINUE_ON_ERROR:") {
7171
t.Error("Detection conclusion step missing GH_AW_DETECTION_CONTINUE_ON_ERROR env var")
7272
}
73+
if !strings.Contains(detectionSection, "DETECTION_AGENTIC_EXECUTION_OUTCOME: ${{ steps.detection_agentic_execution.outcome }}") {
74+
t.Error("Detection conclusion step missing DETECTION_AGENTIC_EXECUTION_OUTCOME env var")
75+
}
7376

7477
// Check that the combined parse-and-conclude step has ID detection_conclusion
7578
if !strings.Contains(detectionSection, "id: detection_conclusion") {

pkg/workflow/threat_detection.go

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,7 @@ func (c *Compiler) buildDetectionConclusionStep(data *WorkflowData) []string {
488488
fmt.Sprintf(" uses: %s\n", getCachedActionPin("actions/github-script", data)),
489489
" env:\n",
490490
" RUN_DETECTION: ${{ steps.detection_guard.outputs.run_detection }}\n",
491+
" DETECTION_AGENTIC_EXECUTION_OUTCOME: ${{ steps.detection_agentic_execution.outcome }}\n",
491492
coeEnvLine,
492493
" with:\n",
493494
" script: |\n",
@@ -774,10 +775,11 @@ func (c *Compiler) buildResultsParsingScriptRequire() string {
774775
await main();
775776
} catch (loadErr) {
776777
const continueOnError = process.env.GH_AW_DETECTION_CONTINUE_ON_ERROR !== 'false';
778+
const detectionExecutionFailed = process.env.DETECTION_AGENTIC_EXECUTION_OUTCOME === 'failure';
777779
const msg = 'ERR_SYSTEM: \u274C Unexpected error loading threat detection module: ' + (loadErr && loadErr.message ? loadErr.message : String(loadErr));
778780
core.error(msg);
779781
core.setOutput('reason', 'parse_error');
780-
if (continueOnError) {
782+
if (continueOnError && !detectionExecutionFailed) {
781783
core.warning('\u26A0\uFE0F ' + msg);
782784
core.setOutput('conclusion', 'warning');
783785
core.setOutput('success', 'false');

0 commit comments

Comments
 (0)