Skip to content

Throttle central PR workflow queue - #338

Merged
seonghobae merged 1 commit into
mainfrom
agent/throttle-central-pr-workflow-queue
Jul 7, 2026
Merged

Throttle central PR workflow queue#338
seonghobae merged 1 commit into
mainfrom
agent/throttle-central-pr-workflow-queue

Conversation

@seonghobae

@seonghobaeseonghobae commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • limit the merge scheduler to one OpenCode/Strix dispatch per run by default
  • add PR-level concurrency cancellation to central required PR workflows that were piling up superseded runs
  • prevent cancelled OpenCode/Strix workflow_run completions from spawning more Noema/Scheduler work
  • add a workflow queue contract test

Root cause

The central PR review merge scheduler defaulted to unlimited review/security dispatches, while several required PR workflows lacked per-PR concurrency. A wave of open PRs could enqueue many heavy CodeQL, OSV, Security Scan, Scorecard, OpenCode, Strix, Noema, and scheduler runs faster than runners could drain them. Cancelling stale OpenCode/Strix runs also exposed a second loop: cancelled workflow_run completions were still triggering Noema/Scheduler runs, refilling the queue.

Impact

Old PRs were left blocked behind queued required checks rather than being merged or clearly rejected. This change makes the queue drain gradually and prevents stale/cancelled runs from recreating the backlog.

Validation

  • python3 -m pytest -s tests/test_required_workflow_queue_contract.py tests/test_codeql_pr_workflow_contract.py -q
  • uv run --python 3.13 python scripts/ci/pr_review_merge_scheduler.py --self-test
  • uv run --python 3.13 pytest -s -q tests/test_required_workflow_queue_contract.py tests/test_codeql_pr_workflow_contract.py tests/test_pr_review_merge_scheduler.py
  • parsed all workflow YAML with PyYAML

@seonghobae
seonghobaeforce-pushed the agent/throttle-central-pr-workflow-queue branch from 49736cb to 4007f69CompareJuly 7, 2026 04:01
@seonghobae
seonghobae merged commit a71be53 into mainJul 7, 2026
1 of 13 checks passed
@seonghobae
seonghobae deleted the agent/throttle-central-pr-workflow-queue branch July 7, 2026 04:08
seonghobae added a commit that referenced this pull request Jul 8, 2026
…redundant SSRF check
Three tests broke the coverage-evidence gate because intentional changes
landed without updating their assertions (or, for noema, added a redundant
guard that shadowed an existing one):
- test_call_llm_handles_configuration_and_verdicts: the redundant
startswith() scheme guard (#297) was shadowed by the comprehensive,
case-insensitive scheme check from #294, was case-sensitive (wrongly
rejecting valid HTTPS:// URLs the suite exercises), and leaked the URL
in its error (violating the "no original URL in error" intent). Removed
the dead guard and its duplicate test block; file:// rejection stays
covered by the existing assertion. Restores scripts/ci 100% coverage.
- test_workflow_provisions_sandbox_tool_and_reviewer_agent: the
opencode-review-target if: block was expanded to a multi-line form in
a4df728 (cancel runs on PR close), growing the header past the regex's
240-char proximity window. Widened to 400; timeout-minutes: 360 is unique.
- test_merge_scheduler_uses_escalating_mutation_credentials: #338 throttled
review_dispatch_limit default from "-1" to "1"; updated the stale
assertion. The "-1" empty-input fallback assertion is retained.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RTAMs4bpSZS77Xe3RQjv9P
seonghobae added a commit that referenced this pull request Jul 8, 2026
…ertions
Three tests failed on this branch (inherited red state from main), blocking
the coverage-evidence job:
1. test_noema_review_gate: #297 re-added a case-sensitive
`api_url.startswith("http://"/"https://")` guard AFTER #294's
case-insensitive urlparse scheme check. It was redundant for rejecting
file:// (the scheme check already raises "URL scheme must be http or
https" first) and, worse, wrongly rejected valid uppercase-scheme URLs
like HTTPS:// that #294 intentionally supports and tests. Removed the
redundant/buggy check; SSRF protection is fully retained by the
urlparse scheme + hostname + private-IP validation. Updated the stale
assertion to expect the scheme-check message.
2. test_opencode_agent_contract (opencode-review-target timeout): the job
grew a legitimate `if:` guard, pushing timeout-minutes:360 to 327 chars
from the job key, past the 240-char proximity window. Widened the
window to 360; still asserts the timeout belongs to that job.
3. test_opencode_agent_contract (scheduler default): #338 "Throttle
central PR workflow queue" deliberately changed the review_dispatch_limit
input default from "-1" to "1" (and shipped its own contract test), but
this assertion was left stale. Updated it to "1".
Full suite: 174 passed, coverage 100% (fail_under=100).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RTAMs4bpSZS77Xe3RQjv9P
seonghobae pushed a commit that referenced this pull request Jul 10, 2026
…heme bug
The coverage-evidence gate runs the PR-head test suite; it was red on main
(3 failing tests) because contract expectations drifted from intentionally
updated source/workflows, masking a real SSRF-guard bug in noema.
- test_noema_review_gate: stale early assertion expected the old
"must start with http:// or https://" message for file:///etc/passwd, but
the scheme guard (checked first) correctly raises "URL scheme must be http
or https". Align the assertion with the stronger guard.
- noema_review_gate.call_llm: the redundant belt-and-suspenders check used a
case-sensitive startswith, so a valid uppercase "HTTPS://" endpoint was
wrongly rejected as SSRF (the case-insensitive scheme guard above already
accepted it). Lowercase before the startswith so valid uppercase schemes
pass while SSRF protection is unchanged.
- test_opencode_agent_contract: the opencode-review-target timeout regex bound
({0,240}) was too tight after the job's if: block grew; widen to {0,400}.
The job still asserts timeout-minutes: 360.
- test_opencode_agent_contract: merge-scheduler review_dispatch_limit input
default was intentionally throttled from "-1" to "1" in #338; update the
contract to "1" (the shell fallback to "-1" is still asserted).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RTAMs4bpSZS77Xe3RQjv9P
opencode-agentBot added a commit that referenced this pull request Jul 10, 2026
* [codex] Fail OpenCode approve when review publish fails
* [codex] Bound OpenCode model pool candidates
* [codex] Guard OpenCode publish failure log groups
* [codex] Cancel stale OpenCode PR review runs
* Re-kick model pool once on OpenCode exhaustion
* Log empty rekick outcome before fail-closed fallback
* Simplify re-kick run invocation in approval gate
* Harden re-kick logging for exhausted model pool
* Retry exhausted OpenCode model pool with exponential backoff
* Bound exhausted model re-kicks with exponential backoff window
* fix(ci): unblock coverage-evidence by fixing three stale contract assertions
Three tests failed on this branch (inherited red state from main), blocking
the coverage-evidence job:
1. test_noema_review_gate: #297 re-added a case-sensitive
`api_url.startswith("http://"/"https://")` guard AFTER #294's
case-insensitive urlparse scheme check. It was redundant for rejecting
file:// (the scheme check already raises "URL scheme must be http or
https" first) and, worse, wrongly rejected valid uppercase-scheme URLs
like HTTPS:// that #294 intentionally supports and tests. Removed the
redundant/buggy check; SSRF protection is fully retained by the
urlparse scheme + hostname + private-IP validation. Updated the stale
assertion to expect the scheme-check message.
2. test_opencode_agent_contract (opencode-review-target timeout): the job
grew a legitimate `if:` guard, pushing timeout-minutes:360 to 327 chars
from the job key, past the 240-char proximity window. Widened the
window to 360; still asserts the timeout belongs to that job.
3. test_opencode_agent_contract (scheduler default): #338 "Throttle
central PR workflow queue" deliberately changed the review_dispatch_limit
input default from "-1" to "1" (and shipped its own contract test), but
this assertion was left stale. Updated it to "1".
Full suite: 174 passed, coverage 100% (fail_under=100).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RTAMs4bpSZS77Xe3RQjv9P
* test: cover noema parsed scheme guard
---------
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: Seongho Bae <seonghobae@me.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: opencode-agent[bot] <219766164+opencode-agent[bot]@users.noreply.github.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@seonghobae