fix(scheduler): isolate central Actions inventory quota - #1231
fix(scheduler): isolate central Actions inventory quota#1231seonghobae wants to merge 23 commits into
Conversation
…-read-token # Conflicts: # CHANGELOG.md
📝 WalkthroughWalkthrough스케줄러가 실행 호스트별 credential을 선택하고 Actions 실행을 조회하거나 취소합니다. 중앙 dispatch는 정확한 제목으로 중복 실행을 판별하고 비권위 대상 실행 정리를 생략합니다. 병합 mutation은 draft PR을 거부합니다. 워크플로 린터와 Strix fallback 동작도 갱신합니다. Changes워크플로와 스케줄러 강화
Estimated code review effort: 4 (Complex) | ~60 minutes Merge Risk: 🟡 Moderate · up to The current changes can still allow workflows with unsupported concurrency settings, introduce a dependency that conflicts with repository licensing policy, and cause the Strix gate to miss Python files during analysis; these issues should be fixed or explicitly accepted before merging. Sequence Diagram(s)sequenceDiagram
participant Scheduler
participant CentralDispatch
participant TargetRepository
participant MergeMutation
Scheduler->>CentralDispatch: 정확한 dispatch 제목과 head SHA로 실행 확인
CentralDispatch-->>Scheduler: 일치하는 실행 반환
Scheduler->>TargetRepository: 중앙 dispatch이면 old-head 실행 정리 생략
Scheduler->>MergeMutation: 병합 또는 자동 병합 요청
MergeMutation-->>Scheduler: draft PR이면 RuntimeError 반환
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 72.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 75 functions across 10 files. (7 skipped: 7 unsupported.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…-read-token # Conflicts: # CHANGELOG.md # tests/test_pr_review_autofix_nvidia_nim_contract.py
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…duler-actions-read-token
…-read-token # Conflicts: # CHANGELOG.md
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
* fix(ci): lint modern Actions schemas safely * fix(ci): preserve multiline workflow diagnostics * test(ci): bound Ruby runtime coverage * fix(ci): preserve deterministic workflow evidence
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
Devin finding "Repository casing blocks valid automation" — confirmed and fixedVerified against the exact PR head ( Fix: both functions now Regression test ( Note: I found the identical case-sensitive- Devin finding "Supported shells escape correct validation" — confirmed and fixedVerified against Fix: Regression tests (
Both verified to fail against the pre-fix functions (posix-vs-bash mismatch; 0 shfmt records instead of 2) and pass against the fix. Other unresolved Devin threads on this PR (not in scope, no action taken)
Validation evidence
Pushed to _Generated by Claude Code Generated by Claude Code |
|
Generated by Claude Code |
seonghobae
added
area: ci-cd
ChatGPT Codex Connector
Merge-conflict triage: genuine architectural divergence (
|
…test-bug fix Follow-up to this same PR's original entry: this round's continued PR sweep found #1065 and #1681 conflicting on strix.yml/noema_review_gate.py (same pattern as the 7 PRs already documented), plus #1271 and #1231 conflicting on scripts/ci/pr_review_merge_scheduler.py -- confirming the #1803 facade/core split is now also an active collision surface (4,074-line monolith on each PR's branch vs. a 241-line facade + separately-evolving core file on main). Evidence-based comments were left on all 4; no guessed resolution was pushed. Also records one genuine pre-existing (not merge-caused) test bug found and fixed while merge-repairing #1655: a jq trailing-newline off-by-one in a new E2E test, and a coverage gap in opencode_review_normalize_output.py's new needs-info wrapper (branches only exercised via subprocess, invisible to coverage.py). Both fixes are test-only, pushed as part of #1655 itself. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
main replaced the 4063-line scheduler monolith this branch was built on with a 241-line facade plus a 6222-line core, so all four scheduler conflicts had ours 656/1322/611/359 lines against theirs 2/17/2/15. That is the #1009 shape, but it is relocatable here, not a re-implementation: all 16 functions the branch's hunks anchor on still exist in pr_review_merge_scheduler_core.py. Method: take main's facade, then replay the branch's own diff against the core module (git apply --3way with the path rewritten). 15 of 17 hunks landed cleanly. The two that did not were placement collisions, resolved by applying the branch's intent at core's existing sites instead of at the branch's: - core already owns active_workflow_runs (23 references vs the branch's 4) and had grown server-side filters and memoization. Kept core's function and rerouted its single call to run_github_actions_for_repository(repo, args), which is what the branch's version of that function did. - core calls cancel_stale_pr_runs at a different point in inspect_pr than the branch did. Wrapped core's call site with the branch's repository_dispatch_target(repo).casefold() == repo.casefold() guard. Other files: - .github/workflows/pr-review-merge-scheduler.yml: main deleted the entire 684-line org-queue-sweep job and pins its absence (assert_file_not_contains 'org-queue-sweep'). Took main's side. Provenance checked: org-queue-sweep is 4x at the merge base and 4x on the branch, 0x on main, and the branch's own change to this file is only +2 lines, so nothing the branch authored is lost with the job. One of those two lines (SCHEDULER_WORKFLOW_TOKEN) belonged to org-queue-sweep; the other had already auto-merged into the job main kept. - tests/test_opencode_agent_contract.py: the branch pinned count('SCHEDULER_WORKFLOW_TOKEN: ...') == 2. One of those two occurrences was org-queue-sweep's, so the merged tree has 1. Changed the pin to 1 with the reason in a comment. - tests/test_pr_review_autofix_nvidia_nim_contract.py: REVIEW_DISPATCH_BLOB_SHA pins git hash-object on opencode-review-dispatch.yml. Both sides are correct for their own tree and both wrong for this merge (branch ea13d12, main 26e8555). Recomputed from the merged file: d7f7c18. - tests/test_pr_review_merge_scheduler.py: kept both sides (main's 337 lines and the branch's 32). The conflict boundary falls between complete top-level functions. - CHANGELOG.md: kept both entries. Evidence: - uvx ruff check --select F821 scripts/ci tests: All checks passed - full suite, branch head b297581 (unmerged): 1920 passed, 0 failed - full suite, this merge: 2919 passed, 0 failed - PR's own oracle (test_lint_github_workflows.py, test_pr_review_merge_ scheduler.py, test_pr_review_autofix_nvidia_nim_contract.py): 372 passed - coverage: TOTAL 100%; pr_review_merge_scheduler_core.py 2557/2557 100%. With fail_under = 100 this is the independent proof that both relocated edits are reached, not merged-in-but-dead. - negative control: deleting the SCHEDULER_WORKFLOW_TOKEN workflow line makes test_merge_scheduler_uses_escalating_mutation_credentials fail by name - interrogate: PASSED (minimum 100.0%) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
main 병합 완료 (
|
|
Same root cause already traced today on Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com Generated by Claude Code |
|
Not this PR's diff. Corroborating evidence for the Already investigating and (if confirmed) fixing this at the source in Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com Generated by Claude Code |
New tooling RED: GitHub
|
seonghobae
left a comment
seonghobae
left a comment
There was a problem hiding this comment.
Exact-head toolchain finding for 99a1a481871f26ee1793338fe401d6373d85281e: current scripts/ci/lint_github_workflows.rb passes actionlint -ignore with QUEUE_DIAGNOSTIC for concurrency.queue.
That implementation predates the now-verified upstream state and conflicts with comment 5565056754's acceptance boundary. GitHub documents queue: max, but actionlint v1.7.12 is still the latest release; upstream issue #657 and PR #654 remain open/unmerged. The local Ruby validator does not make suppressing the released validator's schema error an acceptable protected-main toolchain contract.
Preserve the other #1247/#1231 deltas, but do not land this diagnostic suppression or an unmerged upstream parser. Keep queue:max consumers Proposed/unapplied until upstream native support is merged and released; then pin the immutable released actionlint version and prove positive/negative fixtures, including rejection of queue:max + cancel-in-progress:true. The existing unresolved dynamic-cancellation finding remains valid independently.
GitHub documents concurrency.queue, but actionlint 1.7.12 rejects a minimal queue: max workflow with unexpected key "queue" (exit 1), so the documented syntax cannot pass the governed validator today. Verified the cause at the source rather than inferring it from the error string: actionlint v1.7.12's Concurrency struct in ast.go models only Group and CancelInProgress, with no queue field, so the key is unknown to it by construction. The entry says so explicitly and attributes the observed error text and exit code to the reproduction reported on #1913, because actionlint is not installed in this container and I did not run it. The entry states that this is validator-schema lag rather than evidence against the feature, and that it is not permission to suppress, downgrade or exempt actionlint. queue: max stays available to a lossless release/deploy/migration lane only once the canonical toolchain can validate it, and only with target locking plus protected head/tag/version/digest verification and idempotency. Toolchain ownership is #1231; naruon#1586 owns release/deploy ordering and did not push the rejected key. Guidance only, no workflow change. Gate: 2975 passed, 1 skipped, 21 subtests, coverage 100%, interrogate 100%. Refs #1231, #1586. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
Four related repairs, all documentation and comments; no workflow or census implementation, and no claim that any queue or 60-job definition of done is complete. docs/doctoring/actions-queue-saturation-hourly-sweep.md still read "Status: active repair evidence" with canonical repair #1630 and described the 15-minute-to-hourly sweep change as a live production contract. #1878 (merge 1b65dbc) deleted the org-queue-sweep source outright and 85f4ef9 folded head coalescing into the scheduler; both are ancestors of main@c9052e607 and no such job remains. It is now marked HISTORICAL / SUPERSEDED with those pointers, RCA kept intact. audit_org_codeql_coverage.py cited that stale document as current evidence for real queue congestion. The 35-day constant is unchanged and its rationale stands; only the pointers are corrected, split explicitly into HISTORICAL records for the removed sweep and #1938's BOUNDED LATER EVIDENCE, with a note that neither proves current org-wide occupancy and that this constant is not derived from one. The triage catalog stated "55 of 60 running jobs" with no method and no scope. It now carries both: a running-slot count is a count of jobs and never of runs, so actions/runs?status=in_progress and queued counts are not occupancy numerators; admit a job only when status is in_progress AND it holds a real runner assignment, keeping queued work as separate backlog. The three-repository sample is labelled a bounded precedent rather than an org census, and the shared-ceiling phrasing is bound to its 2026-09-04 reading. Verified independently rather than assumed: no script under scripts/ci/ reads runner_id or runner_name, and audit_org_codeql_coverage.py is a CodeQL coverage census. Org-wide occupancy is recorded as an open collection gap instead of being implied by a sample. Gate: 2975 passed, 1 skipped, 21 subtests, coverage 100%, interrogate 100%. Refs #1878, #1630, #1938, #1231. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BV96rXhqoR3tYZ9AeAVur4
3 participants
Current status — Proposed / decomposition required
This historical branch remains open so no valid delta is lost. It is intentionally Draft because exact head
99a1a481871f26ee1793338fe401d6373d85281eis 23 commits ahead and 38 commits behind protectedmain@78a4937c684a54ca8e415822c913742f41c6efc4, is conflicted, and combines 17 files from several independent responsibilities. Earlier test counts, approvals, and required-workflow receipts belong to predecessor heads and are not current admission evidence.Verified successor carryover
The following bounded responsibilities now have current-base RED→GREEN successors:
8cc62ce…, GREEN7bf3451a…github.tokenand preserve recorded guidance; REDebcc6715…, GREENe2204eeb…890bac2f…, GREEN14f7c85c…08a16caa…, GREENa42ab003…2d140a84…, GREEN5abc0a02…4fb514db…, GREEN75e9d67c…queue: maxcancellation proof; RED98768d6…, implementation822222ed…, exact-head whitespace repair6d104003…ls | head; RED4d976e8…, implementationfd2a497…, exact-head fixture/pin repair4bea311e…570da463…, GREENde96b8b4…All successors are Draft/Proposed and preserve normal non-force lineage. Runtime Quality is now exact-head GREEN on #2002 and #2004–#2007, while CodeQL and other required receipts remain mixed queued/terminal; no stack node inherits another node's evidence. #2030, #2031, and #2033 generated fresh hosted workflows on their own exact heads; terminal evidence remains required. #1231 is not retired or closed.
Remaining delta ledger
The following historical responsibilities still require current-
maincomparison and, where still valid, independent RED→GREEN successors or a non-destructive semantic restack:main: #1983 merged as74224b2091701a754b2fd1a539833c7a6d50b79e; currentmain@78a4937c684a54ca8e415822c913742f41c6efc4is 5 commits ahead / 0 behind that commit, and the production matcher plus OpenCode/Strix rendered-run-name regressions are present. No duplicate successor is required.queue: maxcompatibility, workflow provisioning, and license-boundary changes are preserved in Proposed successor fix(autofix): fail closed on ambiguous workflow validation #2030; they remain incomplete until its exact-head checks are terminal and the delta is integratedmain: #1331 commit33dc57d7984b937e4f5ab915d5eae69a0f42e3a5and #1353 commit874f47b3856ca6bdb6bc71d48173b8f34ba7b9caare ancestors of current main (752 and 743 commits ahead / 0 behind respectively). The production gate maps both direct-OpenAI aliases, isolates fallback credentials/API base, and has permanent regression coverage. No duplicate successor is required.mapfilerewrite was authored in131f494922c7ac9336195d1facebb705e5ed75cbas an implementation-level companion to the large-shell linter repair. Current main preserves the semantic contract—LC_ALL=Cpathname sorting, exclusion ofSHA256SUMS, and per-filesha256sum—with the shorter pipeline; no state escapes the loop subshell and no consumer contract depends onmapfile. Reintroducing the temporary-file/array shape would add machinery without restoring a valid missing behavior, so no duplicate successor is required.The live unresolved thread on dynamic
cancel-in-progressbesidequeue: maxremains substantive and intentionally unresolved. The former #1213 dependency is historical and cannot be used as current merge authority.No reviewer, required check, scanner, permission, ruleset, or cross-repository mutation authority is weakened.