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.
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
OpenCode could not approve from deterministic current-head evidence because GitHub Checks have failed.
Findings
1. HIGH Current-head GitHub Checks - Fix failed required checks before approval
- Problem: Failed same-head checks remain for
7b16617af04431a43f8f7528b8ac7db345e404a7. - Root cause: The model-unavailable evidence fallback is allowed only when peer GitHub Checks are complete and clean.
- Fix: Read and fix the failed check logs below, then rerun the current-head checks.
- Regression test: Keep the model-unavailable fallback gated on an empty failed-check rollup.
Failed checks:
- Strix Security Scan/strix: FAILURE (https://github.com/ContextualWisdomLab/.github/actions/runs/32642826932/job/97202440083)
- Strix Security Scan/strix: failure (https://github.com/ContextualWisdomLab/.github/actions/runs/32642826932/job/97202440083)
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Workflow (4 files)"]
S1 --> I1["GitHub Actions review job"]
I1 --> R1["Review risk: Workflow (4 files)"]
R1 --> V1["actionlint plus required checks"]
Evidence --> S2["Changed file: CHANGELOG.md"]
S2 --> I2["repository behavior"]
I2 --> R2["Review risk: Changed file: CHANGELOG.md"]
R2 --> V2["required checks"]
Evidence --> S3["Docs (3 files)"]
S3 --> I3["operator or user guidance"]
I3 --> R3["Review risk: Docs (3 files)"]
R3 --> V3["docs review"]
Evidence --> S4["CI script (2 files)"]
S4 --> I4["review and security gate shell path"]
I4 --> R4["Review risk: CI script (2 files)"]
R4 --> V4["bash -n plus Strix self-test"]
Evidence --> S5["Test (8 files)"]
S5 --> I5["regression suite"]
I5 --> R5["Review risk: Test (8 files)"]
R5 --> V5["targeted test run"]
…-read-token # Conflicts: # .github/workflows/opencode-review-dispatch.yml # CHANGELOG.md # docs/doctoring/strix-nvidia-nim-not-found-fallback.md # tests/test_pr_review_autofix_nvidia_nim_contract.py # tests/test_strix_nvidia_nim_not_found_fallback.py
|
Resolved the stale merge conflict against current 5 conflicting files, all mechanical/superseded-content — no unresolved product decision:
Test evidence (worktree, head
Pushed as merge commit Generated by Claude Code |
| def validate_concurrency_queue!(path, label, concurrency) | ||
| return unless concurrency.is_a?(Hash) && concurrency.key?("queue") | ||
| unless concurrency["queue"] == "max" | ||
| raise WorkflowLintError, | ||
| "#{path}: #{label} concurrency queue must be exactly max, got #{concurrency['queue'].inspect}" | ||
| end | ||
| return unless concurrency["cancel-in-progress"] == true | ||
|
|
||
| raise WorkflowLintError, | ||
| "#{path}: #{label} concurrency queue max requires cancel-in-progress to be false or absent" |
There was a problem hiding this comment.
🔍 Dynamic cancellation policy remains ambiguous
validate_concurrency_queue! accepts expression-valued cancellation beside queue: max. Define whether dynamic expressions require rejection or proof of false before relying on this compatibility gate.
Was this helpful? React with 👍 or 👎 to provide feedback.
… shell detection Devin findings on PR #1231: 1. scripts/ci/pr_review_merge_scheduler.py: same_repository_head and compare_ref_for_pr_head compared headRepository.nameWithOwner to the configured target repo with exact case-sensitive equality, even though GitHub repository identity is case-insensitive and a sibling stale-run-cancellation path in this same file already case-folds. A same-repository PR whose GitHub-reported canonical name differed only in case from the configured target lost branch updates and automated merge eligibility. Fixed both to casefold consistently with the existing normalization idiom used elsewhere in the file. Added a regression covering same_repository_head, compare_ref_for_pr_head, and the end-to-end inspect_pr update_branch decision with a differently-cased headRepository. 2. scripts/ci/lint_github_workflows.rb: effective_shell ignored job-level `container:`, which defaults a step's shell to sh (not bash) per GitHub Actions semantics when no shell is configured anywhere in the resolution chain — so Bash-only syntax in a container job's default shell could pass validation as if it were bash. shellcheck_dialect also only recognized the bare names "bash"/"sh", skipping absolute-path custom shell templates (e.g. "/bin/bash --noprofile --norc -eo pipefail {0}", "/usr/bin/sh {0}") entirely rather than classifying them. Fixed by defaulting containerized jobs' shell to sh, and by classifying shellcheck_dialect off the executable's basename so absolute paths resolve the same as bare names. Added regression tests for both: a container job with no explicit shell (asserts shfmt is invoked with the posix dialect) and absolute-path bash/sh templates (asserts shfmt is invoked at all, with the correct dialect). All four changes verified against the pre-fix code: each new/extended test fails on the unpatched function and passes after the fix. Evidence: PYTHONPATH=. python3 -m pytest tests -q -> 1920 passed, 1 skipped, 21 subtests passed; coverage 100% statements/branches on scripts/ci; interrogate 100% docstrings; ruby -c clean; git diff --check clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KPmJErfkcHer4UVEgrQxUX
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.
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 36 commits behind protectedmain@c9052e607e5f3cc76e73207e7786b21500721b79, 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…All successors are Draft/Proposed and preserve normal fast-forward lineage. #2004 through #2007 each generated all five hosted workflow runs, including stacked Python Security and Runtime Quality, but those runs remain queued and are not GREEN evidence. #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@c9052e607e5f3cc76e73207e7786b21500721b79is 3 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 changesThe 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.