Uh oh!
There was an error while loading. Please reload this page.
Consolidate shared workflows into a single generated automation.yml entry point - #88
Consolidate shared workflows into a single generated automation.yml entry point#88akolson wants to merge 5 commits into
Conversation
🔵 Review postedLast updated: 2026-07-05 05:15 UTC |
rtibblesbot
left a comment
There was a problem hiding this comment.
Solid generator/registry design and correctly-scoped permission narrowing, but the branch was forked before recent changes landed on main that this registry doesn't account for.
CI passing. No UI files in scope, Phase 3 skipped.
Blocking:
automation-registry.yml:79— stale/too-broadmanage-issue-headerdispatch condition, and a whole new automation (good-first-issue-comment) missing from the registry entirely (inline comment).
Suggestion:
- PR body —
Closes #86will auto-close the tracking issue despite the acknowledged-incomplete migration checklist; considerPart of #86instead, or open a follow-up tracking issue for the remaining per-repo migrations.
Nitpick:
package.json:10—js-yamlis only used by the build-time codegen script; considerdevDependenciesinstead.
@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly
How was this generated?
Ran a phased review pipeline over the pull request diff:
- Classified the diff to select review passes (core, frontend, backend) and whether manual QA was required
- Core review pass checked correctness, design, architecture, testing, completeness, and DRY/SRP/Rule-of-Three principles
- Specialized frontend/backend review passes applied framework-specific lenses where those files changed
- For UI changes: manual QA and an accessibility audit against a live dev server, when available
- Checked CI status and linked issue acceptance criteria
- Synthesized one review from those passes and chose the verdict from the findings, CI status, and QA evidence
| on: | ||
| issues: | ||
| types: [opened, reopened, labeled, unlabeled] | ||
| if: "github.event_name == 'issues' && contains(fromJSON('[\"opened\",\"reopened\",\"labeled\",\"unlabeled\"]'), github.event.action)" |
There was a problem hiding this comment.
blocking: This branch's merge-base with upstream/main (5512260) is stale — main has since merged #62/#90, which refactored issue-header dispatch:
manage-issue-header.yml's own leaf-levelif:filter (opened/reopened/labeled+help wanted/unlabeled+help wanted) was removed and pushed up into two new callers,issue-open.yml(unconditional for opened/reopened) andissue-label.yml(labeled/unlabeled gated tolabel.name == 'help wanted'only).- This registry entry's
if:fires on any labeled/unlabeled action regardless of label name, and since the leaf no longer self-filters, merging as-is meansmanage-issue-header.jsruns on every label add/remove on every issue — a real behavior regression, not just missed parity. issue-label.ymlalso adds a second job,good-first-issue-comment(gated onlabel.name in ['good first issue', 'help wanted']), calling a brand-new leafgood-first-issue-comment.yml. This automation has no entry anywhere in the registry, so it's silently dropped from the generatedautomation.yml.
Please rebase onto current upstream/main, add a good-first-issue-comment registry entry, and split this entry's if: to match issue-label.yml's help wanted-only gating.
There was a problem hiding this comment.
Done — rebased onto current main, split manage-issue-header into issue-open (opened/reopened, unconditional) and issue-label-header (labeled/unlabeled, help wanted only), and added a good-first-issue-comment entry calling the new leaf directly from automation.yml.
There was a problem hiding this comment.
Confirmed — all three are resolved. issue-open (opened/reopened, unconditional) and issue-label-header (labeled/unlabeled gated to help wanted) now match the split in issue-open.yml/issue-label.yml, and the new good-first-issue-comment entry dispatches good-first-issue-comment.yml directly with the good first issue/help wanted gate. Generated automation.yml reflects all three. Thanks for the quick turnaround.
| "axios": "^1.13.6", | ||
| "dotenv": "^17.3.1", | ||
| "googleapis": "^142.0.0", | ||
| "js-yaml": "^4.1.0", |
There was a problem hiding this comment.
nitpick:js-yaml is only consumed by scripts/generate-automation.js (a build-time tool), not by any runtime workflow script — consider moving it to devDependencies.
There was a problem hiding this comment.
Good catch — moved to devDependencies.
…ntry point Replace the per-feature call-*.yml callers and the pull-request-target.yml dispatcher with one reusable automation.yml plus a single generated caller template, driven by automation-registry.yml as the source of truth. A pre-commit hook regenerates and fails on drift between the registry and the generated files. Closes#86 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…chedule handling, permissions model - Mark all Slack/spreadsheet/GCP secrets as required in the registry to match leaf workflow declarations (restoring pre-PR behavior where callers forwarded all secrets unconditionally and leaves failed loudly if any were absent) - Add automation-caller.yml to the generate-automation pre-commit hook's files: pattern so direct edits to that generated file are caught by the drift check - Fix unionOn() to handle both object and list form for schedule entries, guarding against undefined cron if a second schedule is added in GHA-native format - Give automation-caller.yml its own generated header distinct from the consumer-facing one in automation-template.yml - Replace needs_write_permissions boolean with a generic permissions: map in the registry; generator now emits it directly rather than special-casing a flag - Compute collectSecrets() once in main() and pass to both build functions Regenerated automation.yml, automation-template.yml, and automation-caller.yml. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
automation.yml now dispatches good-first-issue-comment directly rather than going through the issue-label intermediate workflow. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
dab693f to
01729bfCompareOnly used by scripts/generate-automation.js at build time. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Replaces per-feature
call-*.ymlcallers across consumer repos with a singleautomation-template.ymlto copy once. The template calls a central reusableautomation.ymlthat dispatches to each leaf workflow based on the triggering event.Key pieces:
automation-registry.yml— source of truth: one entry per automation with its leaf, trigger events, dispatchif:, secrets, and permissionsscripts/generate-automation.js— generatesautomation.yml,automation-template.yml, andautomation-caller.yml(this repo's own copy) from the registryresolve-bot-pr-threadsneedscontents: write/pull-requests: writevia the default GITHUB_TOKEN; all other jobs narrow tocontents: readvia job-level permissions overrides inautomation.ymlAutomations wired up:
review-requestedpull-request-labeldependabot-reviewercontributor-pr-replyholiday-messageissue-openissue-label-headerhelp wantedgood-first-issue-commentgood first issueorhelp wantedupdate-pr-spreadsheetcommunity-contribution-labelcontributor-issue-commentresolve-bot-pr-threadsunassign-inactive-issuesNo leaf workflow's internal logic changed — this is a routing/packaging consolidation only.
References
Closes#86
Reviewer guidance
Verify that every automation in the table above fires on exactly the same events it did before this consolidation and that no leaf workflow's behaviour, permissions, or secret access has changed as a side effect of the routing restructure.
AI usage
Implemented end-to-end with Claude Code (Sonnet 4.6) from the architecture already laid out in issue #86 (which was itself drafted with Claude Code per its own AI-usage note). I directed the registry/generator design, reviewed the generated
automation.yml/automation-template.ymloutput, and specifically caught and directed the fix for a permissions-scoping gap (resolve-bot-pr-threadsneeding write access that the rest of the automations shouldn't inherit) before it became a security issue. Claude traced trigger parity against the originalcall-*.ymlfiles, ran the pre-commit/actionlint validation, and verified the generator's drift detection actually works rather than just asserting it does.