feat(automation): coordinate hourly organization readiness - #832
feat(automation): coordinate hourly organization readiness#832seonghobae wants to merge 36 commits into
Conversation
📝 WalkthroughWalkthrough시간별 중앙 워크플로와 상업 준비성 코디네이터를 추가했습니다. 코디네이터는 저장소 상태와 writer lease를 검증하고, 제한된 리뷰 복구 및 제품 개발 작업을 디스패치합니다. Python 테스트와 품질 CI가 정책, 오류 처리, 출력 형식, 커버리지를 검증합니다. Changes조직 상업 준비성 코디네이터
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant HourlyWorkflow
participant Coordinator
participant GitHubAPI
participant RepositoryWorkflow
HourlyWorkflow->>Coordinator: 예약 실행
Coordinator->>GitHubAPI: 저장소, 워크플로, 실행, PR 조회
Coordinator->>GitHubAPI: 디스패치 직전 상태 재조회
Coordinator->>RepositoryWorkflow: 리뷰 복구 또는 제품 개발 디스패치
Coordinator->>HourlyWorkflow: 실행 보고서 생성
Possibly related issues
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review @opencode-agent review @cwl-noema-review Please review exact current head |
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
scripts/ci/organization_commercial_readiness_loop.py (3)
726-731: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value함수 이름이 동작과 일치하지 않습니다.
_positive_int는 0을 허용합니다. 오류 문구도 "value must be zero or greater"입니다._non_negative_int로 이름을 바꾸면 의도가 명확해집니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/ci/organization_commercial_readiness_loop.py` around lines 726 - 731, Rename the _positive_int argument parser helper to _non_negative_int so its name reflects that zero is valid, and update every reference to the helper accordingly. Preserve its current validation and error message behavior.
762-786: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win모든 저장소 검사가 실패해도 종료 코드가 0입니다.
run_once는inspection_errors와dispatch_failed를 보고서에만 기록합니다.main은 항상 0을 반환합니다. 토큰이 만료되거나 조직 전체 API가 실패하면 매시간 실행이 성공으로 표시됩니다. 운영자는 실패를 인지하지 못합니다.선택된 저장소가 있는데 스냅샷이 하나도 성공하지 않은 경우, 또는 계획된 모든 디스패치가
dispatch_failed인 경우에 비영(非零) 종료 코드를 반환하십시오. 부분 실패는 현재대로 0을 유지해도 됩니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/ci/organization_commercial_readiness_loop.py` around lines 762 - 786, Update main’s result handling after run_once to return a nonzero exit code when repositories were selected but none produced a successful snapshot, or when every planned dispatch is marked dispatch_failed. Preserve exit code 0 for successful runs and partial failures, and use the report fields populated by run_once rather than changing its reporting behavior.
315-363: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftAPI 호출량과 rate limit을 확인하십시오.
이 메서드는 워크플로 1개마다
contents요청을 1회 추가합니다.snapshot()은 대상 저장소마다 이 목록을 조회하고, 계획된 대상은 디스패치 직전에 다시 조회합니다.MAX_REPOSITORIES가 200이고 저장소당 워크플로가 10개이면 한 번의 패스에서 수천 건의 REST 호출이 발생합니다. 사용자 토큰의 시간당 5,000건 한도에 근접합니다. 한도를 초과하면snapshot()이GitHubError를 발생시키고, 모든 저장소가 inspection error로 기록됩니다.writer 신호 이름/경로가 일치하는 워크플로에 대해서만 소스를 가져오면 호출량을 크게 줄일 수 있습니다.
is_dedicated_writer_workflow와is_manual_product_entrypoint는 모두_writer_signal을 요구하므로, 그 외 워크플로의 본문은 정책 판단에 사용되지 않습니다. 다만 이 변경은fingerprint의content_sha의미를 바꾸므로 테스트 갱신이 필요합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/ci/organization_commercial_readiness_loop.py` around lines 315 - 363, Update list_workflows to fetch content only for workflows whose name or path matches the existing writer-signal criteria, reusing is_dedicated_writer_workflow and is_manual_product_entrypoint or the shared _writer_signal logic. Keep metadata collection for all workflows, but leave content and content_sha empty for non-matching workflows; update fingerprint-related tests and expectations to reflect the narrower content_sha coverage..github/workflows/organization-commercial-readiness-loop.yml (1)
61-68: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winJSON 보고서가 실행 종료와 함께 사라집니다.
--json-output은$RUNNER_TEMP에 기록합니다. 러너는 작업 종료 시 이 디렉터리를 삭제합니다. 스크립트 docstring은 "auditable receipts"를 남긴다고 서술합니다. 남는 증적은 job summary 마크다운뿐입니다.JSON을 보존하려면
actions/upload-artifact로 업로드하십시오. 보존 기간을 짧게 설정하면 저장 비용도 제한됩니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/organization-commercial-readiness-loop.yml around lines 61 - 68, Preserve the JSON report generated by the organization commercial readiness loop by adding an actions/upload-artifact step after validation with python -m json.tool. Upload the file from organization-commercial-readiness-loop.json and configure a short retention period so the auditable receipt survives runner cleanup without unnecessary storage.tests/test_organization_commercial_readiness_loop_policy.py (1)
56-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win정책 부정 케이스 두 개가 빠졌습니다.
is_manual_product_entrypoint는NVIDIA_NIM_API_KEY존재와workflow_dispatch트리거를 함께 요구합니다(scripts/ci/organization_commercial_readiness_loop.py:478-493). 현재 변형 목록은 schedule 추가,COPILOT_GITHUB_TOKEN추가, 마커 제거,concurrency제거만 다룹니다. 두 필수 조건이 회귀로 삭제되어도 테스트는 통과합니다.변형 목록에 두 케이스를 추가하십시오.
♻️ 변형 추가 제안
for changed in ( (safe.content or "") + 'schedule:\n - cron: "1 * * * *"\n', (safe.content or "") + "COPILOT_GITHUB_TOKEN: forbidden\n", (safe.content or "").replace("# cwl-org-commercial-entrypoint: v1\n", ""), (safe.content or "").replace("concurrency:\n", ""), + (safe.content or "").replace("NVIDIA_NIM_API_KEY", "OTHER_API_KEY"), + (safe.content or "").replace("on:\n workflow_dispatch:\n", "on:\n push:\n"), ):🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_organization_commercial_readiness_loop_policy.py` around lines 56 - 62, Extend the negative-case variants in the test around is_manual_product_entrypoint by adding cases that remove the NVIDIA_NIM_API_KEY and remove the workflow_dispatch trigger from the workflow content. Keep the existing variants unchanged and assert both new mutations are still rejected.tests/test_organization_commercial_readiness_loop.py (1)
9-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value중복 검증을 정리하면 좋습니다.
이 테스트는
tests/test_organization_commercial_readiness_loop_policy.py의test_static_and_live_writer_lease_policy첫 단언(Line 38)과 동일한 조건을 확인합니다. 또한 다른 테스트는organization_commercial_readiness_fixtures.workflow로 레코드를 만들지만, 이 파일만WorkflowRecord를 직접 생성합니다. 필드가 바뀌면 두 곳을 따로 고쳐야 합니다.이 파일을 제거하고 정책 테스트로 통합하거나, 최소한 공용 fixture 헬퍼를 사용하십시오.
♻️ fixture 헬퍼 사용 예시
-from scripts.ci.organization_commercial_readiness_loop import ( - WorkflowRecord, - is_dedicated_writer_workflow, -) +from organization_commercial_readiness_fixtures import workflow +from scripts.ci.organization_commercial_readiness_loop import ( + is_dedicated_writer_workflow, +) def test_active_scheduled_writer_claims_the_repository_lease() -> None: """An enabled scheduled product writer excludes the generic coordinator.""" - workflow = WorkflowRecord( - workflow_id=1, - name="Hourly Product Development", - path=".github/workflows/hourly-product-development.yml", - state="active", - content_sha="sha-1", - content='on:\n schedule:\n - cron: "37 * * * *"\n', - ) - - assert is_dedicated_writer_workflow(workflow) + record = workflow(content='on:\n schedule:\n - cron: "37 * * * *"\n') + + assert is_dedicated_writer_workflow(record)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/test_organization_commercial_readiness_loop.py` around lines 9 - 20, Remove the duplicate test_active_scheduled_writer_claims_the_repository_lease test and rely on the existing test_static_and_live_writer_lease_policy coverage, or refactor it to use the shared organization_commercial_readiness_fixtures.workflow helper instead of constructing WorkflowRecord directly. Keep the lease-policy assertion covered without maintaining duplicate workflow data.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scripts/ci/organization_commercial_readiness_loop.py`:
- Around line 496-511: Use one organization-derived central repository
identifier consistently: update repository_is_eligible and build_plan to exclude
the same derived name, and pass that organization-specific value through
dispatch_review_repair instead of using a hardcoded ContextualWisdomLab/.github
target. Preserve --organization as a variable CLI parameter and ensure all
central-repository checks and dispatches use it.
- Around line 365-382: Update
OrganizationCommercialReadinessLoop.list_active_runs to paginate each status
query until a page returns fewer than 100 workflow runs, placing the page
parameter after per_page in the URL as `...&page={page}`. Preserve collection of
all returned RunRecord entries and stop paging only when the current page is
incomplete.
In `@tests/test_organization_commercial_readiness_loop_credential_contract.py`:
- Around line 16-20: Extend the assertions in the credential contract test to
verify that the workflow checkout step includes persist-credentials: false. Keep
the existing GH_TOKEN and forbidden-setting checks unchanged, and ensure the
test fails if this checkout credential-isolation setting is removed.
In `@tests/test_organization_commercial_readiness_loop_github.py`:
- Line 37: 테스트 더미 토큰을 검증하는 assert 문에 Ruff S105 억제를 추가하십시오.
`kwargs["env"]["GH_TOKEN"]` 검증은 그대로 유지하고, 해당 줄에 S105 전용 `noqa` 주석을 붙여 린트 경고만
억제하십시오.
- Around line 146-174: Update the fake request handler in the test around fake
and its actions/runs branch to parse the page query parameter and return the
existing workflow run only for page 1, then return an empty workflow_runs list
from page 2 onward. Preserve the current status-based response and assertions so
list_active_runs can exercise pagination and terminate.
In `@tests/test_organization_commercial_readiness_loop_run_pagination.py`:
- Around line 20-21: Update the page parsing in the test to split on the exact
query parameter delimiter "&page=" so per_page cannot be matched, while
preserving integer conversion of the actual page value.
- Around line 51-55: Update list_active_runs to iterate through pagination for
each run status, requesting subsequent pages whenever a page returns 100 records
or otherwise indicates more results. Aggregate all pages into the returned
records so 101 active runs are included and the page=2 request is generated,
while preserving the existing status filtering behavior.
---
Nitpick comments:
In @.github/workflows/organization-commercial-readiness-loop.yml:
- Around line 61-68: Preserve the JSON report generated by the organization
commercial readiness loop by adding an actions/upload-artifact step after
validation with python -m json.tool. Upload the file from
organization-commercial-readiness-loop.json and configure a short retention
period so the auditable receipt survives runner cleanup without unnecessary
storage.
In `@scripts/ci/organization_commercial_readiness_loop.py`:
- Around line 726-731: Rename the _positive_int argument parser helper to
_non_negative_int so its name reflects that zero is valid, and update every
reference to the helper accordingly. Preserve its current validation and error
message behavior.
- Around line 762-786: Update main’s result handling after run_once to return a
nonzero exit code when repositories were selected but none produced a successful
snapshot, or when every planned dispatch is marked dispatch_failed. Preserve
exit code 0 for successful runs and partial failures, and use the report fields
populated by run_once rather than changing its reporting behavior.
- Around line 315-363: Update list_workflows to fetch content only for workflows
whose name or path matches the existing writer-signal criteria, reusing
is_dedicated_writer_workflow and is_manual_product_entrypoint or the shared
_writer_signal logic. Keep metadata collection for all workflows, but leave
content and content_sha empty for non-matching workflows; update
fingerprint-related tests and expectations to reflect the narrower content_sha
coverage.
In `@tests/test_organization_commercial_readiness_loop_policy.py`:
- Around line 56-62: Extend the negative-case variants in the test around
is_manual_product_entrypoint by adding cases that remove the NVIDIA_NIM_API_KEY
and remove the workflow_dispatch trigger from the workflow content. Keep the
existing variants unchanged and assert both new mutations are still rejected.
In `@tests/test_organization_commercial_readiness_loop.py`:
- Around line 9-20: Remove the duplicate
test_active_scheduled_writer_claims_the_repository_lease test and rely on the
existing test_static_and_live_writer_lease_policy coverage, or refactor it to
use the shared organization_commercial_readiness_fixtures.workflow helper
instead of constructing WorkflowRecord directly. Keep the lease-policy assertion
covered without maintaining duplicate workflow data.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c199a4e-ccf2-4788-bf96-9b7d5e355157
📒 Files selected for processing (13)
.github/workflows/organization-commercial-readiness-loop-quality-ci.yml.github/workflows/organization-commercial-readiness-loop.ymlCHANGELOG.mddocs/doctoring/organization-commercial-readiness-loop.mdorganization_commercial_readiness_fixtures.pyscripts/ci/organization_commercial_readiness_loop.pytests/test_organization_commercial_readiness_loop.pytests/test_organization_commercial_readiness_loop_coordinator.pytests/test_organization_commercial_readiness_loop_credential_contract.pytests/test_organization_commercial_readiness_loop_github.pytests/test_organization_commercial_readiness_loop_import_contract.pytests/test_organization_commercial_readiness_loop_policy.pytests/test_organization_commercial_readiness_loop_run_pagination.py
|
@coderabbitai review @opencode-agent review @cwl-noema-review Please review exact current head |
|
|
|
@cwl-noema-review Exact current head: |
1 participant
Purpose
Add one organization-central hourly coordinator for the complete
ContextualWisdomLabrepository fleet without creating a second merge engine or competing with repositories that already own an enabled dedicated writer loop.Live-state rationale
Protected central
mainalready runs an organization-wide review/merge sweep every 15 minutes. Duplicating that implementation would increase Actions load and create conflicting merge decisions. The missing boundary is a bounded hourly coordinator that can:This PR deliberately does not modify
pr-review-merge-scheduler.yml,pr-review-fix-scheduler.yml, or the autofix worker currently changed by other central PRs.Safety and authority boundary
workflow_dispatchpath;github.tokenor reviewer-token fallback for cross-repository work;PR_REVIEW_MERGE_TOKEN, scoped to the final dispatch shell step;OPENCODE_APPROVE_TOKENremains isolated to the reviewer credential chain;NVIDIA_NIM_API_KEYnorCOPILOT_GITHUB_TOKEN;persist-credentials: false;# cwl-org-commercial-entrypoint: v1, withconcurrency,NVIDIA_NIM_API_KEY, no schedule, and noCOPILOT_GITHUB_TOKEN.A missing compliant product entrypoint is a no-op, not permission for the central workflow to inject a writer into an arbitrary repository. Existing scheduled repository loops retain their leases and continue operating independently.
Operability and evidence
TDD and hosted repair evidence
The first branch commit
60f6f2c5d91cfb0dc66c78b4f6d93cff69881f31added the writer-lease contract before the coordinator module existed, so collection failed on the intentionally absent import.Hosted run
31258006346exposed a test-packaging defect: split test modules imported a helper fromtests/as though it were a top-level module. A permanent import contract was added first; the helper now has an import-stable repository-root identity and the focused gate uses pytest--import-mode=importlib.Exact head
03a9124322c4f80d91eb4ccc4742821fcf11c304passed the focused quality gate, but Strix run31258235842correctly rejected the centralworkflow_dispatchentrypoint under the repository-wide no-branch-selected-manual-dispatch policy. The entrypoint was removed and reviewer/maintainer credentials were separated.Later RED heads captured active-run pagination, fixed organization scope, step-scoped maintainer credentials, fleet-wide operational failures, writer-source API-call bounds, strict NVIDIA/manual product opt-in, and durable JSON receipt requirements before their implementations.
Exact-current-head state
mainat6eb06cdd08c79a06f7b390069d4ffa49e2eb7dba;9f46ea9dd067d2b30a042710ab0ce022513b13f6;31260065881: success;independent-reviewersteam request is not currently resolvable for this repository.The only submitted formal review is a predecessor-head CodeRabbit
COMMENTEDreview. Its valid findings were implemented test-first and all inline threads are resolved, but that review does not transfer to the current head. Exact current-head@opencode-agentand@cwl-noema-reviewinvocations were submitted through the supported comment path; no qualifying formal current-head approval has yet been recorded.Activation and merge gate
Scheduled workflows execute only from protected
main; this loop is not active while this PR is unmerged. Every exact-head quality, security, and supply-chain check currently passes. Merge remains prohibited until current-head automated review requirements are satisfied, a qualifying independent non-author approval exists, and branch protection/repository policy accept the unchanged head without bypass.No queued, pending, skipped-required, cancelled, absent, neutral-required, stale-head, predecessor-head, status-only, author-only, synthetic-merge-only, or failed evidence is accepted as success.