Comfy-Org / Comfy-Org/github-workflows

[groom] reviewers.yml has two hand-ported parsers whose parity is asserted by two independent suites and no shared corpus

未关闭
#286 0 条评论 0 个 reaction 已指派 0 人 在 GitHub 查看
groom
主要语言
Shell
星标
6
派生
1
平均合并
14 小时 4 分钟
30 天内合并 PR
98

描述

**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/34828869870)

## Problem

parseReviewerConfig (JS, inline at assign-reviewers.yml:424) and parse_reviewer_config (Python, generate.py:172, under a header at 131 reading "parity with parseReviewerConfig") are a line-by-line port: same comment-stripping quote-state machine, same unquote, same flow-sequence parser, same indent helper, same dash/list-key walk. globToRegExp is ported the same way (generate.py:96, docstring "Port of assign-reviewers.yml's globToRegExp"). Parity is asserted only by two independent hand-written suites (.github/assign-reviewers/tests/assignment.test.cjs and .github/refresh-reviewers/tests/test_generate.py); there is no shared corpus, so a case exercised on one side proves nothing about the other.

Candidate divergences visible on a read, offered as corpus seeds rather than as claims: the JS splits lines on a CR-optional newline regex while the Python splits on a bare newline; the JS unquote has no length guard, so a one-character double-quote would slice to empty, while the Python _unquote requires length >= 2.

## The harness is fragile, exactly as described

assignment.test.cjs:7 recovers the shipped script by splitting the workflow text on a literal 10-space-indented "script: |" line and on the exact downstream step name "Publish reviewer history manifest", then stripping 12 leading spaces per line. Verified: assign-reviewers.yml contains exactly ONE "script: |" (line 166) and the anchor step is at line 710, so it works today -- but it is load-bearing on there never being a second "script: |" at that indentation, on that step name never changing, and on the indentation staying at 12. Any of those would silently capture the wrong span rather than fail.

## The unification half is correctly dropped

Grepping assign-reviewers.yml for actions/checkout or workflows_ref returns NOTHING -- this workflow deliberately has no checkout at all, so moving the JS to a file would add a workflows_ref pin to every existing caller, a breaking contract change for a cosmetic gain. And the Python genuinely computes something the JS does not: parse_reviewer_config returns a (config, locations) pair recording flow/block line positions so the drift generator can rewrite only those bytes. They are not the same function. Do NOT collapse them.

## Scope (test-only)

1. Add one shared fixture corpus under .github/assign-reviewers/ -- JSON mapping config text to expected parsed shape, and glob+path to expected match -- and have BOTH assignment.test.cjs and test_generate.py iterate it. The Python side asserts only the config half of its return; locations stays covered by its own suite.
2. Re-anchor the extraction in assignment.test.cjs on a sentinel comment emitted inside the inline script, rather than on indentation plus an unrelated downstream step name, and fail loudly if the sentinel is not found.

## Value and risk

The config format is a contract read by the runtime and REWRITTEN by the drift generator, so a divergence could mean the generator edits lists that the runtime reads differently -- nothing today could catch that. Risk is low: test-only, no shipped code changes. Not marked security: this decides who is ASSIGNED to review, not who may approve or what may execute.

贡献指南

打开贡献指南

调研方向

Read .github/assign-reviewers/tests/assignment.test.cjs and .github/refresh-reviewers/tests/test_generate.py, then inspect the parser entry points in assign-reviewers.yml:424 and generate.py:172. Add the shared JSON corpus under .github/assign-reviewers/, make both suites iterate it, and re-anchor script extraction on an in-script sentinel that fails loudly when absent. Keep Python location assertions in its existing suite and run both test suites.

由索引模型根据 Issue 内容生成。

评估

技术栈
github-actions, javascript, python
领域
ci-cd, testing
Issue 类型
重构
难度
3/5
预计耗时
1-2 天
活跃度
活跃
描述清晰度
描述清楚
新手友好度
74/100

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。