Comfy-Org / Comfy-Org/github-workflows

[groom] reviewers.yml is parsed by two independent implementations kept in parity by comment only

Abierto
#270 0 comentarios 0 reacciones 0 asignados Ver en GitHub
groom
Lenguaje dominante
Shell
Estrellas
6
Forks
1
Merge medio
14 h 4 min
PR fusionados (30 d)
98

Descripción

**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.

Guía de contribución

Abrir la guía de contribución

Línea de trabajo

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.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
github-actions, javascript, python
Área
ci-cd, testing
Tipo de issue
Error
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Activo
Claridad
Bien especificado
Aptitud para principiantes
48/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.