[PR #56] [MERGED] feat(groom): configurable run cadence via GROOM_INTERVAL_DAYS interval gate (BE-4004) #70

New issue

Closed

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

bsakaguchi commented

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

Owner

📋 Pull Request Information

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

Base: main ← Head: matt/be-4004-groom-interval-days

📝 Commits (10+)

dd2ac0e feat(groom): configurable run cadence via GROOM_INTERVAL_DAYS interval gate (BE-4004)
bfb6a5e Merge branch 'main' into matt/be-4004-groom-interval-days
8eb8e0e docs(AGENTS): fix interval.py state description per CodeRabbit
acf6445 Merge remote-tracking branch 'origin/main' into matt/be-4004-groom-interval-days
160fdea fix(groom): normalize the volume-gate window through interval.py (BE-4004)
5195cbe Merge branch 'main' into matt/be-4004-groom-interval-days
7ec610b fix(groom): absorb cron jitter in the interval gate; string cadence inputs (BE-4004)
55edfa6 fix(groom): reject non-finite cadence, grant gate issues/PR read, fix comment (BE-4004)
2297266 Merge main into be-4004-groom-interval-days
507669c docs(groom): a missing actions:read grant rejects the run, it does not fail open

📊 Changes

7 files changed (+903 additions, -44 deletions)

View changed files

📝 .github/groom/README.md (+69 -0)
➕ .github/groom/interval.py (+345 -0)
➕ .github/groom/tests/test_interval.py (+262 -0)
📝 .github/workflows/ci-groom.yml (+48 -19)
📝 .github/workflows/groom.yml (+172 -20)
📝 AGENTS.md (+5 -3)
📝 README.md (+2 -2)

📄 Description

ELI-5

Groom used to run on a hardcoded weekly cron — to change how often it runs you had to edit the workflow file. GitHub Actions crons can't be "every N days" at runtime, so this switches groom to the standard trick: fire a cheap tick every day, but only actually do work every N days, where N is a live knob you set with a repo variable (GROOM_INTERVAL_DAYS, default 7 = today's weekly). A tick that's too soon exits in a few seconds before the expensive finder ever starts. Change the variable, cadence changes — no PR, no YAML edit.

⚠️ REQUIRED MERGE STEP — bump the caller pin

The interval gate lives in the new groom.yml, so ci-groom.yml pins a placeholder (@REPLACE_AT_MERGE_WITH_THIS_PRS_SQUASH_SHA) that fails loud (unresolvable ref / failed assets checkout) until bumped. This is the same reusable-lands-then-bump-caller two-step the pilot used (#49 → #53), compressed into one PR. After squash-merge, bump both uses: and workflows_ref in .github/workflows/ci-groom.yml to this PR's merge SHA. groom is schedule/dispatch-only (never on-PR), so nothing breaks on the PR itself; the placeholder just guarantees the daily cron can't silently run against the old (gate-less) reusable.

(No bump-groom-callers.yml automation exists yet — groom has a single caller. A dedicated bumper is a reasonable follow-up if more repos enroll.)

What changed

ci-groom.yml (caller): weekly cron → daily 17 9 * * *; grants actions: read; wires interval_days + cadence to ${{ vars.GROOM_INTERVAL_DAYS || '7' }}.
groom.yml (reusable): new interval_days input (default 7); the gate job now checks out the assets, runs the interval gate before the finder, then the (unchanged) volume gate, composing both into should_run. Gate job gains actions: read.
.github/groom/interval.py + tests: the gate. Derives the last real groom run from this caller's Actions run history — a run counts only if its Audit — finder job actually ran, so interval-skip ticks never reset the clock — and early-exits unless interval_days elapsed. Pure decision logic split from the gh I/O shell, mirroring ledger.py. Fail-open on any history-read error.
Docs: README table, .github/groom/README.md (new section), AGENTS.md.

Acceptance criteria

Cadence set by GROOM_INTERVAL_DAYS (default 7); changing it changes frequency with no workflow-file edit.
Base cron fires daily; a within-interval tick no-ops cheaply (exits in the gate job, before the finder).
workflow_dispatch always runs regardless of the interval.
Last-run state durable across CI runs (GitHub run history — no net-new secret, only actions: read).
Default (unset variable) stays weekly.

Judgment calls (worth a reviewer's eye)

Last-run state = run history, not a GROOM_LAST_RUN repo variable. The ticket sanctioned either. A repo variable needs a Variables: write credential the run doesn't otherwise carry — and a missing grant would fail silently into a daily over-spend. Run history is durable, needs only actions: read, and has no silent-failure trap.
Interval delivered as a reusable input wired to the var (rather than the reusable reading vars.GROOM_INTERVAL_DAYS directly). The caller evaluates the var in its own context — unambiguous — matching how cadence/dry_run/volume_gate are already wired. Same "no file edit" ergonomics.
workflow_dispatch bypasses the interval gate but not the pre-existing volume gate for a non-dry-run dispatch. That volume-gate-on-dispatch behavior is a deliberate spend guard the pilot chose (volume_gate: dry_run != 'true'); the ticket's "always runs regardless of the interval" is about the interval gate, so I left the volume gate alone (Chesterton's fence). A dry-run dispatch bypasses both, as before.
github.workflow_ref resolves to the caller (ci-groom.yml), not the reusable — the OIDC workflow_ref (caller) vs job_workflow_ref (reusable) distinction. If that assumption were ever wrong, the run-history query returns nothing → the gate fails open (runs), never crashes.
A finder job that failed still counts as a real run (not just success) — so a run that spent the billed audit but died at a later step doesn't re-spend on the next daily tick. Trade-off documented in interval.py.

Testing

python3 -m unittest discover -s .github/groom/tests → 54 passed (16 new for the interval gate: within/at/after-interval, skip-ticks-don't-reset, dispatch-always-runs, no-history/API-error fail-open, job-name matching).
actionlint (with shellcheck on the embedded run: blocks) clean on both workflows.

_{🔄 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/56 **Author:** [@mattmillerai](https://github.com/mattmillerai) **Created:** 7/22/2026 **Status:** ✅ Merged **Merged:** 7/27/2026 **Merged by:** [@mattmillerai](https://github.com/mattmillerai) **Base:** `main` ← **Head:** `matt/be-4004-groom-interval-days` --- ### 📝 Commits (10+) - [`dd2ac0e`](https://github.com/Comfy-Org/github-workflows/commit/dd2ac0e814a20a5d9b609ac7aafc9eb443754208) feat(groom): configurable run cadence via GROOM_INTERVAL_DAYS interval gate (BE-4004) - [`bfb6a5e`](https://github.com/Comfy-Org/github-workflows/commit/bfb6a5eb3094e1d49105fa2db17f2c4ccfe5567e) Merge branch 'main' into matt/be-4004-groom-interval-days - [`8eb8e0e`](https://github.com/Comfy-Org/github-workflows/commit/8eb8e0e02286e6465295e35f318467528fe27285) docs(AGENTS): fix interval.py state description per CodeRabbit - [`acf6445`](https://github.com/Comfy-Org/github-workflows/commit/acf6445e668bbf29b366d88d1a8ee7bdef52a554) Merge remote-tracking branch 'origin/main' into matt/be-4004-groom-interval-days - [`160fdea`](https://github.com/Comfy-Org/github-workflows/commit/160fdea0a05135a1b9590d7e4d85a71f0bcc74de) fix(groom): normalize the volume-gate window through interval.py (BE-4004) - [`5195cbe`](https://github.com/Comfy-Org/github-workflows/commit/5195cbe80e66129cc3ca63915679f567972bf880) Merge branch 'main' into matt/be-4004-groom-interval-days - [`7ec610b`](https://github.com/Comfy-Org/github-workflows/commit/7ec610b34b93e3c1f7a9c2cceefef897e54f8003) fix(groom): absorb cron jitter in the interval gate; string cadence inputs (BE-4004) - [`55edfa6`](https://github.com/Comfy-Org/github-workflows/commit/55edfa6ed1a0e815dd27324a2eceb241bdda98eb) fix(groom): reject non-finite cadence, grant gate issues/PR read, fix comment (BE-4004) - [`2297266`](https://github.com/Comfy-Org/github-workflows/commit/22972663b48c6c2151b92033c2f6b4e61d51b5a6) Merge main into be-4004-groom-interval-days - [`507669c`](https://github.com/Comfy-Org/github-workflows/commit/507669c77ef3645364c66e491f2958193f50d7ec) docs(groom): a missing actions:read grant rejects the run, it does not fail open ### 📊 Changes **7 files changed** (+903 additions, -44 deletions) <details> <summary>View changed files</summary> 📝 `.github/groom/README.md` (+69 -0) ➕ `.github/groom/interval.py` (+345 -0) ➕ `.github/groom/tests/test_interval.py` (+262 -0) 📝 `.github/workflows/ci-groom.yml` (+48 -19) 📝 `.github/workflows/groom.yml` (+172 -20) 📝 `AGENTS.md` (+5 -3) 📝 `README.md` (+2 -2) </details> ### 📄 Description ## ELI-5 Groom used to run on a **hardcoded weekly cron** — to change how often it runs you had to edit the workflow file. GitHub Actions crons can't be "every N days" at runtime, so this switches groom to the standard trick: **fire a cheap tick every day, but only actually do work every `N` days**, where `N` is a live knob you set with a repo variable (`GROOM_INTERVAL_DAYS`, default `7` = today's weekly). A tick that's too soon exits in a few seconds *before* the expensive finder ever starts. Change the variable, cadence changes — no PR, no YAML edit. --- ## ⚠️ REQUIRED MERGE STEP — bump the caller pin The interval gate lives in the **new** `groom.yml`, so `ci-groom.yml` pins a placeholder (`@REPLACE_AT_MERGE_WITH_THIS_PRS_SQUASH_SHA`) that **fails loud** (unresolvable ref / failed assets checkout) until bumped. This is the same reusable-lands-then-bump-caller two-step the pilot used (#49 → #53), compressed into one PR. **After squash-merge, bump both `uses:` and `workflows_ref` in `.github/workflows/ci-groom.yml` to this PR's merge SHA.** groom is schedule/dispatch-only (never on-PR), so nothing breaks on the PR itself; the placeholder just guarantees the daily cron can't silently run against the old (gate-less) reusable. *(No `bump-groom-callers.yml` automation exists yet — groom has a single caller. A dedicated bumper is a reasonable follow-up if more repos enroll.)* ## What changed - **`ci-groom.yml`** (caller): weekly cron → **daily** `17 9 * * *`; grants `actions: read`; wires `interval_days` + `cadence` to `${{ vars.GROOM_INTERVAL_DAYS || '7' }}`. - **`groom.yml`** (reusable): new `interval_days` input (default 7); the `gate` job now checks out the assets, runs the interval gate **before** the finder, then the (unchanged) volume gate, composing both into `should_run`. Gate job gains `actions: read`. - **`.github/groom/interval.py`** + **tests**: the gate. Derives the last **real** groom run from this caller's Actions run history — a run counts only if its `Audit — finder` job actually ran, so interval-skip ticks never reset the clock — and early-exits unless `interval_days` elapsed. Pure decision logic split from the `gh` I/O shell, mirroring `ledger.py`. Fail-open on any history-read error. - Docs: README table, `.github/groom/README.md` (new section), AGENTS.md. ## Acceptance criteria - [x] Cadence set by `GROOM_INTERVAL_DAYS` (default 7); changing it changes frequency with no workflow-file edit. - [x] Base cron fires daily; a within-interval tick no-ops cheaply (exits in the `gate` job, before the finder). - [x] `workflow_dispatch` always runs regardless of the interval. - [x] Last-run state durable across CI runs (GitHub run history — no net-new secret, only `actions: read`). - [x] Default (unset variable) stays weekly. ## Judgment calls (worth a reviewer's eye) 1. **Last-run state = run history, not a `GROOM_LAST_RUN` repo variable.** The ticket sanctioned either. A repo variable needs a `Variables: write` credential the run doesn't otherwise carry — and a missing grant would fail *silently* into a daily over-spend. Run history is durable, needs only `actions: read`, and has no silent-failure trap. 2. **Interval delivered as a reusable `input` wired to the var** (rather than the reusable reading `vars.GROOM_INTERVAL_DAYS` directly). The caller evaluates the var in its own context — unambiguous — matching how `cadence`/`dry_run`/`volume_gate` are already wired. Same "no file edit" ergonomics. 3. **`workflow_dispatch` bypasses the *interval* gate but not the pre-existing *volume* gate** for a non-dry-run dispatch. That volume-gate-on-dispatch behavior is a deliberate spend guard the pilot chose (`volume_gate: dry_run != 'true'`); the ticket's "always runs regardless of the interval" is about the interval gate, so I left the volume gate alone (Chesterton's fence). A dry-run dispatch bypasses both, as before. 4. **`github.workflow_ref` resolves to the caller** (`ci-groom.yml`), not the reusable — the OIDC `workflow_ref` (caller) vs `job_workflow_ref` (reusable) distinction. If that assumption were ever wrong, the run-history query returns nothing → the gate **fails open** (runs), never crashes. 5. **A finder job that *failed* still counts as a real run** (not just success) — so a run that spent the billed audit but died at a later step doesn't re-spend on the next daily tick. Trade-off documented in `interval.py`. ## Testing - `python3 -m unittest discover -s .github/groom/tests` → **54 passed** (16 new for the interval gate: within/at/after-interval, skip-ticks-don't-reset, dispatch-always-runs, no-history/API-error fail-open, job-name matching). - `actionlint` (with shellcheck on the embedded `run:` blocks) clean on both workflows. --- <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:54 +00:00