Comfy-Org / Comfy-Org/github-workflows
[groom] reviewers.yml is parsed by two independent implementations kept in parity by comment only
- Ngôn ngữ chính
- Shell
- Star
- 6
- Fork
- 1
- Merge trung bình
- 14 giờ 4 phút
- Pull request đã merge (30 ngày)
- 98
Mô tả
**Groom auto-builder** — Comfy-Org/github-workflows: this CONFIRMED finding could not be auto-built (patch modifies a CI-privileged or dataset-of-record path (per .github/groom/patch_policy.py — workflow/action defs, lockfiles, build/test config that executes in pre-review CI, graded eval cases under a suites/**/cases/ tree, or a symlink into such a tree) — a human must author these changes), so it is filed for a human. · [run](https://github.com/Comfy-Org/github-workflows/actions/runs/34210376864)
**Problem (verified).** `refresh-reviewers.yml` WRITES `.github/reviewers.yml` and `assign-reviewers.yml` READS it, and each ships its own hand-rolled parser for the same `default_pool` + `rules[{paths, reviewers}]` schema plus its own glob-to-regex translator. JS: `assign-reviewers.yml:361` (`globToRegExp`) and `:386` (`parseReviewerConfig`). Python: `generate.py:96` (`glob_to_regexp`, whose docstring says it 'Must stay byte-for-byte semantics-equal to the JS original - the map is only correct if it is scored with the same matcher the runtime assigns with') and `:172` (`parse_reviewer_config`, under the 'parity with parseReviewerConfig' banner at `:131`). Parity is asserted in prose only: `.github/assign-reviewers/tests/assignment.test.cjs:5-9` extracts and executes the inline JS, `.github/refresh-reviewers/tests/test_generate.py` drives the Python, and no shared corpus is fed through both.
**A divergence already exists.** The JS `unquote` (`assign-reviewers.yml:398-400`) tests `startsWith`/`endsWith` with no length guard, so a one-character token consisting of a single double-quote is both an opening and a closing quote and it returns the empty string; the Python `_unquote` (`generate.py:152-155`) requires `len(s) >= 2` and returns that character unchanged. Degenerate input, but it is live proof that the 'byte-for-byte equal' comment is unverified rather than checked.
**Honest scope - not a security finding.** `generate.py` is a drift DETECTOR that emits a rewritten config for a human-reviewed PR (see its header, `:36-40`), and assignment is advisory. A divergence would mis-score the committed expertise map or mis-route a review request; it grants no permission and gates no merge.
**Fix.** Keep both implementations - `assign-reviewers.yml` deliberately declares no `workflows_ref` (zero occurrences in the file) and loads nothing at run time, which is what lets a caller pin it by `uses:` alone. Add the missing parity test instead: one shared fixture corpus of `reviewers.yml` documents plus glob/path pairs covering the edge cases each parser documents (comment stripping inside quotes, flow vs block sequences, `*` / `**` / `**/` / `?`, and the degenerate quote above), run through the Python functions and through the inline JS the `.cjs` harness already extracts, asserting identical parse output and identical match verdicts. Path-filter it to both surfaces so an edit to either side runs it.
**Risk.** Low: a test, no runtime change. The cost is one CI job needing both `node` and `python3`, and choosing a corpus that covers the documented edge cases rather than only the happy path.
Hướng dẫn đóng góp
Hướng nghiên cứu
Start with .github/refresh-reviewers/generate.py, .github/assign-reviewers.yml, and the existing tests in .github/refresh-reviewers/tests/test_generate.py and .github/assign-reviewers/tests/assignment.test.cjs. Run both harnesses and inspect how the inline JavaScript is extracted. Done means a shared corpus exercises the documented parser and glob edge cases, both implementations produce identical results, and changes to either surface run the test.
Do mô hình lập chỉ mục viết ra từ nội dung của issue.
Đánh giá
- Công nghệ
- github-actions, javascript, python
- Lĩnh vực
- ci-cd, testing
- Loại issue
- Lỗi
- Độ khó
- 4/5
- Thời gian dự kiến
- 3-5 ngày
- Mức độ hoạt động
- Sôi nổi
- Độ rõ ràng
- Đặc tả rõ ràng
- Mức phù hợp với người mới
- 48/100