ci(actions): consolidate PR workflow ownership - #784
Conversation
Replace this repo's hand-written dependency-review.yml with a thin caller into ContextualWisdomLab/.github's new workflow_call workflow, preserving this repo's exact original policy (unset fail-on-severity -> action default "low", allow-ghsas: GHSA-69w3-r845-3855). This repo's FORCE_JAVASCRIPT_ACTIONS_TO_NODE24 opt-in is now applied uniformly inside the central workflow itself (the other three callers previously lacked it), so it is no longer needed here. Adds the cancel-in-progress concurrency group this workflow previously lacked. See ContextualWisdomLab/.github#1724 and its docs/adr/0024-.../ docs/doctoring/dependency-review-reusable-workflow-consolidation.md for the full audit and design rationale. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughDependency review 워크플로에 동시성 제어와 읽기 전용 권한이 추가되었습니다. 인라인 작업은 SHA로 고정된 재사용 워크플로 호출로 대체되었습니다. 워크플로 설정과 재사용 호출 예외를 검증하는 테스트가 추가되었습니다. ChangesDependency review 워크플로
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to The workflow test now skips Node environment validation for reusable workflow-call jobs, which cannot define job-level environment variables. No current merge-blocking risk is evidenced. 🚥 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 |
|
Fleet repair note (2026-09-02): the mutable cross-repository Do not merge this consumer yet. The canonical Owner-path acceptance: merge |
|
Exact-head follow-up: the reusable-workflow admission failure is now causally reproduced and repaired. At immutable-pin head RED Still do not merge: |
…_in_pr A peer session's org-wide survey found naruon independently carrying its own dependency-review.yml with a harden-runner egress-audit step this consolidation's original four callers lacked. That step (and the comment_summary_in_pr input naruon's explicit "never" choice required) were added to the central reusable workflow in ContextualWisdomLab/.github#1732, after this caller's original PR opened. Re-pinning to the new commit picks up harden-runner for free before this PR's first merge -- no `with:` change needed, since this repo never set comment_summary_in_pr and the new input's default ("on-failure") matches the value this reusable workflow already hardcoded before. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…d workflow SHA The prior commit (4cff220) re-pinned dependency-review.yml's uses: to 5f8e5b2a79e709c4ab1a4179a605d34c458b13a1 (harden-runner + comment_summary_in_pr, .github#1732) but missed the matching test constant, leaving it asserting the old 0bcd22d8... SHA -- test_dependency_review_uses_immutable_central_workflow was failing. Caught by CodeRabbit's second review on this PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…d workflow SHA dependency-review.yml pins to 5f8e5b2a79e709c4ab1a4179a605d34c458b13a1 (harden-runner + comment_summary_in_pr, .github#1732), but this test's constant was left at the stale 0bcd22d8... SHA, failing test_companion_workflows_pin_central_dependency_review_and_cover_osv_manifests. Same class of mistake caught in ContextualWisdomLab/newsdom-api#784; a peer session found this one after my own tree-grep check missed it (the file lives at tests/config/test_strix_static_repo_adaptations.py, not a readme-named path). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
implicitly needing write
CodeRabbit: the central reusable workflow's default comment_summary_in_pr
("on-failure") forwards to dependency-review-action's
comment-summary-in-pr, which needs pull-requests: write to post a PR
comment. This caller only grants pull-requests: read, so an actual
dependency-review failure would attempt to comment without permission.
No PR summary comment is needed here -- explicitly opting out with
"never" matches the already-declared read-only permission instead of
escalating to write for a feature nothing here uses.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
A job shaped as `uses: owner/repo/.github/workflows/x.yml@sha` cannot carry an `env:` key at all -- GitHub Actions' schema for that job shape only allows name/needs/if/permissions/secrets/strategy/uses/with. The dependency-review job added in this PR calls .github's central reusable workflow this way, and that called workflow's own job already sets FORCE_JAVASCRIPT_ACTIONS_TO_NODE24 itself, so the safety property still holds -- it just can't be expressed on the caller. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Noema LLM review
The PR correctly migrates the dependency-review job to the central reusable workflow, pins it to an immutable SHA, adds concurrency control, and adds tests that synchronize the SHA and verify the workflow's inputs. However, the pinned SHA is outdated: it lacks the security-critical fail-closed behavior from ContextualWisdomLab/.github#1725, which is still open. Merging now would leave the supply-chain gate vulnerable to silent 403/404 pass-through. The combined required check name dependency-review / dependency-review matches branch protection, so that earlier concern is resolved, but the SHA content is the blocking issue. Request changes until the pin and test constant are updated to a SHA that includes the #1725 repair.
Reviewed changed lines
.github/workflows/dependency-review.yml:16 (RIGHT): The reusable workflow is pinned to5f8e5b2a79e709c4ab1a4179a605d34c458b13a1. This SHA predates the fail-closed HTTP 403/404 repair from ContextualWisdomLab/.github#1725, which is still open with no merge commit. Without that repair, the dependency-review gate remains fail-open when the dependency graph API returns 403/404, allowing the check to pass silently. The pin must be updated to the merge commit of #1725.tests/test_readme.py:3 (RIGHT): The test constantCENTRAL_DEPENDENCY_REVIEW_WORKFLOW_SHAcurrently matches the outdated pin5f8e5b2a79e709c4ab1a4179a605d34c458b13a1. It must be synced to the updated SHA that includes the #1725 repair once the workflow caller is repinned.
Adversarial validation
.github/workflows/dependency-review.yml:16 (RIGHT)confirmed: The dependency-review gate remains fail-open when the dependency graph API returns 403/404 because the pinned SHA predates the #1725 repair. — Prior review thread evidence confirms #1725 is open and that5f8e5b2a79e709c4ab1a4179a605d34c458b13a1is the merge of #1732 only (harden-runner + comment_summary_in_pr), lacking the required fail-closed repair..github/workflows/dependency-review.yml:16 (RIGHT)confirmed: The branch-protection required check name matches the combined reusable workflow namedependency-review / dependency-review. — Fresh protected-branch evidence in the review thread shows develop requiresdependency-review / dependency-reviewalongside pytest/scorecard/codeql/quality-gate; no required gate was removed or weakened.- Residual risk: After updating the pin to a SHA that includes the #1725 fail-closed repair, the test constant must be re-synced and the combined required check name re-verified. There is residual risk that the central workflow's inputs (fail_on_severity, allow_ghsas, comment_summary_in_pr) may change when the repair lands, so the caller's
withblock and tests should be re-checked against the final merge SHA.
Findings
- [high] .github/workflows/dependency-review.yml:16 (RIGHT): The pinned reusable workflow SHA
5f8e5b2a79e709c4ab1a4179a605d34c458b13a1predates the fail-closed HTTP 403/404 repair from ContextualWisdomLab/.github#1725, which is still open with no merge commit. This pin corresponds to the merge of #1732 only, so the central workflow at this SHA lacks the required handling to fail the dependency-review gate when the dependency graph API returns 403/404. Merging this PR would keep the gate fail-open and allow the check to pass silently on those errors. Update the pin to the merge commit of #1725 and synctests/test_readme.py:3to the same SHA.
- Result: REQUEST_CHANGES
- Head SHA:
2ba859e4444edaf2a7c93b377b5cee429c88ff3e - Reviewer credential:
noema-review-github-app-refresh - Actor:
cwl-noema-review[bot]
Remove duplicate local PR gates, preserve scheduled security backstops, and scope remaining concurrency by workflow, repository, and PR or ref. Signed-off-by: Seongho Bae <me@seonghobae.me>
Summary
testsworkflowOwnership evidence
Compared against
ContextualWisdomLab/.github@769691526f8c73cf714de8fe8ba51ae6cfa2901a. Organization ruleset18156473injects CodeQL PR, Noema, OpenCode, merge scheduler, Security Scan, Strix, and SAST Semgrep. Security Scan is the central PR owner for dependency review and Scorecard.The local repository ruleset and classic protection remain strict and now require only the non-duplicated
pytestcontext; approval, last-push approval, thread resolution, linear history, deletion, non-fast-forward, and all seven organization required workflows remain active.Queue impact
Verification
uv run pytest -q-> 491 passeduv run pytest --cov=src/newsdom_api --cov-branch --cov-report=term-missing --cov-fail-under=100 -q-> 491 passed, 100% coverageactionlint .github/workflows/*.ymlgit diff --checkExact base at push:
e06b1f3fb10903569124af011da213951e6e2473Exact head:
3ba1add4088e7e328feda5f277c9a2e98327de9e