[PR #49] [MERGED] feat(groom): reusable groom.yml — two-phase finder→verifier, GitHub-issue sink (BE-3872) #59

New issue

Closed

opened 2026-10-26 22:35:49 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-26 22:35:49 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Comfy-Org/github-workflows/pull/49
Author: @mattmillerai
Created: 7/21/2026
Status: ✅ Merged
Merged: 7/21/2026
Merged by: @mattmillerai

Base: main ← Head: matt/be-3872-groom-reusable-workflow

📝 Commits (2)

f0444f5 feat(groom): reusable groom.yml — two-phase finder→verifier, GitHub-issue sink (BE-3872)
447941d fix(groom): address cursor-review panel — isolate verifier job, harden allowlist + filing (BE-3872)

📊 Changes

3 files changed (+686 additions, -5 deletions)

View changed files

➕ .github/workflows/groom.yml (+675 -0)
📝 AGENTS.md (+10 -5)
📝 README.md (+1 -0)

📄 Description

ELI-5

We already have a robot that reviews pull requests (cursor-review.yml). This adds a second robot, groom, that goes the other way: on a schedule it reads a whole repo (not a diff) and looks for cleanups worth doing — duplicated code, dead code, inconsistent patterns. One agent proposes candidates, a second, independent agent plays skeptic and confirms/rejects each, and the survivors are filed as GitHub issues (labeled groom) for humans to pick up. It never writes code, opens a PR, or merges anything — it only files issues. A durable ledger (built in Phase 1) remembers what it already filed or what a human rejected, so it never nags you twice.

What this does

Adds .github/workflows/groom.yml as a workflow_call reusable, cloning the cursor-review topology and reusing the Phase-1 briefs + ledger under .github/groom/ as the single source of truth:

gate job — validates sink and applies the volume gate (skip a repo with no merges in cadence days; fail-open).
audit job (contents: read only) — checks out a clean default-branch copy, substitutes the {{…}} placeholders in finder.md/verifier.md, runs the finder then a fresh independent verifier via anthropics/claude-code-action, and uploads the verified findings. This job holds no write credentials — it is structurally incapable of writing to GitHub.
file job — dedups survivors (CONFIRM/DOWNGRADE only) against the durable ledger and opens groom-labeled GitHub issues as the configured bot (Comfy: cloud-code-bot via bot_app_id + BOT_APP_PRIVATE_KEY), embedding the ledger's signature marker so the next run won't re-file. Security-adjacent findings get groom-security (investigate, don't auto-implement).

Triggers (schedule + workflow_dispatch, never on-PR) and the required concurrency: group live in the caller (documented in the header + README), matching how cursor-review's triggers live in its caller. A workflow-level concurrency group also serializes runs per target repo as a backstop against the ledger's TOCTOU race.

Inputs

themes, cadence, volume_gate, sink (default github), max_findings, scope_label/scope_desc, model, workflows_ref, bot_app_id, dry_run. Secrets: ANTHROPIC_API_KEY (required), BOT_APP_PRIVATE_KEY (when bot_app_id set).

Acceptance criteria

✅ groom.yml reusable exists; files verified findings as GitHub issues, acted as cloud-code-bot, no credentials in the agent step (the audit job is contents: read-only and the agent gets only ANTHROPIC_API_KEY).
✅ Dedup hook consults the rejection-memory store (.github/groom/ledger.py) before filing, reusing its exact signature marker.
✅ SHA-pinnable like the other reusables; documented in README + AGENTS.md. Third-party actions are SHA-pinned per the AGENTS.md convention.
✅ Dry-run parity: the placeholder substitution reproduces the studio supervisor-groom.sh briefs verbatim (same .github/groom/*.md), and dry_run prints exactly what it would file for a spot-check. Live parity against a studio run on a test repo is the remaining manual verification step — see below.
⚠️ Linear agent-ok sink is deferred (needs org LINEAR_API_KEY), per the ticket — sink: linear fails the run loudly rather than silently filing nothing.

Verification done

actionlint clean on groom.yml.
.github/groom ledger tests: 37 passing (dep unchanged).
agents-md-integrity checker passes (1 pre-existing CODEOWNERS warning, unrelated).
Local smoke tests: placeholder substitution leaves no leftover tokens in either brief; the signature_marker reuse round-trips; the filing logic verified for dry-run, live-command shape, max_findings capping (with deferral warning), invalid-signature skip, and groom-security routing.
Not runnable from here: a live workflow_dispatch on a test repo requires the org ANTHROPIC_API_KEY + cloud-code-bot install on that repo — that end-to-end run (and the studio-parity spot-check) is the reviewer/operator step before moving the v1 tag.

Judgment calls (flagged)

themes default: I could not read epic BE-3870, so I set the default to the finder brief's own five dimensions verbatim — a no-op that preserves studio parity by default while letting a caller narrow it. If the epic's "safe starter set" is a specific narrower list, adjust the default.
Bot filing: modeled on cursor-review's bot_app_id/BOT_APP_PRIVATE_KEY (consumer supplies its own App). Comfy uses cloud-code-bot.
Negative-claim (sink != github) path: this is a not-yet-built feature with a clear error, not a regression removing an existing capability — there is no existing linear-sink path in this new workflow to redirect to, and the ticket explicitly defers Linear. So it ships as a loud guard.
Checkout ref: the audit checks out the triggering ref (the default branch on schedule); a workflow_dispatch from a non-main branch would groom that branch. Acceptable and arguably desirable; noted for awareness.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Comfy-Org/github-workflows/pull/49 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 7/21/2026 **Status:** ✅ Merged **Merged:** 7/21/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-3872-groom-reusable-workflow` --- ### 📝 Commits (2) - [`f0444f5`](https://github.com/Comfy-Org/github-workflows/commit/f0444f54f07ab5dfbf5e04f4446c73fc62afa574) feat(groom): reusable groom.yml — two-phase finder→verifier, GitHub-issue sink (BE-3872) - [`447941d`](https://github.com/Comfy-Org/github-workflows/commit/447941df90056cecd2ad4331848d12d9746d9bda) fix(groom): address cursor-review panel — isolate verifier job, harden allowlist + filing (BE-3872) ### 📊 Changes **3 files changed** (+686 additions, -5 deletions) <details> <summary>View changed files</summary> ➕ `.github/workflows/groom.yml` (+675 -0) 📝 `AGENTS.md` (+10 -5) 📝 `README.md` (+1 -0) </details> ### 📄 Description ## ELI-5 We already have a robot that reviews pull requests (`cursor-review.yml`). This adds a second robot, **groom**, that goes the other way: on a schedule it reads a whole repo (not a diff) and looks for *cleanups worth doing* — duplicated code, dead code, inconsistent patterns. One agent proposes candidates, a second, independent agent plays skeptic and confirms/rejects each, and the survivors are filed as GitHub **issues** (labeled `groom`) for humans to pick up. It never writes code, opens a PR, or merges anything — it only files issues. A durable ledger (built in Phase 1) remembers what it already filed or what a human rejected, so it never nags you twice. ## What this does Adds `.github/workflows/groom.yml` as a `workflow_call` reusable, cloning the cursor-review topology and reusing the Phase-1 briefs + ledger under `.github/groom/` as the single source of truth: - **`gate` job** — validates `sink` and applies the volume gate (skip a repo with no merges in `cadence` days; fail-open). - **`audit` job** (`contents: read` **only**) — checks out a clean default-branch copy, substitutes the `{{…}}` placeholders in `finder.md`/`verifier.md`, runs the **finder** then a **fresh independent verifier** via `anthropics/claude-code-action`, and uploads the verified findings. This job holds **no write credentials** — it is structurally incapable of writing to GitHub. - **`file` job** — dedups survivors (CONFIRM/DOWNGRADE only) against the durable ledger and opens `groom`-labeled GitHub issues **as the configured bot** (Comfy: cloud-code-bot via `bot_app_id` + `BOT_APP_PRIVATE_KEY`), embedding the ledger's signature marker so the next run won't re-file. Security-adjacent findings get `groom-security` (investigate, don't auto-implement). Triggers (`schedule` + `workflow_dispatch`, never on-PR) and the required `concurrency:` group live in the **caller** (documented in the header + README), matching how cursor-review's triggers live in its caller. A workflow-level `concurrency` group also serializes runs per target repo as a backstop against the ledger's TOCTOU race. ## Inputs `themes`, `cadence`, `volume_gate`, `sink` (default `github`), `max_findings`, `scope_label`/`scope_desc`, `model`, `workflows_ref`, `bot_app_id`, `dry_run`. Secrets: `ANTHROPIC_API_KEY` (required), `BOT_APP_PRIVATE_KEY` (when `bot_app_id` set). ## Acceptance criteria - ✅ `groom.yml` reusable exists; files verified findings as GitHub issues, acted as cloud-code-bot, **no credentials in the agent step** (the `audit` job is `contents: read`-only and the agent gets only `ANTHROPIC_API_KEY`). - ✅ Dedup hook consults the rejection-memory store (`.github/groom/ledger.py`) before filing, reusing its exact signature marker. - ✅ SHA-pinnable like the other reusables; documented in README + AGENTS.md. Third-party actions are SHA-pinned per the AGENTS.md convention. - ✅ **Dry-run parity:** the placeholder substitution reproduces the studio `supervisor-groom.sh` briefs verbatim (same `.github/groom/*.md`), and `dry_run` prints exactly what it would file for a spot-check. *Live parity against a studio run on a test repo is the remaining manual verification step — see below.* - ⚠️ Linear `agent-ok` sink is **deferred** (needs org `LINEAR_API_KEY`), per the ticket — `sink: linear` fails the run loudly rather than silently filing nothing. ## Verification done - `actionlint` clean on `groom.yml`. - `.github/groom` ledger tests: 37 passing (dep unchanged). - `agents-md-integrity` checker passes (1 pre-existing CODEOWNERS warning, unrelated). - Local smoke tests: placeholder substitution leaves **no** leftover tokens in either brief; the `signature_marker` reuse round-trips; the filing logic verified for dry-run, live-command shape, `max_findings` capping (with deferral warning), invalid-signature skip, and `groom-security` routing. - **Not runnable from here:** a live `workflow_dispatch` on a test repo requires the org `ANTHROPIC_API_KEY` + cloud-code-bot install on that repo — that end-to-end run (and the studio-parity spot-check) is the reviewer/operator step before moving the `v1` tag. ## Judgment calls (flagged) - **`themes` default:** I could not read epic BE-3870, so I set the default to the finder brief's own five dimensions verbatim — a no-op that **preserves studio parity** by default while letting a caller narrow it. If the epic's "safe starter set" is a specific narrower list, adjust the default. - **Bot filing:** modeled on cursor-review's `bot_app_id`/`BOT_APP_PRIVATE_KEY` (consumer supplies its own App). Comfy uses cloud-code-bot. - **Negative-claim (`sink != github`) path:** this is a *not-yet-built* feature with a clear error, not a regression removing an existing capability — there is no existing linear-sink path in this new workflow to redirect to, and the ticket explicitly defers Linear. So it ships as a loud guard. - **Checkout ref:** the audit checks out the triggering ref (the default branch on `schedule`); a `workflow_dispatch` from a non-main branch would groom that branch. Acceptable and arguably desirable; noted for awareness. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-26 22:35:49 +00:00