Skip to content

ci(concurrency): scope noema/strix/opencode workflows by PR number - #1787

Closed
seonghobae wants to merge 11 commits into
mainfrom
autoresearch/sep03
Closed

ci(concurrency): scope noema/strix/opencode workflows by PR number#1787
seonghobae wants to merge 11 commits into
mainfrom
autoresearch/sep03

Conversation

@seonghobae

@seonghobae seonghobae commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

변경 요약\n- noema-review, strix, opencode-review-dispatch 워크플로우의 concurrency 그룹에서 run_id 폴백을 제거\n- 그룹 형식을 기준으로 통일\n- 동일 PR push/재분배 시 구형 실행이 같은 그룹에서 cancel-in-progress 되도록 정합화\n\n요구사항 항목 13(동일 PR 재실행 정합성) 및 전체 동시성 규칙의 핵심 워크플로우부터 반영.


Devin Review

…s (minute 44) callers

- Thin hourly callers delegate to pr-review-fix-scheduler with develop
  bases, single dispatch, and two-hour same-head retry floors.
- appguardrail fills its long-reserved roster slot; macos_utility_packs
  takes a fresh non-colliding minute.
- Doctoring records activation prerequisites incl. the
  OPENCODE_REPOSITORY_DISPATCH_TARGETS allowlist entries (set at org
  level in the same change window) with APA 7th references.
- Focused contract tests mirror the established trio; quality gate
  pull/push/compileall path blocks track all six files.
…provider base URL

resolved_llm_api_base_for_model had cross-provider handling only for
github_models/* fallbacks. When the primary provider was nvidia_nim (or
any provider that sets LLM_API_BASE), openai-direct/gpt-5.4 fallback
attempts inherited the NVIDIA inference endpoint while carrying the
OpenAI key, so litellm posted to the wrong host and every attempt died
with 'Error: 404 page not found' (STRIX_PROVIDER_UNAVAILABLE). Direct
OpenAI attempts now resolve to no custom base so litellm routes to
api.openai.com by default (#1327).
@coderabbitai

coderabbitai Bot commented Sep 3, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 31 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Team

Run ID: e32b6c8e-a5e2-4616-83c0-487879aa761a

📥 Commits

Reviewing files that changed from the base of the PR and between 0e195d3 and 8da772e.

⛔ Files ignored due to path filters (1)
  • uv.lock is excluded by !**/*.lock
📒 Files selected for processing (37)
  • .github/workflows/agent-mention-router-quality-ci.yml
  • .github/workflows/agent-mention-router.yml
  • .github/workflows/appguardrail-hourly-review-repair.yml
  • .github/workflows/close-empty-pr.yml
  • .github/workflows/cloudflare-dns.yml
  • .github/workflows/codeql-pr.yml
  • .github/workflows/deploy-pages.yml
  • .github/workflows/exact-artifact-sbom-attestation-quality.yml
  • .github/workflows/exact-artifact-sbom-attestation.yml
  • .github/workflows/hourly-nvidia-nim-review-repair.yml
  • .github/workflows/javascript-coverage-quality-ci.yml
  • .github/workflows/macos_utility_packs-hourly-review-repair.yml
  • .github/workflows/noema-review.yml
  • .github/workflows/opencode-review-dispatch.yml
  • .github/workflows/opencode-review.yml
  • .github/workflows/opencode-rust-coverage-toolchain-quality-ci.yml
  • .github/workflows/organization-commercial-readiness-loop-quality-ci.yml
  • .github/workflows/osv-scanner-pr.yml
  • .github/workflows/pr-review-merge-scheduler.yml
  • .github/workflows/python-security.yml
  • .github/workflows/repair-pr827-coderabbit-comments.yml
  • .github/workflows/sast-semgrep.yml
  • .github/workflows/sbom-generation.yml
  • .github/workflows/scheduled-security-scan.yml
  • .github/workflows/scorecard-analysis.yml
  • .github/workflows/scorecard-pr.yml
  • .github/workflows/secret-scan.yml
  • .github/workflows/security-scan.yml
  • .github/workflows/strix-changed-path-quality-ci.yml
  • .github/workflows/strix.yml
  • .github/workflows/trusted-uv-materializer-quality-ci.yml
  • docs/doctoring/appguardrail-hourly-review-caller.md
  • docs/doctoring/macos-utility-packs-hourly-review-caller.md
  • scripts/ci/strix_quick_gate.sh
  • tests/test_appguardrail_hourly_review_caller.py
  • tests/test_macos_utility_packs_hourly_review_caller.py
  • tests/test_pr_review_autofix_nvidia_nim_contract.py

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 8 potential issues.

Devin Review

github.event.client_payload.target_repository || github.repository }}-${{
github.event.client_payload.pr_number && format('pr-{0}', github.event.client_payload.pr_number) ||
github.run_id }}
group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event.client_payload.pr_number || github.event.pull_request.number || github.event.workflow_run.pull_requests[0].number }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Review dispatches for different repos with the same PR number cancel each other

For repository_dispatch, github.repository is always the central .github repo, and the group no longer includes client_payload.target_repository. Two target repos reviewing the same PR number now share one group, so cancel-in-progress kills the earlier review.

Suggested change
group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event.client_payload.pr_number || github.event.pull_request.number || github.event.workflow_run.pull_requests[0].number }}
group: ${{ github.workflow }}-${{ github.event.client_payload.target_repository || github.repository }}-${{ github.event.client_payload.pr_number || github.run_id }}
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +70 to +71
# PR-number scope keeps the queue on the current HEAD.
group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event.pull_request.number || github.event.client_payload.pr_number || github.event.workflow_run.pull_requests[0].number }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Strix scans on different branches and the weekly schedule cancel each other

Strix has no workflow_run trigger, so push and schedule runs leave every number term empty and share the constant group <workflow>-<repo>-. Pushes to main, develop, and master and the weekly scan then cancel each other, and a merge-time strix-scan dispatch can cancel the required pull_request_target run for the same PR.

Prompt for agents
The new concurrency group drops github.event_name and github.ref. strix.yml has no workflow_run trigger, so the github.event.workflow_run.pull_requests[0].number term is always empty. For push and schedule events (and any event without a PR number) the group collapses to a single constant string per repository, so pushes to different protected branches and the weekly scheduled scan all cancel each other under cancel-in-progress: true. Separately, merging pull_request_target and repository_dispatch into the same PR-number group re-introduces the exact problem the removed comment warned about: a default-branch strix-scan retry can cancel the required pull_request_target Strix context read by branch protection. Restore an event-class and/or github.ref (or github.run_id) fallback so non-PR events and different event classes do not share one group.
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


concurrency:
group: python-security-${{ github.event.pull_request.base.repo.full_name || github.repository }}-${{ github.event.pull_request.number || github.ref }}
group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event.pull_request.number }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Push and scheduled security scans across branches cancel one another

The group dropped its github.ref fallback, so every non-PR event (push to main, master, or develop, the weekly schedule, and repository_dispatch) resolves to the constant <workflow>-<repo>-. With cancel-in-progress they share one group and cancel each other. Identical regression in secret-scan and sast-semgrep.

Suggested change
group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event.pull_request.number }}
group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event.pull_request.number || github.ref }}
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +37 to +38
group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event.pull_request.number }}
cancel-in-progress: true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔴 Cloudflare apply can be cancelled mid-reconcile by a later run

For push and repository_dispatch there is no PR number, so every such run shares the constant group, and cancel-in-progress was flipped from false to true. A push dry-run or a second apply now cancels an in-flight mode=apply reconcile, which can leave DNS records partially written.

Suggested change
group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event.pull_request.number }}
cancel-in-progress: true
group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: false
Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

github.event_name == 'workflow_run' && github.event.workflow_run.pull_requests[0].number && format('pr-{0}', github.event.workflow_run.pull_requests[0].number) ||
github.event_name == 'repository_dispatch' && github.event.client_payload.pr_number ||
github.run_id }}
group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event.pull_request.number || github.event.client_payload.pr_number || github.event.workflow_run.pull_requests[0].number }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Noema review may cancel across event types for the same PR

The old group included github.event_name; the new one merges pull_request_target, workflow_run, and repository_dispatch for a PR into one group. A retry dispatch or workflow_run completion can cancel an in-progress required pull_request_target review. Additionally, workflow_run events with an empty pull_requests array leave every number term empty and collapse to the constant <workflow>-<repo>- group.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.


concurrency:
group: sbom-generation-${{ github.event.pull_request.base.repo.full_name || github.repository }}-${{ github.event.pull_request.number || github.event.release.tag_name || github.ref }}
group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event.pull_request.number }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Concurrent releases cancel each other's SBOM upload

Release events carry no PR number, so the group collapses to the constant <workflow>-<repo>- after the release.tag_name fallback was removed. Two releases published close together share it, and cancel-in-progress aborts the earlier SBOM and release-asset upload.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

contents: read

concurrency:
group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event.pull_request.number || github.event.workflow_call.head_repository.full_name || github.run_id }}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Info: deploy-pages concurrency references a non-existent context property

github.event.workflow_call.head_repository.full_name is not a real context property, so it always resolves empty and the group falls through to github.run_id. Harmless because run_id makes each call unique, but the middle term never contributes.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +90 to +91
group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event.pull_request.number || github.event.workflow_run.pull_requests[0].number || inputs.pr_number || github.event.client_payload.pr_number || github.run_id }}
cancel-in-progress: true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Merge scheduler cancel-in-progress made unconditional

cancel-in-progress is now always true, where the old key excluded schedule and org-sweep runs. The retained github.run_id fallback gives those events unique groups, so behavior is preserved. Confirm that repository_dispatch runs with a pr_number for different target repos do not collide, since github.repository is the constant .github and the old key included target_repository.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@seonghobae

Copy link
Copy Markdown
Contributor Author

병합 반대 — 여러 파일에서 오늘 이미 고친 설계를 되돌리고, 실제 운영 위험을 새로 만듭니다

전체 diff(~30개 워크플로우 파일)를 직접 읽어봤습니다. {workflow}-{repository}-{PR번호} 형식을 모든 파일에 기계적으로 동일하게 적용했는데, 각 파일의 실제 트리거 형태를 고려하지 않아 여러 곳에서 심각한 문제가 생깁니다.

1. cloudflare-dns.yml — 실제 운영 인프라 안전성 회귀 (가장 심각)

  • 기존: group: cloudflare-dns-${{ github.ref }} + cancel-in-progress: false
  • 변경: group: ${{ github.workflow }}-${{ github.repository }}-${{ github.event.pull_request.number }} + cancel-in-progress: true
  • 이 워크플로우는 push(main 반영)와 repository_dispatch(실제 mode=apply Cloudflare 쓰기)에서도 트리거되는데, 두 이벤트 모두 pull_request.number가 없습니다. 즉 모든 push/dispatch 실행이 하나의 그룹(끝이 빈 값)을 공유하게 되고, cancel-in-progress: true로 인해 새 push/dispatch가 들어오면 진행 중인 실제 Cloudflare DNS apply가 중간에 취소될 수 있습니다. curl+jq로 순차 반영하는 스크립트가 중간에 잘리면 DNS 레코드가 부분 반영 상태로 남을 수 있는 실제 프로덕션 리스크입니다. cancel-in-progress: false를 되돌린 이유가 원래 주석에 있습니다.

2. opencode-review.yml — 오늘 오전에 고친 데드락 버그를 그대로 되살립니다

  • .github#1786(오늘 병합, squash 8122fff)에서 이 파일의 concurrency를 workflow-level에서 job-level(opencode-review-target에만 한정)로 옮긴 이유는: workflow-level에 두면 cancel-superseded-opencode-review-runs cleanup job까지 같은 그룹에 묶여서, 응답 시간에 상한이 없는(Model timeout 기본 null) 오래된 review가 자기 자신의 취소 메커니즘까지 막아버리는 데드락이 생기기 때문입니다.
  • 이 PR은 on: 블록 바로 뒤(workflow-level 위치)에 다시 concurrency:를 넣어서, 오늘 고친 위치 자체를 되돌립니다. Devin Review가 캐치했던 그 데드락이 재발합니다.

3. codeql-pr.yml — 몇 시간 전 병합한 재작성(.github#1778)의 근거 있는 설계를 무시

  • 기존 group에 head SHA를 넣지 않은 이유가 코드 주석에 명시돼 있습니다: 이 파일은 cancel-superseded-* 같은 전용 cleanup job이 없어서, 이 group의 cancel-in-progress: true 자체가 PR close 시 유일한 취소 수단이기 때문입니다. 이 PR의 새 group도 우연히 여기서는 동작은 하겠지만, 그 근거를 이해하고 반영한 흔적이 diff에 없습니다.

4. strix.yml — event-class 분리 제거

  • 기존 주석: "Include the event name so default-branch repository_dispatch evidence cannot cancel the required pull_request_target Strix context that branch protection reads." — pull_request_targetrepository_dispatch를 의도적으로 분리해서 서로 다른 이벤트 클래스의 실행이 서로를 취소하지 못하게 막아뒀는데, 이 PR은 github.event_name을 group에서 아예 빼버려서 그 분리를 없앱니다.

5. pr-review-merge-scheduler.yml — 가장 광범위한 손실, 그리고 사용자의 명시적 원칙 위반

  • 기존 group은 pull_request_target/pull_request_review/workflow_run(PR 유무 분기)/workflow_call(pr_number 또는 base_branch 분기)/schedule/repository_dispatch(org_sweep/target_repository/pr_number 세분화) 11개 분기를 갖고 있었고, cancel-in-progress도 이벤트별로 다르게 조건화돼 있었습니다(schedule/workflow_call은 취소 안 함).
  • 이 PR은 이를 전부 하나의 평평한 표현식 + 무조건 cancel-in-progress: true로 대체합니다. 이건 이번 loop brief의 일반지침 8.2에 명시된 사용자 본인의 원칙 — "merge·release·deploy·migration은 취소하지 않고 lock·idempotency·exact-head로 직렬화한다" — 을 정확히 위반합니다. 이 파일은 실제로 PR 병합을 수행하는 스케줄러 그 자체입니다.

6. noema-review.yml — 동일한 event-class 분리 제거 패턴 반복

요청

이 PR은 사용자의 "concurrency group을 {workflow}-{repository}-{PR번호} 형식으로"라는 지시의 의도(같은 PR의 구형 실행이 새 실행을 방해하지 않게)는 맞지만, 이를 모든 파일에 무차별 find-replace로 적용하면서 각 파일이 실제로 처리해야 하는 non-PR 트리거(push/schedule/workflow_call/repository_dispatch)와 이미 검증된 개별 설계를 깨뜨립니다. 병합 전에 최소한 위 6개 파일은 각자의 트리거 형태에 맞게 다시 설계해야 합니다 — 특히 cloudflare-dns.yml은 실제 DNS 안전성 문제라 우선순위가 가장 높습니다.

작업을 막지 않기 위해 draft로 전환해뒀습니다. 필요하면 제가 각 파일을 개별적으로 다시 작업하는 것도 도울 수 있습니다.

@seonghobae
seonghobae marked this pull request as draft September 3, 2026 08:49
@seonghobae

Copy link
Copy Markdown
Contributor Author

추가로 하나 더: 방금(.github#1788, #1750) exact-head-path-policy를 조직 전체에서 실패시키던 stale bash assertion 두 건을 고쳤는데, 그 assertion들이 지금 strix.ymlformat('{0}-{1}-{2}', ...) 리터럴과 pr-review-merge-scheduler.yml의 cron 리터럴을 정확히 문자열로 검사합니다. 이 PR이 지금 형태로 병합되면 format(...) 자체를 다른 표현식으로 대체하기 때문에, 방금 고친 것과 같은 종류의 조직 전체 required-check 회귀가 즉시 재발합니다 — 이 PR을 다시 작업할 때 scripts/ci/test_strix_quick_gate.sh의 해당 assertion도 같은 PR 안에서 함께 갱신해야 합니다.

seonghobae added a commit that referenced this pull request Sep 3, 2026
…catch

The ROPC config fix (79fe43d) exposed a real bug it was masking: password
registration created accounts with no way to authenticate once Direct
Access Grants was off. Fixed (44f0cb9): endpoint fails closed behind a
flippable module constant, validate_realm.py now guards against a silent
re-enable, ADR-0015 correction added. Also recorded independently
confirming a peer's catch that .github#1787 (stale, pre-dates the
already-landed noema-review.yml concurrency-deadlock fix) would have
reverted it if merged -- now in draft pending rebase.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@seonghobae

seonghobae commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

Close (delta-none) — 결론 공유

두 세션이 각자 독립적으로 이 PR이 건드리는 모든 파일을 검증한 결과, 예외 없이 동일한 결론에 도달했습니다: main은 이미 이 PR이 하려는 일(같은 PR의 구형 실행만 취소)을 각 파일의 실제 트리거 형태에 맞게 더 올바르게 구현하고 있고, 이 PR은 그걸 되돌리는 순수 회귀입니다.

확인된 파일:

  • cloudflare-dns.yml, opencode-review.yml, codeql-pr.yml, strix.yml, pr-review-merge-scheduler.yml (제가 확인)
  • noema-review.yml, opencode-review-dispatch.yml (host 1 세션이 확인 — cross-repo repository_dispatch 케이스에서 target_repository를 안 쓰고 github.repository를 쓰는 바람에 서로 다른 저장소의 PR 번호끼리 충돌하는 정합성 버그까지 추가로 확인)

이 PR에 담긴 유일한 실질 아이디어(workflow-repository-PR번호 리터럴 형식)는 main에 이미 실질적으로(더 정확한 형태로) 반영돼 있습니다. 되살릴 unlanded delta가 없다고 판단해 org의 repair-not-close 원칙(delta 없음/악성 변경/완전 승계에만 close 허용) 중 'delta 없음'에 해당한다고 보고 close합니다. 브랜치는 남겨두니 혹시 다른 관점의 delta가 있다고 판단되면 reopen 해주세요.

@seonghobae seonghobae closed this Sep 3, 2026
seonghobae added a commit that referenced this pull request Sep 3, 2026
…close

Devin's third pass on keyverse#128 found the OpenAPI schema still promised
201-only, and the disable lacked a changelog/operability/doctoring record.
Fixed (86de970). Also caught and self-corrected a citation error before
committing: had drafted named individual RFC editors for RFC 9700/10017,
realized RFC 10017 postdates any available verification and the RFC 9700
author list wasn't independently confirmable either, replaced both with
organizational-author citations. Recorded .github#1787's resolution: two
peer sessions cross-checked all 8 touched files independently, found the
same "pure revert of already-correct work" pattern in every one, closed
under the delta-none exception.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Sign up for free to 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