-
Notifications
You must be signed in to change notification settings - Fork 1
fix: bypass rate-limit-only Gate cancellations - proceed with work #702
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.
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
Changes from 1 commit
b1879c1
86b6e45
79bea4a
a0d9761
c7e6558
9d9ec34
9c87503
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -1129,8 +1129,13 @@ async function evaluateKeepaliveLoop({ github, context, core, payload: overrideP | |||||||||||||||||||||||||||||||||
| runId: gateRun.runId, | ||||||||||||||||||||||||||||||||||
| core, | ||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||
| // forceRetry bypasses defer/wait for cancelled gates | ||||||||||||||||||||||||||||||||||
| if (forceRetry && tasksRemaining) { | ||||||||||||||||||||||||||||||||||
| // Rate limits are infrastructure noise, not code quality issues | ||||||||||||||||||||||||||||||||||
| // Proceed with work if Gate only failed due to rate limits | ||||||||||||||||||||||||||||||||||
| if (gateRateLimit && tasksRemaining) { | ||||||||||||||||||||||||||||||||||
| action = 'run'; | ||||||||||||||||||||||||||||||||||
| reason = 'bypass-rate-limit-gate'; | ||||||||||||||||||||||||||||||||||
| if (core) core.info('Gate cancelled due to rate limits only - proceeding with work (rate limits are not code quality issues)'); | ||||||||||||||||||||||||||||||||||
|
stranske marked this conversation as resolved.
Outdated
|
||||||||||||||||||||||||||||||||||
| } else if (forceRetry && tasksRemaining) { | ||||||||||||||||||||||||||||||||||
| action = 'run'; | ||||||||||||||||||||||||||||||||||
| reason = 'force-retry-cancelled'; | ||||||||||||||||||||||||||||||||||
| if (core) core.info(`Force retry enabled: bypassing cancelled gate (rate_limit=${gateRateLimit})`); | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+1134
to
1141
|
||||||||||||||||||||||||||||||||||
| if (gateRateLimit && tasksRemaining) { | |
| action = 'run'; | |
| reason = 'bypass-rate-limit-gate'; | |
| if (core) core.info('Gate cancelled due to rate limits only - proceeding with work (rate limits are not code quality issues)'); | |
| } else if (forceRetry && tasksRemaining) { | |
| action = 'run'; | |
| reason = 'force-retry-cancelled'; | |
| if (core) core.info(`Force retry enabled: bypassing cancelled gate (rate_limit=${gateRateLimit})`); | |
| if (forceRetry && tasksRemaining) { | |
| action = 'run'; | |
| reason = 'force-retry-cancelled'; | |
| if (core) core.info(`Force retry enabled: bypassing cancelled gate (rate_limit=${gateRateLimit})`); | |
| } else if (gateRateLimit && tasksRemaining) { | |
| action = 'run'; | |
| reason = 'bypass-rate-limit-gate'; | |
| if (core) core.info('Gate cancelled due to rate limits only - proceeding with work (rate limits are not code quality issues)'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This change will cause existing tests to fail. The tests at lines 423-469 in keepalive-loop.test.js expect action='defer' and reason='gate-cancelled-rate-limit' when rate limit cancellations are detected. With this new code, when tasksRemaining is true, the action will be 'run' and reason will be 'bypass-rate-limit-gate' instead. The tests need to be updated to reflect this new behavior, or new tests should be added to verify the bypass logic works as intended.