feat(groom): turn on the review-gated auto-builder

[mattmillerai]

ELI-5

This repo's groom robot was set up to hand its findings over as pull requests, but nobody ever told it it was allowed to write code — so it has been reading the whole repo on a schedule, finding real problems, and throwing them away. This gives it permission. It still can't merge anything; it opens PRs a human reviews.

Draft on purpose — one credential has to land in this repo first (below).

What was broken

vars.GROOM_CONFIG here sets max_findings: 0. That is a literal slice (to_file[:0]), not "unlimited" — it parks issue filing, which is correct only when the builder is on and findings arrive as PRs instead. Comfy-Org/cloud and comfy-infra run that same variable and both set builder: true in the caller. This caller never did, so it has had neither sink.

Visible in the record: the variable was set 2026-08-04, and the last groom-labeled issue here is from 2026-07-30. Every scheduled run since has been green and produced nothing. max_prs in the variable was inert the whole time — it only governs the builder path, and builder is in the reusable's _LOCKED_KEYS, so a variable can never switch it on.

The fix

builder: true in the caller — the reviewed diff the lock is designed to force. Nothing else changes:

• max_prs deliberately not added here. The variable already carries it (2) and outranks with:; a duplicate would be dead config that reads as authoritative.
• max_findings: 0 must be raised, not kept (corrected after review). The original reasoning — "with the builder on it means PRs, not issues" — holds only for the findings the builder will actually build. DOWNGRADE and security/auth-adjacent findings are deliberately excluded from the builder and land in the file remainder, where max_findings: 0 truncates them away. Bail issues really are unaffected (build_pr files those on an uncapped path), but the excluded classes are not, so raising the variable is now a merge prerequisite above.
• Findings skipped while the cap was 0 were deferred, not lost; the dedup ledger holds them.

BLOCKER — merge only after this is done

This repo has neither vars.APP_ID nor secrets.CLOUD_CODE_BOT_PRIVATE_KEY today — confirmed absent at both repo and org level via the Actions variables/secrets API.

The damage is wider than "the PR sink dies", corrected after review: with builder: true and an empty bot_app_id, build_select fails its Validate builder config step with exit 1, and the file job declares needs: [gate, build_select] — so the issue sink dies with the PR sink and the run emits nothing at all, after the finder and verifier have already billed ANTHROPIC_API_KEY. (The PR sink is doomed independently: build_pr reads steps.bot_token.outputs.token with no github.token fallback, and callers grant only contents: read.) And because builder is in the reusable's _LOCKED_KEYS, vars.GROOM_CONFIG cannot switch it back off — only another PR can.

• cloud-code-bot (App 2016716) installed on Comfy-Org/comfy-cli with contents + pull-requests write
• Gate run-on-gpu.yml — do this BEFORE the credentials below. It triggers on pull_request to main for comfy_cli/** and runs pip install -e . + TEST_E2E=true pytest tests/e2e on the self-hosted gpu-runners group. Groom PRs are same-repo, so that fires with no approval gate — LLM-authored code on our hardware before a human reads the diff. The reusable's patch-path policy covers workflows/manifests/lockfiles but not ordinary source, so the gate must live here. Tracked as a follow-up.
• Raise max_findings above 0 in vars.GROOM_CONFIG. The builder builds only CONFIRM, non-security findings; DOWNGRADE and security/auth-adjacent findings route to the file remainder, which max_findings: 0 slices to nothing — silently discarding exactly the class the builder refuses to touch. (Bail issues are a separate uncapped path and are unaffected.)
• secrets.CLOUD_CODE_BOT_PRIVATE_KEY set — first
• vars.APP_ID = 2016716 — second
• gh workflow run groom.yml --repo Comfy-Org/comfy-cli -f dry_run=true green, with "Mint bot-identity token" succeeding

Order matters. The reusable mints whenever bot_app_id is non-empty and hard-fails on an empty key, so setting APP_ID before the key breaks the runs that work today. Verify the op read returns bytes before piping it into gh secret set — a failed read stores an empty secret, which passes Actions startup validation and then hard-fails inside the job.

Security posture

Unchanged, and worth stating since this grants an agent the ability to author code:

• The agent jobs (audit_find, audit_verify, build) run contents: read and mint no GitHub token — the process that reads untrusted code never holds write credentials.
• build_pr is the only job in the path with a write token. It runs no agent and no repo code: it applies the patch artifact on a fresh branch, commits, pushes, opens the PR.
• PRs are review-gated. Groom never merges. Security-adjacent findings are excluded from the builder and stay as issues to investigate.

Provenance

Authored by: agent-work loop

Verified: yaml.safe_load parses the caller; ruff check clean. Every review finding re-verified against the pinned reusable (eaee6df) rather than taken on trust: build_select's exit 1 validation step, file's needs: [gate, build_select], builder absent from the GROOM_CONFIG DEFAULTS overlay, capped = to_file[:max_findings] in the file job, and run-on-gpu.yml's pull_request/gpu-runners/TEST_E2E steps. Credential and variable state read live from the Actions API (APP_ID and CLOUD_CODE_BOT_PRIVATE_KEY absent at repo and org; GROOM_CONFIG = max_prs: 2, max_findings: 0).

Deviations: The GPU-runner gating finding was deferred to a follow-up rather than fixed here — it changes a different workflow in a way that affects every contributor's PR, so the choice between a protected environment, a maintainer label, or an author-association skip is a maintainer policy call. It is recorded as merge prerequisite #2 above. Raising max_findings is a live Actions-variable edit (an operator action, like provisioning the credentials), so it was documented as a prerequisite rather than made unilaterally.

Background: Found by sweeping every repo with a groom caller after Comfy-Org/evals turned up the same misconfiguration (evals#487): the max_findings: 0 variable was copied from repos that run builder: true to three that don't. Semantics verified against github-workflows/.github/groom/config.py (_LOCKED_KEYS, and the comment scoping max_findings to the file job) and build_pr's credential handling at the pinned SHA.

Testing

• yaml.safe_load parses; jobs.groom.with.builder resolves to true.
• Credential state checked live via the Actions variables/secrets API for this repo (both absent) — hence the draft.
• Not verifiable pre-merge: the builder path only runs from the default branch. The dry run in the checklist above is the gate.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 16236d4e-61d5-4f67-9d52-98cc9172804f

📥 Commits

Reviewing files that changed from the base of the PR and between 81c8778 and 7f1f0b3.

📒 Files selected for processing (1)

• .github/workflows/groom.yml

Included review availability: Your plan provides up to 10 included reviews per hour; 1 remains after this review.

---

📝 Walkthrough

Walkthrough

The Groom workflow now runs in builder mode. Eligible confirmed non-security findings target review-gated pull requests. Missing App credentials prevent both pull request and issue sinks from emitting results. max_findings: 0 still suppresses issue filing.

Changes

Groom builder workflow

Layer / File(s)	Summary
Enable Groom builder mode .github/workflows/groom.yml	The reusable Groom workflow enables builder mode. It documents max_prs, max_findings, App credentials, and the GPU workflow gate. Missing or invalid credentials prevent result emission. Builder mode handles confirmed non-security findings, while max_findings: 0 suppresses issue filing.

Merge Risk: 🟡 Moderate · up to 7f1f0

This change enables pull-request creation, but the repository currently lacks the required bot permissions and signing key, so scheduled runs could consume resources and then fail instead of opening pull requests. Keep the PR blocked until the credentials are provisioned and the documented dry run succeeds.

🚥 Pre-merge checks | ✅ 2

✅ Passed checks (2 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch matt/groom-enable-builder

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch matt/groom-enable-builder

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

🔍 Cursor Review — Consolidated panel

Triggered by @mattmillerai.

Found 5 finding(s).

Severity	Count
🔴 Critical	1
🟠 High	1
🟡 Medium	2
🟢 Low	1

Panel: 8/8 reviewers contributed findings.

[github-actions]

🔴 Critical — This flips on the auto-builder while the block's own HARD PREREQUISITE is unmet — the repo has neither vars.APP_ID nor secrets.CLOUD_CODE_BOT_PRIVATE_KEY — so the reusable's builder validation in build_select (builder=true with an empty bot_app_id) exits 1 on every scheduled run, after the finder and verifier have already billed ANTHROPIC_API_KEY. The failure is earlier and wider than the comment describes: because file needs build_select, the issue sink is skipped too, so the run emits nothing at all, and builder being a locked config key means vars.GROOM_CONFIG cannot switch it back off — only another PR can. Provision the private key first and the APP_ID second before merging this line. Raised by 8 of 8 reviewers (gpt-5.6-sol-max adversarial + edge-case, claude-opus-5-thinking-max adversarial + edge-case, gemini-3.1-pro adversarial + edge-case, kimi-k3-max adversarial + edge-case).

[mattmillerai]

Confirmed, and the correction to the failure point is right — fixed in 7f1f0b3.

Verified against the pinned reusable (eaee6df) and the live repo config:

• vars.APP_ID and secrets.CLOUD_CODE_BOT_PRIVATE_KEY are absent at both repo and org level (actions/variables, actions/secrets, actions/organization-variables, actions/organization-secrets).
• The failure is indeed earlier than the comment said: build_select fails its Validate builder config step with exit 1, and file declares needs: [gate, build_select], so the issue sink dies with the PR sink and the run emits nothing at all. My comment blamed build_pr, which is only the second-order failure.
• builder is genuinely not a GROOM_CONFIG knob — the reusable reads inputs.builder directly and the variable overlay only covers the keys in its DEFAULTS blob — so only another PR can switch it off. Now stated in the file.

The comments now describe this accurately, but the merge blocker itself stands and is not something code can fix: the credentials still have to be provisioned. Leaving this thread open on purpose so it stays visible to whoever presses merge.

[mattmillerai]

🤖 The reviews loop filed Linear follow-up ticket(s) for review thread(s) deferred as out of scope for this PR:

• BE-10590 — Gate run-on-gpu.yml behind maintainer approval before enabling the groom auto-builder — filed as agent-spike (premise unverified)

The following carry agent-spike instead of agent-ok because their reachability claim was not backed by evidence (BE-5378) — the claim is investigated before any code is written, and "the premise does not hold" is a valid, successful outcome:

• Gate run-on-gpu.yml behind maintainer approval before enabling the groom auto-builder — no reachability block in the proposal