feat(operations): restack runner-assignment audit on current main - #252
feat(operations): restack runner-assignment audit on current main#252seonghobae wants to merge 9 commits into
Conversation
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthrough읽기 전용 GitHub Actions runner-assignment 감사 CLI를 추가했다. 정확한 source head와 workflow run을 검증하고 모든 job 페이지를 수집한다. runner 할당 상태를 ChangesRunner assignment 감사
Estimated code review effort: 4 (Complex) | ~45 minutes Mergeability Score: 🔵 Low · up to This PR adds a read-only runner-assignment audit and CLI entry point. The current implementation may retain more jobs than documented, can hide the original file-write error during cleanup, and may conflict with the repository’s secret-handling policy unless an exception is confirmed. CI and security checks pass, so the PR is mergeable with explicit owner awareness of these bounded risks. Possibly related issues
Possibly related PRs
Sequence Diagram(s)sequenceDiagram
participant Operator
participant RunnerAssignmentCLI
participant GitHubActionsAPI
participant EvidenceEvaluator
participant ReportFile
Operator->>RunnerAssignmentCLI: operations:runner-assignment 실행
RunnerAssignmentCLI->>GitHubActionsAPI: workflow run 및 전체 jobs 조회
GitHubActionsAPI-->>RunnerAssignmentCLI: paginated run/job 증거 반환
RunnerAssignmentCLI->>EvidenceEvaluator: source head와 runner assignment 평가
EvidenceEvaluator-->>RunnerAssignmentCLI: PASS, PENDING 또는 FAIL
RunnerAssignmentCLI->>ReportFile: JSON 보고서 원자적 기록
RunnerAssignmentCLI-->>Operator: 상태 출력 및 종료 코드 반환
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
scripts/actions-runner-assignment-audit.mjs (1)
146-158: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win
parseQueueGrace가 평가기 상한을 반영하지 않는다.
scripts/lib/actions-runner-assignment-audit.mjs는 30분 초과 grace를runner_evidence_invalidFAIL로 거부한다. 여기서는 상한이 없으므로, 잘못된 환경변수 값이 입력 오류가 아니라 감사 실패 보고서로 나타난다. 운영자 진단이 어려워진다. CLI에서 동일한 상한을 적용하십시오.🤖 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/actions-runner-assignment-audit.mjs` around lines 146 - 158, Update parseQueueGrace to enforce the same 30-minute maximum used by the evaluator, rejecting values above that limit as invalid environment configuration before audit processing. Preserve the existing positive-integer and safe-integer validation, and reuse the shared upper-bound symbol if one is available.test/actions-runner-assignment-source.test.ts (1)
16-28: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression coverage for the two input and API boundary contracts introduced by this audit: reject oversized run-id text and missing read adapters, and verify that
ghApirejects absolute paths, traversal segments, control characters, and overlong paths without invokinggh. These tests should remain local and use Vitest.🤖 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 `@test/actions-runner-assignment-source.test.ts` around lines 16 - 28, Add regression cases in the existing tests for parseSelectedRunIds covering input exceeding MAX_RUN_ID_TEXT_BYTES and asserting rejection, plus the adapter-missing path for the relevant assignment-source API and its expected failure. Keep the tests in Vitest style alongside the existing boundary cases and use the established symbols and error messages. Apply the same fix in `@test/actions-runner-assignment-cli.test.ts` around lines 11 - 23: Covers the CLI API-path validation cases listed in the original comment.Source: Learnings
🤖 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/actions-runner-assignment-audit.mjs`:
- Around line 173-184: Update the cleanup logic in the finally block around
descriptor and temporaryPath handling so unlinkSync failures other than ENOENT
do not throw and replace an exception from the preceding try body. Preserve
descriptor closure and missing-file tolerance, while swallowing cleanup errors
to ensure the original openSync or writeFileSync failure propagates.
In `@scripts/lib/actions-runner-assignment-source.mjs`:
- Around line 59-69: scripts/lib/actions-runner-assignment-source.mjs:59-69의 수집
루프에서 모든 run의 job을 합산한 누적 개수가 총량 상한 2,000건을 넘지 않도록 MAX_SELECTED_JOBS 검사를 전역 누적
기준으로 적용하십시오. scripts/lib/actions-runner-assignment-source.mjs의 jobs 수집 동작은 유지하되
상한 초과 시 기존 오류 흐름을 사용하십시오.
docs/doctoring/actions-runner-assignment-audit.md:20-20은 총량 상한을 적용한 뒤 별도 변경이 필요
없습니다.
---
Nitpick comments:
In `@scripts/actions-runner-assignment-audit.mjs`:
- Around line 146-158: Update parseQueueGrace to enforce the same 30-minute
maximum used by the evaluator, rejecting values above that limit as invalid
environment configuration before audit processing. Preserve the existing
positive-integer and safe-integer validation, and reuse the shared upper-bound
symbol if one is available.
In `@test/actions-runner-assignment-source.test.ts`:
- Around line 16-28: Add regression cases in the existing tests for
parseSelectedRunIds covering input exceeding MAX_RUN_ID_TEXT_BYTES and asserting
rejection, plus the adapter-missing path for the relevant assignment-source API
and its expected failure. Keep the tests in Vitest style alongside the existing
boundary cases and use the established symbols and error messages.
Apply the same fix in `@test/actions-runner-assignment-cli.test.ts` around lines
11 - 23: Covers the CLI API-path validation cases listed in the original
comment.
🪄 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: 12129f2d-8abb-461e-820a-60ee59b5eb20
📒 Files selected for processing (10)
CHANGELOG.mddocs/doctoring/actions-runner-assignment-audit.mdpackage.jsonscripts/actions-runner-assignment-audit.mjsscripts/lib/actions-runner-assignment-audit.mjsscripts/lib/actions-runner-assignment-source.mjstest/actions-runner-assignment-audit.test.tstest/actions-runner-assignment-cli.test.tstest/actions-runner-assignment-docs.test.tstest/actions-runner-assignment-source.test.ts
Purpose
Create a fresh direct protected-main successor for stale runner-assignment Drafts without rebasing, force-pushing, or transferring predecessor evidence. Protected
mainadvanced through #250; this successor is built directly on that protected lineage and is now fully revalidated at its current exact head.Exact source identity
82d884d0415a83423d56fba9bac5262c3470a646;aeb567797ee9fd18c7e86fc03c36b91dd5e89223;93f3a585d9b584b1e901c02f03059db7ba4736c5;edf9f7e4c92f29b66a19913574006cf294d4a8b9;The two post-restack commits are retained evidence of a test-discovered documentation defect and its correction. The first changelog correction attempt changed unrelated protected wording; an immediate exact-base comparison detected that no-op-quality failure. The current head restores all protected changelog text exactly and adds only the intended one-line runner-assignment contract. History was not rewritten.
Restack integrity
The eight runner-assignment source/test/operator blobs are preserved exactly from #251:
docs/doctoring/actions-runner-assignment-audit.md—3920cc778152b366263e94f324c6e74a2b8becb8;scripts/actions-runner-assignment-audit.mjs—e09422a7a5effe883af6515b0255134dfe2effb3;scripts/lib/actions-runner-assignment-audit.mjs—f7dba42424cd2c6c93ba95459e7a12eecf4135c4;scripts/lib/actions-runner-assignment-source.mjs—0cad167a30e933d56f97d47e111be40ec280d2d3;test/actions-runner-assignment-audit.test.ts—45ca88e2a9467afd6d132d0386a9bc0038a23b0f;test/actions-runner-assignment-cli.test.ts—93e24358f70efd7844340c24d85d4f52e774e366;test/actions-runner-assignment-docs.test.ts—83076fbf1f05234b1e5710348f48f6e5317b27ca;test/actions-runner-assignment-source.test.ts—e35adf11d2ff167f44014d1c8f9a9388249b8bc9.Protected-main
package.jsonwas independently refetched at base82d884d0415a83423d56fba9bac5262c3470a646. The successor changes that file only by addingoperations:runner-assignment; current package-manager/install-script metadata and #250 KPI provenance controls are preserved. The finalCHANGELOG.mddiff is one additive line only.Evidence boundary
The operator is read-only and exact-source-head/workflow-run-ID bound. Paginated
filter=alljob evidence separates runner assignment from workflow/test conclusions and dependency/environment waiting. A fresh unassigned queue remains non-passing; bounded-grace expiry fails closed; an assigned runner followed by workflow failure proves assignment only, never workflow success.The
ghsubprocess receives boundedPATH, explicit read-onlyGH_TOKEN, pinnedGH_HOST=github.com, andNO_COLOR=1; ambient write/model/App/proxy/home authority is not intentionally propagated. Reports carry no required-check, formal-review, merge, release, deployment, production, KPI, licensing, IP, or acquisition authority.Fresh exact-head validation
For unchanged exact head
edf9f7e4c92f29b66a19913574006cf294d4a8b9on protected base82d884d0415a83423d56fba9bac5262c3470a646:cirun31659834342: terminal success;reviewer-cirun31659834314: terminal success;Security Scanrun31659834299: terminal success;82d884d0415a83423d56fba9bac5262c3470a646, then exact headedf9f7e4c92f29b66a19913574006cf294d4a8b9; both scans produced 0 findings and the reporter found no issues;24.19.0/ npm11.17.0, installed with 0 vulnerabilities, and passedrelease:verifywith 120 test files / 1,028 tests;npm audit --audit-level=highreports 0 vulnerabilities;The non-strict KPI step still reports SKIP because no production log is present. The acquisition manifest continues to report missing final-gate production KPI/provenance, independent security-validation, immutable release-publication, production deployment/attestation/verification and environment-governance, revenue/customer, and legal/transfer evidence. None of those absences is promoted to PASS by this PR.
Governance and merge boundary
Live enforceable Noema governance was refetched against the current head/base. Active organization ruleset
18794436applies central.github/workflows/security-scan.ymlfrom.githubmainto the default branch, has no bypass actors, and does not currently impose an independent-approval rule. Central.githubmainis6eb06cdd08c79a06f7b390069d4ffa49e2eb7dba; its Security Scan remains protected-base triggered and hard-gates OSV PR-introduced vulnerabilities, supported dependency review, and fixable MEDIUM/HIGH/CRITICAL Trivy findings, with Scorecard posture evidence.No predecessor CI, reviewer, scanner, model, operational, or production evidence transfers from #251/#240/#94. Marking this PR ready may itself trigger new evidence; any new pending, queued, skipped, absent, neutral, failed, cancelled, stale, predecessor-head, status-only, model-only, or rate-limited result is non-passing until terminally resolved. Immediately before merge, refetch exact head/base, ancestry, reviews/threads, actual checks and checkout SHAs, live ruleset, and central Security Scan authority again and require zero valid unresolved findings.
#240 and #94 remain historical predecessors until this current protected-main successor is actually integrated; only then may they be closed as proven superseded work.
Related: #27, #30, #77, #79, #88, #91, #94, #240, #251
Summary by CodeRabbit
새 기능
PASS,PENDING,FAIL상태로 명확히 분류합니다.문서